Gerrit change 17952 changed Special:UserLogin to return a Status object from the account creation function, passing it a RawMessage which breaks the expectation of the API's dieStatus method that it can get a sane key from the message object's getKey method.
But then Gerrit change 47821 broke things worse by changing Special:UserLogin to pass a raw string rather than a Message object or message key as Status::newFatal expects.
Version: 1.23.0
Severity: normal
I think the thing to do here is to allow sending back a *useful* Status object from the hook in place of just a string, which should be able to return a sane error. Lemme try...
The issue here is that the Status class is too intertwined with the Message class, and this was even before RawMessage was introduced. When you made a Status object, it was assumed any error keys were backed by messages.
The solution should be that the concerns need to be separated. The Status class should not have anything to do with the Message class. However, that may be a bit difficult to achieve, so at the very least there needs to be a separation between error codes and message keys.
Change 108088 had a related patch set uploaded by Brion VIBBER:
Add Status outparam for AbortNewAccount hook to fix API error handling
Change 108089 had a related patch set uploaded by Brion VIBBER:
Update ConfirmEdit to return Status object on AbortNewAccount hook
(In reply to comment #3)
The issue here is that the Status class is too intertwined with the Message
class, and this was even before RawMessage was introduced. When you made a
Status object, it was assumed any error keys were backed by messages.The solution should be that the concerns need to be separated. The Status
class
should not have anything to do with the Message class. However, that may be a
bit difficult to achieve, so at the very least there needs to be a separation
between error codes and message keys.
I'd rather not redo the entire interface of SpecialUserlogin and ApiCreateAccount just to get this going. Any strong objections to using this existing approach as a basic fix?
(In reply to comment #6)
Any strong objections to using this existing approach as a basic fix?
Nope (as indicated by my +1).
Change 108089 merged by jenkins-bot:
Update ConfirmEdit to return Status object on AbortNewAccount hook
Change 108088 merged by jenkins-bot:
Add Status outparam for AbortNewAccount hook to fix API error handling
Change 111113 had a related patch set uploaded by JGonera:
Add Status outparam for AbortNewAccount hook to fix API error handling
Change 111118 had a related patch set uploaded by MaxSem:
Update ConfirmEdit to return Status object on AbortNewAccount hook
Change 111118 merged by jenkins-bot:
Update ConfirmEdit to return Status object on AbortNewAccount hook
Change 111113 merged by MaxSem:
Add Status outparam for AbortNewAccount hook to fix API error handling