Set ThrottledError's response code to 429
authorFaidon Liambotis <faidon@wikimedia.org>
Tue, 25 Mar 2014 10:35:49 +0000 (12:35 +0200)
committerFaidon Liambotis <faidon@wikimedia.org>
Fri, 28 Mar 2014 10:25:59 +0000 (12:25 +0200)
ThrottledError currently returns a 503, which in turn results into
badly-written spambots occasionally flooding our 5xx logs and graphs.

There is no reason, however, for ThrottledError to return a 5xx in the
first place: it's a user-generated error (user hitting a rate limit and
being throttled), not a server error. 5xx error codes in general have
many other implications, such as frontend caches treating this as a
backend failure and potentially retrying the same request, so they are
unsuitable and undesirable for the ThrottledError exception.

RFC 6585 (April 2012, updates: 2616) has added a special 4xx code
specifically for rate-limiting, 429 Too Many Requests. As the
description of that code matches exactly what ThrottledError was meant
for, switch it to using 429 instead.

Note that there is a chance 429 might be mistreated and not showed by
older or badly-written user agents as it's fairly new and not part of
RFC 2616, the original HTTP/1.1 spec. However, the last paragraph of
section 6.1.1 of RFC 2616, specifically covers the issue of UAs &
unknown status codes: it dictates that applications MUST understand the
class of any status code and treat them as the "x00 status code of that
class" (here: 400), MUST NOT be cached, and "SHOULD present to the user
the entity returned with the response, since that entity is likely to
include human-readable information which will explain the unusual
status".

Change-Id: I46335a76096ec800ee8ce5471bacffd41d2dc4f6

includes/exception/ThrottledError.php
tests/phpunit/includes/exception/ThrottledErrorTest.php

index ce5d52e..bec0d90 100644 (file)
@@ -34,7 +34,7 @@ class ThrottledError extends ErrorPageError {
 
        public function report() {
                global $wgOut;
-               $wgOut->setStatusCode( 503 );
+               $wgOut->setStatusCode( 429 );
                parent::report();
        }
 }
index 8995a0d..bdb143f 100644 (file)
@@ -37,7 +37,7 @@ class ThrottledErrorTest extends MediaWikiTestCase {
                        ->getMock();
                $mock->expects( $this->once() )
                        ->method( 'setStatusCode' )
-                       ->with( 503 );
+                       ->with( 429 );
                return $mock;
        }