Skip to content

Commit

Permalink
When doing a retry, throw some randomness into the mix
Browse files Browse the repository at this point in the history
  • Loading branch information
Luc45 committed Mar 7, 2024
1 parent cbd4bf0 commit c8051a4
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 10 deletions.
2 changes: 2 additions & 0 deletions src/src/RequestBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,8 @@ protected function wait_after_429( string $headers, int $max_wait = 60 ): int {
// And no longer than 60 seconds.
$retry_after = min( $max_wait, $retry_after );

$retry_after += rand( 0, 5 ); // Add a random number of seconds to avoid all clients retrying at the same time.

return $retry_after;
}

Expand Down
53 changes: 43 additions & 10 deletions src/tests/RequestBuilderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace QIT_CLI_Tests;

use PHPUnit\Framework\AssertionFailedError;
use QIT_CLI\RequestBuilder;
use PHPUnit\Framework\TestCase;

Expand All @@ -20,10 +21,21 @@ public function wait_after_429( string $headers, int $max_wait = 60 ): int {
};
}

protected function assertRetryDelayWithinRange( $expected, $actual, $delta ) {
if ( $actual < $expected ) {
$this->fail( "Expected value is $expected, actual value is $actual, which is less than expected." );
} elseif ( $actual > $expected + $delta ) {
$this->fail( "Expected value is $expected, actual value is $actual, which is greater than expected + delta ($delta)." );
} else {
// If the actual value is within the acceptable range, explicitly assert true.
$this->assertTrue( true );
}
}

public function test_retry_after_seconds() {
$headers = "Retry-After: 59\r\nOther-Header: value";

$this->assertEquals( 59, $this->sut->wait_after_429( $headers ) );
$this->assertRetryDelayWithinRange( 59, $this->sut->wait_after_429( $headers ), 5 );
}

public function test_retry_after_http_date() {
Expand All @@ -34,35 +46,56 @@ public function test_retry_after_http_date() {
// Since time will pass between the creation of the date and this calculation,
// allow a small margin in the assertion
$expected_delay = $dateTime->getTimestamp() - time();
$this->assertEqualsWithDelta( $expected_delay, $this->sut->wait_after_429( $headers, 130 ), 5 );
$this->assertRetryDelayWithinRange( $expected_delay, $this->sut->wait_after_429( $headers, 130 ), 5 );
}

public function test_no_retry_after_header() {
$headers = "Other-Header: value";

$this->assertEquals( 5, $this->sut->wait_after_429( $headers ) );
$this->assertRetryDelayWithinRange( 5, $this->sut->wait_after_429( $headers ), 5 );
}

public function test_invalid_retry_after_header() {
$headers = "Retry-After: invalid\r\nOther-Header: value";

$this->assertEquals( 5, $this->sut->wait_after_429( $headers ) );
$this->assertRetryDelayWithinRange( 5, $this->sut->wait_after_429( $headers ), 5 );
}

public function test_exponential_backoff() {
$headers = "Retry-After: invalid\r\nOther-Header: value";

$this->assertEquals( 5, $this->sut->wait_after_429( $headers ) );
$this->assertRetryDelayWithinRange( 5, $this->sut->wait_after_429( $headers ), 5 );
// Mimick integration-level test.
$this->sut->retry_429 --;
$this->assertEquals( 10, $this->sut->wait_after_429( $headers ) );
$this->assertRetryDelayWithinRange( 10, $this->sut->wait_after_429( $headers ), 5 );
$this->sut->retry_429 --;
$this->assertEquals( 20, $this->sut->wait_after_429( $headers ) );
$this->assertRetryDelayWithinRange( 20, $this->sut->wait_after_429( $headers ), 5 );
$this->sut->retry_429 --;
$this->assertEquals( 40, $this->sut->wait_after_429( $headers ) );
$this->assertRetryDelayWithinRange( 40, $this->sut->wait_after_429( $headers ), 5 );
$this->sut->retry_429 --;
$this->assertEquals( 80, $this->sut->wait_after_429( $headers, 300 ) );
$this->assertRetryDelayWithinRange( 80, $this->sut->wait_after_429( $headers, 200 ), 5 );
$this->sut->retry_429 --;
$this->assertEquals( 160, $this->sut->wait_after_429( $headers, 300 ) );
$this->assertRetryDelayWithinRange( 160, $this->sut->wait_after_429( $headers, 200 ), 5 );
}

//
// Some tests for our custom assertion.
//
public function testExactValue() {
$this->assertRetryDelayWithinRange( 10, 10, 1 ); // Delta of 1
}

public function testWithinDelta() {
$this->assertRetryDelayWithinRange( 10, 11, 1 ); // Within delta of 1
}

public function testExceedsDelta() {
$this->expectException( AssertionFailedError::class );
$this->assertRetryDelayWithinRange( 10, 12, 1 ); // Exceeds delta of 1
}

public function testBelowExpected() {
$this->expectException( AssertionFailedError::class );
$this->assertRetryDelayWithinRange( 10, 9, 1 ); // Below expected
}
}

0 comments on commit c8051a4

Please sign in to comment.