r52190 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r52189‎ | r52190 | r52191 >
Date:08:10, 20 June 2009
Author:catrope
Status:reverted (Comments)
Tags:
Comment:
API: Return HTTP 503 status code on maxlag error, like index.php does
Modified paths:
  • /trunk/phase3/RELEASE-NOTES (modified) (history)
  • /trunk/phase3/includes/api/ApiMain.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/api/ApiMain.php
@@ -378,11 +378,10 @@
379379 if ( $lag > $maxLag ) {
380380 header( 'Retry-After: ' . max( intval( $maxLag ), 5 ) );
381381 header( 'X-Database-Lag: ' . intval( $lag ) );
382 - // XXX: should we return a 503 HTTP error code like wfMaxlagError() does?
383382 if( $wgShowHostnames ) {
384 - $this->dieUsage( "Waiting for $host: $lag seconds lagged", 'maxlag' );
 383+ $this->dieUsage( "Waiting for $host: $lag seconds lagged", 'maxlag', 503 );
385384 } else {
386 - $this->dieUsage( "Waiting for a database server: $lag seconds lagged", 'maxlag' );
 385+ $this->dieUsage( "Waiting for a database server: $lag seconds lagged", 'maxlag', 503 );
387386 }
388387 return;
389388 }
Index: trunk/phase3/RELEASE-NOTES
@@ -225,6 +225,7 @@
226226 * (bug 18785) Add siprop=languages to meta=siteinfo
227227 * (bug 14200) Added user and excludeuser parameters to list=watchlist and
228228 list=recentchanges
 229+* Return HTTP 503 status code on maxlag error, like index.php does
229230
230231 === Languages updated in 1.16 ===
231232

Follow-up revisions

RevisionCommit summaryAuthorDate
r53353Revert r52190 ("Return HTTP 503 on API maxlag error"): announcement prompted ...catrope08:04, 16 July 2009

Comments

#Comment by Mr.Z-man (talk | contribs)   15:14, 20 June 2009

Note that this could be a breaking change for some clients. Python urllib/urllib2 for example raises an exception on 503 errors by default.

#Comment by Catrope (talk | contribs)   15:18, 20 June 2009

Good point, I'll announce it on the mailing list.

#Comment by Anomie (talk | contribs)   17:20, 30 June 2009

Don't forget to send that announcement. Unless our IRC conversation a few days ago changed your mind. ;)

For anyone else: While this change makes sense from one viewpoint, it doesn't from what is IMO a better view. index.php returns 503 because in the HTTP protocol that's how you indicate an error. But the API isn't HTTP, it just uses HTTP as a transport and reports its errors inside a successful HTTP response. This change basically raises API maxlag from "normal API error" to "transport-layer error", which could be considered equivalent to index.php sending an ICMP error on maxlag.

#Comment by R'n'B (talk | contribs)   13:59, 15 July 2009

I still haven't seen any announcement of this on the mailing list. Besides knowing that maxlag will throw an HTTP 503 status code, it would be helpful to know what the content of the message body will be, so that this can be distinguished from other 503 conditions (squid errors).

Status & tagging log