Skip to content

Support maxlag and Retry-After header #28

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 23, 2017
Merged

Support maxlag and Retry-After header #28

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 42 additions & 24 deletions src/Guzzle/MiddlewareFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,23 @@ public function retry( $delay = true ) {
* @return callable
*/
private function getRetryDelay() {
return function( $numberOfRetries ) {
return function( $numberOfRetries, Response $response = null ) {
// The $response argument is only passed as of Guzzle 6.2.2.
if( $response !== null ) {
// Retry-After may be a number of seconds or an absolute date (RFC 7231,
// section 7.1.3).
$retryAfter = $response->getHeaderLine( 'Retry-After' );

if( is_numeric( $retryAfter ) ) {
return 1000 * $retryAfter;
}

if( $retryAfter ) {
$seconds = strtotime( $retryAfter ) - time();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there some documentation that states that the header may contain a date / timestamp rather than a value in seconds?
I can't see it on the wiki page and I imagine including code like this will lead to some odd wait times if a date is returned due to timezones.

Copy link
Contributor Author

@c960657 c960657 Jan 23, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The API doc doesn't mention in, but it is allowed according to RFC 7231, section 7.1.3, so I thought it was worth spending a few lines on.

The RFC requires the absolute date returned to be a full date with timezone in a format supported by strtotime(), so timezones are not a problem. But it may cause issues if the local clock is very far behind – but I don't expect that to be a common problem.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added a reference to RFC 7231 in a comment.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Brilliant!

return 1000 * max( 1, $seconds );
}
}

return 1000 * $numberOfRetries;
};
}
Expand Down Expand Up @@ -81,38 +97,40 @@ private function newRetryDecider() {
}

if( $response ) {
$headers = $response->getHeaders();
$data = json_decode( $response->getBody(), true );

// Retry on server errors
if( $response->getStatusCode() >= 500 ) {
$shouldRetry = true;
}

if ( array_key_exists( 'mediawiki-api-error', $headers ) ) {
foreach( $headers['mediawiki-api-error'] as $mediawikiApiErrorHeader ) {
if (
// Retry if we have a response with an API error worth retrying
in_array(
$mediawikiApiErrorHeader,
array(
'ratelimited',
'readonly',
'internal_api_error_DBQueryError',
)
)
||
// Or if we have been stopped from saving as an 'anti-abuse measure'
// Note: this tries to match "actionthrottledtext" i18n messagae for mediawiki
(
$mediawikiApiErrorHeader == 'failed-save' &&
strstr( $data['error']['info'], 'anti-abuse measure' )
foreach( $response->getHeader( 'Mediawiki-Api-Error' ) as $mediawikiApiErrorHeader ) {
if (
// Retry if the API explicitly tells us to:
// https://www.mediawiki.org/wiki/Manual:Maxlag_parameter
$response->getHeaderLine( 'Retry-After' )
||
// Retry if we have a response with an API error worth retrying
in_array(
$mediawikiApiErrorHeader,
array(
'ratelimited',
'maxlag',
'readonly',
'internal_api_error_DBQueryError',
)
) {
$shouldRetry = true;
}

)
||
// Or if we have been stopped from saving as an 'anti-abuse measure'
// Note: this tries to match "actionthrottledtext" i18n messagae for mediawiki
(
$mediawikiApiErrorHeader == 'failed-save' &&
strstr( $data['error']['info'], 'anti-abuse measure' )
)
) {
$shouldRetry = true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method is really long... though that was the case before already and you did not make it worse

}

}
}

Expand Down
233 changes: 145 additions & 88 deletions tests/unit/Guzzle/MiddlewareFactoryTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,66 +20,52 @@
class MiddlewareFactoryTest extends \PHPUnit_Framework_TestCase {

public function testRetriesConnectException() {
$middlewareFactory = new MiddlewareFactory();

$mock = new MockHandler(
array(
new ConnectException( "Error 1", new Request( 'GET', 'test' ) ),
new Response( 200, [ 'X-Foo' => 'Bar' ] ),
)
);
$queue = [
new ConnectException( 'Error 1', new Request( 'GET', 'test' ) ),
new Response( 200, [ 'X-Foo' => 'Bar' ] ),
];

$handler = HandlerStack::create( $mock );
$handler->push( $middlewareFactory->retry( false ) );
$client = new Client( [ 'handler' => $handler ] );
$client = $this->getClient( $queue, $delays );
$response = $client->request( 'GET', '/' );

$this->assertEquals( 200, $client->request( 'GET', '/' )->getStatusCode() );
$this->assertEquals( 200, $response->getStatusCode() );
$this->assertEquals( [ 1000 ], $delays );
}

public function testRetries500Errors() {
$middlewareFactory = new MiddlewareFactory();

$mock = new MockHandler(
array(
new Response( 500 ),
new Response( 200 ),
)
);
$queue = [
new Response( 500 ),
new Response( 200 ),
];

$handler = HandlerStack::create( $mock );
$handler->push( $middlewareFactory->retry( false ) );
$client = new Client( [ 'handler' => $handler ] );
$client = $this->getClient( $queue, $delays );
$response = $client->request( 'GET', '/' );

$this->assertEquals( 200, $client->request( 'GET', '/' )->getStatusCode() );
$this->assertEquals( 200, $response->getStatusCode() );
$this->assertEquals( [ 1000 ], $delays );
}

public function testRetriesSomeMediawikiApiErrorHeaders() {
$middlewareFactory = new MiddlewareFactory();

$mock = new MockHandler(
array(
new Response( 200, array( 'mediawiki-api-error' => 'ratelimited' ) ),
new Response( 200, array( 'mediawiki-api-error' => 'readonly' ) ),
new Response( 200, array( 'mediawiki-api-error' => 'internal_api_error_DBQueryError' ) ),
new Response( 200, array( 'mediawiki-api-error' => 'DoNotRetryThisHeader' ) ),
)
);

$handler = HandlerStack::create( $mock );
$handler->push( $middlewareFactory->retry( false ) );
$client = new Client( [ 'handler' => $handler ] );

$queue = [
new Response( 200, array( 'mediawiki-api-error' => 'ratelimited' ) ),
new Response( 200, array( 'mediawiki-api-error' => 'maxlag' ) ),
new Response( 200, array( 'mediawiki-api-error' => 'readonly' ) ),
new Response( 200, array( 'mediawiki-api-error' => 'internal_api_error_DBQueryError' ) ),
new Response( 200, array( 'mediawiki-api-error' => 'DoNotRetryThisHeader' ) ),
];

$client = $this->getClient( $queue, $delays );
$response = $client->request( 'GET', '/' );

$this->assertEquals( 200, $response->getStatusCode() );
$this->assertEquals(
array( 'DoNotRetryThisHeader' ),
$response->getHeader( 'mediawiki-api-error' )
);
$this->assertEquals( [ 1000, 2000, 3000, 4000 ], $delays );
}

public function testRetryAntiAbuseMeasure() {
$middlewareFactory = new MiddlewareFactory();

$antiAbusejson = json_encode(
array(
'error' => array(
Expand All @@ -88,71 +74,142 @@ public function testRetryAntiAbuseMeasure() {
)
);

$mock = new MockHandler(
array(
new Response( 200, array( 'mediawiki-api-error' => 'failed-save' ), $antiAbusejson ),
new Response( 200, array( 'mediawiki-api-error' => 'DoNotRetryThisHeader' ) ),
)
);

$handler = HandlerStack::create( $mock );
$handler->push( $middlewareFactory->retry( false ) );
$client = new Client( [ 'handler' => $handler ] );
$queue = [
new Response( 200, array( 'mediawiki-api-error' => 'failed-save' ), $antiAbusejson ),
new Response( 200, array( 'mediawiki-api-error' => 'DoNotRetryThisHeader' ) ),
];

$client = $this->getClient( $queue, $delays );
$response = $client->request( 'GET', '/' );
$this->assertSame( 200, $response->getStatusCode() );
$this->assertEquals(
array( 'DoNotRetryThisHeader' ),
$response->getHeader( 'mediawiki-api-error' )
);

$this->assertEquals( 200, $response->getStatusCode() );
$this->assertEquals( 'DoNotRetryThisHeader', $response->getHeaderLine( 'mediawiki-api-error' ) );
}

public function testRetryLimit() {
$middlewareFactory = new MiddlewareFactory();

$mock = new MockHandler(
array(
new ConnectException( "Error 1", new Request( 'GET', 'test' ) ),
new ConnectException( "Error 2", new Request( 'GET', 'test' ) ),
new ConnectException( "Error 3", new Request( 'GET', 'test' ) ),
new ConnectException( "Error 4", new Request( 'GET', 'test' ) ),
new ConnectException( "Error 5", new Request( 'GET', 'test' ) ),
new ConnectException( "Error 6", new Request( 'GET', 'test' ) ),
)
);

$handler = HandlerStack::create( $mock );
$handler->push( $middlewareFactory->retry( false ) );
$client = new Client( [ 'handler' => $handler ] );
$queue = [
new ConnectException( 'Error 1', new Request( 'GET', 'test' ) ),
new ConnectException( 'Error 2', new Request( 'GET', 'test' ) ),
new ConnectException( 'Error 3', new Request( 'GET', 'test' ) ),
new ConnectException( 'Error 4', new Request( 'GET', 'test' ) ),
new ConnectException( 'Error 5', new Request( 'GET', 'test' ) ),
new ConnectException( 'Error 6', new Request( 'GET', 'test' ) ),
new Response( 200 ),
];

$client = $this->getClient( $queue );

$this->setExpectedException(
'GuzzleHttp\Exception\ConnectException',
'Error 6'
);

$client->request( 'GET', '/' )->getStatusCode();
$client->request( 'GET', '/' );
}

public function testRetryDelay() {
$middlewareFactory = new MiddlewareFactory();
public function testConnectExceptionRetryDelay() {
$queue = [
new ConnectException( '+1 second delay', new Request( 'GET', 'test' ) ),
new ConnectException( '+2 second delay', new Request( 'GET', 'test' ) ),
new Response( 200 ),
];

$mock = new MockHandler(
array(
new ConnectException( "+1 second delay", new Request( 'GET', 'test' ) ),
new ConnectException( "+2 second delay", new Request( 'GET', 'test' ) ),
new Response( 200 ),
)
);
$client = $this->getClient( $queue, $delays );
$response = $client->request( 'GET', '/' );

$this->assertEquals( 200, $response->getStatusCode() );
$this->assertEquals( [ 1000, 2000 ], $delays );
}

public function testServerErrorRetryDelay() {
$queue = [
new Response( 500 ),
new Response( 503 ),
new Response( 200 ),
];

$client = $this->getClient( $queue, $delays );
$response = $client->request( 'GET', '/' );

$this->assertEquals( 200, $response->getStatusCode() );
$this->assertEquals( [ 1000, 2000 ], $delays );
}

public function testRelativeRetryDelayHeaderRetryDelay() {
$queue = [
new Response( 200, [ 'mediawiki-api-error' => 'maxlag', 'retry-after' => 10 ] ),
new Response( 200 ),
];

$this->getClient( $queue, $delays )->request( 'GET', '/' );

$this->assertEquals( [ 10000 ], $delays );
}

public function testAbsoluteRetryDelayHeaderRetryDelay() {
$queue = [
new Response(
200,
[
'mediawiki-api-error' => 'maxlag',
'retry-after' => gmdate( DATE_RFC1123, time() + 600 ),
]
),
new Response( 200 ),
];

$client = $this->getClient( $queue, $delays );
$response = $client->request( 'GET', '/' );

$this->assertEquals( 200, $response->getStatusCode() );
$this->assertCount( 1, $delays );
// Allow 5 second delay while running this test.
$this->assertGreaterThan( 600000 - 5000 , $delays[0] );
}

public function testPastRetryDelayHeaderRetryDelay() {
$queue = [
new Response(
200,
[
'mediawiki-api-error' => 'maxlag',
'retry-after' => 'Fri, 31 Dec 1999 23:59:59 GMT',
]
),
new Response( 200 ),
];

$client = $this->getClient( $queue, $delays );
$response = $client->request( 'GET', '/' );

$this->assertEquals( 200, $response->getStatusCode() );
$this->assertEquals( [ 1000 ] , $delays );
}

private function getClient( array $queue, &$delays = null ) {
$mock = new MockHandler( $queue );

$handler = HandlerStack::create( $mock );
$handler->push( $middlewareFactory->retry( true ) );
$client = new Client( [ 'handler' => $handler ] );

$startTime = time();
$client->request( 'GET', '/' )->getStatusCode();
$endTime = time();
$middlewareFactory = new MiddlewareFactory();
$handler->push( $middlewareFactory->retry() );

$delayMocker = $this->getDelayMocker( $delays );
$handler->push( $delayMocker );

$this->assertGreaterThan( $startTime + 2, $endTime );
return new Client( [ 'handler' => $handler ] );
}

private function getDelayMocker( &$delays ) {
return function( callable $handler) use ( &$delays ) {
return function( $request, array $options ) use ( $handler, &$delays ) {
if( isset( $options['delay'] ) ) {
$delays[] = $options['delay'];
unset( $options['delay'] );
}
return $handler( $request, $options );
};
};

}
}