From acdfea883670354991d2cb254f73c07b3293363e Mon Sep 17 00:00:00 2001 From: Faidon Liambotis Date: Tue, 25 Mar 2014 12:35:49 +0200 Subject: [PATCH] Set ThrottledError's response code to 429 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 | 2 +- tests/phpunit/includes/exception/ThrottledErrorTest.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/includes/exception/ThrottledError.php b/includes/exception/ThrottledError.php index ce5d52e659..bec0d904be 100644 --- a/includes/exception/ThrottledError.php +++ b/includes/exception/ThrottledError.php @@ -34,7 +34,7 @@ class ThrottledError extends ErrorPageError { public function report() { global $wgOut; - $wgOut->setStatusCode( 503 ); + $wgOut->setStatusCode( 429 ); parent::report(); } } diff --git a/tests/phpunit/includes/exception/ThrottledErrorTest.php b/tests/phpunit/includes/exception/ThrottledErrorTest.php index 8995a0d80f..bdb143faaf 100644 --- a/tests/phpunit/includes/exception/ThrottledErrorTest.php +++ b/tests/phpunit/includes/exception/ThrottledErrorTest.php @@ -37,7 +37,7 @@ class ThrottledErrorTest extends MediaWikiTestCase { ->getMock(); $mock->expects( $this->once() ) ->method( 'setStatusCode' ) - ->with( 503 ); + ->with( 429 ); return $mock; } -- 2.20.1