Last modified: 2014-08-30 20:19:16 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T47843, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 45843 - Rework API error reporting
Rework API error reporting
Status: NEW
Product: MediaWiki
Classification: Unclassified
API (Other open bugs)
1.21.x
All All
: Normal enhancement (vote)
: ---
Assigned To: Nobody - You can work on this!
http://www.mediawiki.org/wiki/Request...
:
Depends on:
Blocks: 35074
  Show dependency treegraph
 
Reported: 2013-03-07 14:44 UTC by Daniel Kinzler
Modified: 2014-08-30 20:19 UTC (History)
9 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Daniel Kinzler 2013-03-07 14:44:34 UTC
Error reporting from the API urgently needs work. The dieUsage() function originally designed to signal usage errors to bot owners has become the default way of reporting errors from the API, but is not suitable for showing detailed localized messages on the UI.


After some discussion, Anomie and I came up with the following plan (as far as I recall):

* The API needs to return message keys and parameters for errors and warnings, for client side localization. The
messages are returned as <message> elements in the <error> resp <warning> section.

* Error codes are no longer needed, message keys can be used as machine readable identifiers

* For backwards compatibility, the English translation of the message(s) (and, for errors, and error code) should be returned to the called. This can skip messages from the database.

* The old behavior should be retained as the default for the <error> and <warning> sections. The new behavior can be triggered using appropriate parameters (perhaps apimessages=true or some such)

* For convenience, the API can be instructed to include a translated HTML version of the messages in the output, in the user's language or some language given as a parameter (e.g. triggered using apimessage=html & apilang=fr), considering customization using message pages. The text is returned in a <html> element in the <error> resp <warning> section.


To allow the new functionality, the ApiBase class needs to support new functions for reporting errors and warnings:

* dieMessage( Message $msg ): report the given message as an error and abort. If old style output is requested, use the message key as the error code and use the English translation of the message as the error info.

* warnMessage( Message $msg ): report the given message as a warning and continue. If old style output is requested, use the English translation of the message in the <warning> section.

* handleStatus( Status $status ): use the messages from a Status object
** if the status is fatal, report any error messages in the Status object as errors and abort.
** if the status is not fatal but has errors or warnings, report all messages in the Status object as warnings and continue.
** else, just carry on.


For backwards compatibility, the old reporting functions should be retained (but deprecated). They should implement a sensible default when used with the new style output:

* dieUsage( $info, $code ): if the new style of error reporting is requested, this will generate a generic message (perhaps api-usage-error) with $info and $code as the two message parameters.

* dieUsageMsg( $error ): if the new style of error reporting is requested, report the message given as an error. Otherwise, keep using the old hard coded message-key-to-error-code mapping in ApiMain::$messageMap.


Example of the new behavior:

  <error>
    <message key="something-went-wrong">
      <param>Foo</param>
      <param>23</param>
    </message>
    <html lang="de">
      &lt;p&gt;Mit "Foo" ist etwas schiefgegangen (ID: 23).&lt;/p&gt;
    </html>
  </error>

  <warning>
    <message key="some-stuff"/>
    <message key="other-stuff">
      <param>X</param>
    </message>
    <html lang="de">
      &lt;ul&gt;
        &lt;li&gt;So Zeug!&lt;/li&gt;
        &lt;li&gt;Noch mehr Zeug: X!&lt;/li&gt;
      &lt;/ul&gt;
    </html>
  </warning>


Example of old style output for dieError( wfMessage( "nasty-error", "X" ) ):

  <error code="nasty-error" info="Nasty Error with X"/>

Example of new style output for dieUsage( "Nasty Error", "silly-code" ):

  <error>
    <message key="api-usage-error">
      <param>Nasty Error</param>
      <param>silly-code</param>
    </message>
  </error>
Comment 1 Brad Jorsch 2013-03-07 15:19:24 UTC
Looks like generally a good outline to me. A few comments:

* While MediaWiki hopefully has enough anti-IE-stupidity code in place, avoiding the string "<html>" in the XML output format would probably be for the best.

* Returning $msg->useDatabase( false )->text() is not just for backwards compatibility (although the back-compat output format will have to do that), it's also for bots that output to a text log or console where on-wiki-customized HTML messages don't make sense. $msg->useDatabase( true )->parse() is, of course, for UI callers where on-wiki-customized HTML messages do make sense.

* I think <message key="some-stuff" rendered="..."/> and allowing the client to combine the rendered values would be better than concatenating them all into <html> as you have it shown. Of course, the back-compat output would continue to concatenate all the plain-text warnings under a "*" key like it does now.

* For the back-compat output, dieMessage() should probably look up the key in the static array the same way dieUsageMsg() does now, for the code if not also for the text. But it should of course not choke like dieUsageMsg() does if the key isn't in the static array.
Comment 2 Kevin Israel (PleaseStand) 2013-03-07 15:45:19 UTC
Duplicate of bug 35074?
Comment 3 Daniel Kinzler 2013-03-07 15:48:26 UTC
(In reply to comment #2)
> Duplicate of bug 35074?

Possibly, though this bug is really a todo item for a concrete spec. We could mark bug 35074 as depending on this one, since implementing what is proposed here would solve that bug.
Comment 4 Yuri Astrakhan 2013-03-07 16:13:08 UTC
A few months ago I wrote a few notes on how to improve errors & warnings in the API roadmap RFC (make sure to expand the implementation section):
http://www.mediawiki.org/wiki/Requests_for_comment/API_roadmap#Errors_and_Warnings_Localization

I will try to keep that spec in sync with the ideas proposed here, but feel free to modify that page, so we can keep one "current proposal". Looking for good bits in the bug comment is not as good, and some good ideas might be accidently overlooked :)
Comment 5 Daniel Kinzler 2013-03-07 19:55:40 UTC
An additional idea, induced by looking at bug 44111:

When catching a ErrorPageError, the Message it contains should be passed to dieMessage(), so the caller's preferences as to localization and representation of the error are taken into account.
Comment 6 Brad Jorsch 2013-03-07 20:16:15 UTC
Note that's orthogonal to the actual bug in 44111, which has to do with what is written to the server logfile rather than what is output to the client.
Comment 7 Andre Klapper 2013-10-31 12:17:14 UTC
[replacing wikidata keyword by adding CC - see bug 56417]

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links