Skip to content

Commit

Permalink
Merge pull request #755 from stripe/ralex/retry-headers
Browse files Browse the repository at this point in the history 
Respect Stripe-Should-Retry and Retry-After headers
  • Loading branch information
@rattrayalex-stripe
rattrayalex-stripe authored Oct 7, 2019
2 parents bebdb83 + 9f8463f commit 1dc57c4
Show file tree
Hide file tree
Showing 3 changed files with 105 additions and 88 deletions.
103 changes: 50 additions & 53 deletions lib/HttpClient/CurlClient.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -204,18 +204,6 @@ public function request($method, $absUrl, $headers, $params, $hasFile)
}
}

// Create a callback to capture HTTP headers for the response
$rheaders = new Util\CaseInsensitiveArray();
$headerCallback = function ($curl, $header_line) use (&$rheaders) {
// Ignore the HTTP request line (HTTP/1.1 200 OK)
if (strpos($header_line, ":") === false) {
return strlen($header_line);
}
list($key, $value) = explode(":", trim($header_line), 2);
$rheaders[trim($key)] = trim($value);
return strlen($header_line);
};

// By default for large request body sizes (> 1024 bytes), cURL will
// send a request without a body and with a `Expect: 100-continue`
// header, which gives the server a chance to respond with an error
Expand All @@ -235,7 +223,6 @@ public function request($method, $absUrl, $headers, $params, $hasFile)
$opts[CURLOPT_RETURNTRANSFER] = true;
$opts[CURLOPT_CONNECTTIMEOUT] = $this->connectTimeout;
$opts[CURLOPT_TIMEOUT] = $this->timeout;
$opts[CURLOPT_HEADERFUNCTION] = $headerCallback;
$opts[CURLOPT_HTTPHEADER] = $headers;
$opts[CURLOPT_CAINFO] = Stripe::getCABundlePath();
if (!Stripe::getVerifySslCerts()) {
Expand All @@ -247,7 +234,7 @@ public function request($method, $absUrl, $headers, $params, $hasFile)
$opts[CURLOPT_HTTP_VERSION] = CURL_HTTP_VERSION_2TLS;
}

list($rbody, $rcode) = $this->executeRequestWithRetries($opts, $absUrl);
list($rbody, $rcode, $rheaders) = $this->executeRequestWithRetries($opts, $absUrl);

return [$rbody, $rcode, $rheaders];
}
Expand All @@ -264,6 +251,19 @@ private function executeRequestWithRetries($opts, $absUrl)
$rcode = 0;
$errno = 0;

// Create a callback to capture HTTP headers for the response
$rheaders = new Util\CaseInsensitiveArray();
$headerCallback = function ($curl, $header_line) use (&$rheaders) {
// Ignore the HTTP request line (HTTP/1.1 200 OK)
if (strpos($header_line, ":") === false) {
return strlen($header_line);
}
list($key, $value) = explode(":", trim($header_line), 2);
$rheaders[trim($key)] = trim($value);
return strlen($header_line);
};
$opts[CURLOPT_HEADERFUNCTION] = $headerCallback;

$this->resetCurlHandle();
curl_setopt_array($this->curlHandle, $opts);
$rbody = curl_exec($this->curlHandle);
Expand All @@ -278,9 +278,9 @@ private function executeRequestWithRetries($opts, $absUrl)
$this->closeCurlHandle();
}

if ($this->shouldRetry($errno, $isPost, $rcode, $rbody, $numRetries)) {
if ($this->shouldRetry($errno, $rcode, $rheaders, $numRetries)) {
$numRetries += 1;
$sleepSeconds = $this->sleepTime($numRetries);
$sleepSeconds = $this->sleepTime($numRetries, $rheaders);
usleep(intval($sleepSeconds * 1000000));
} else {
break;
Expand All @@ -291,7 +291,7 @@ private function executeRequestWithRetries($opts, $absUrl)
$this->handleCurlError($absUrl, $errno, $message, $numRetries);
}

return [$rbody, $rcode];
return [$rbody, $rcode, $rheaders];
}

/**
Expand Down Expand Up @@ -340,14 +340,13 @@ private function handleCurlError($url, $errno, $message, $numRetries)
* HTTP statuses.
*
* @param int $errno
* @param bool $isPost
* @param int $rcode
* @param string $rbody
* @param array|CaseInsensitiveArray $rheaders
* @param int $numRetries
*
* @return bool
*/
private function shouldRetry($errno, $isPost, $rcode, $rbody, $numRetries)
private function shouldRetry($errno, $rcode, $rheaders, $numRetries)
{
if ($numRetries >= Stripe::getMaxNetworkRetries()) {
return false;
Expand All @@ -365,51 +364,43 @@ private function shouldRetry($errno, $isPost, $rcode, $rbody, $numRetries)
return true;
}

// 409 Conflict
if ($rcode === 409) {
return true;
}

// 429 Too Many Requests
//
// There are a few different problems that can lead to a 429. The most
// common is rate limiting, on which we *don't* want to retry because
// that'd likely contribute to more contention problems. However, some
// 429s are lock timeouts, which is when a request conflicted with
// another request or an internal process on some particular object.
// These 429s are safe to retry.
if ($rcode === 429) {
// It's not great that we're doing this here. In a future version,
// we should decouple the retry logic from the CurlClient instance,
// so that we don't need to deserialize here (and also so that the
// retry logic applies to non-curl clients).
$resp = json_decode($rbody, true);
if ($resp !== null && array_key_exists('error', $resp)) {
$error = \Stripe\ErrorObject::constructFrom($resp['error']);
if ($error->code === \Stripe\ErrorObject::CODE_LOCK_TIMEOUT) {
return true;
}
// The API may ask us not to retry (eg; if doing so would be a no-op)
// or advise us to retry (eg; in cases of lock timeouts); we defer to that.
if (isset($rheaders['stripe-should-retry'])) {
if ($rheaders['stripe-should-retry'] === 'false') {
return false;
}
if ($rheaders['stripe-should-retry'] === 'true') {
return true;
}
}

// 500 Internal Server Error
//
// We only bother retrying these for non-POST requests. POSTs end up
// being cached by the idempotency layer so there's no purpose in
// retrying them.
if ($rcode >= 500 && !$isPost) {
// 409 Conflict
if ($rcode === 409) {
return true;
}

// 503 Service Unavailable
if ($rcode == 503) {
// Retry on 500, 503, and other internal errors.
//
// Note that we expect the stripe-should-retry header to be false
// in most cases when a 500 is returned, since our idempotency framework
// would typically replay it anyway.
if ($rcode >= 500) {
return true;
}

return false;
}

private function sleepTime($numRetries)
/**
* Provides the number of seconds to wait before retrying a request.
*
* @param int $numRetries
* @param array|CaseInsensitiveArray $rheaders
*
* @return int
*/
private function sleepTime($numRetries, $rheaders)
{
// Apply exponential backoff with $initialNetworkRetryDelay on the
// number of $numRetries so far as inputs. Do not allow the number to exceed
Expand All @@ -426,6 +417,12 @@ private function sleepTime($numRetries)
// But never sleep less than the base sleep seconds.
$sleepSeconds = max(Stripe::getInitialNetworkRetryDelay(), $sleepSeconds);

// And never sleep less than the time the API asks us to wait, assuming it's a reasonable ask.
$retryAfter = isset($rheaders['retry-after']) ? floatval($rheaders['retry-after']) : 0.0;
if (floor($retryAfter) == $retryAfter && $retryAfter <= Stripe::getMaxRetryAfter()) {
$sleepSeconds = max($sleepSeconds, $retryAfter);
}

return $sleepSeconds;
}

Expand Down
11 changes: 11 additions & 0 deletions lib/Stripe.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@ class Stripe
// @var float Maximum delay between retries, in seconds
private static $maxNetworkRetryDelay = 2.0;

// @var float Maximum delay between retries, in seconds, that will be respected from the Stripe API
private static $maxRetryAfter = 60.0;

// @var float Initial delay between retries, in seconds
private static $initialNetworkRetryDelay = 0.5;

Expand Down Expand Up @@ -235,6 +238,14 @@ public static function getMaxNetworkRetryDelay()
return self::$maxNetworkRetryDelay;
}

/**
* @return float Maximum delay between retries, in seconds, that will be respected from the Stripe API
*/
public static function getMaxRetryAfter()
{
return self::$maxRetryAfter;
}

/**
* @return float Initial delay between retries, in seconds
*/
Expand Down
79 changes: 44 additions & 35 deletions tests/Stripe/HttpClient/CurlClientTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ public function testShouldRetryOnTimeout()

$curlClient = new CurlClient();

$this->assertTrue($this->shouldRetryMethod->invoke($curlClient, CURLE_OPERATION_TIMEOUTED, true, 0, "", 0));
$this->assertTrue($this->shouldRetryMethod->invoke($curlClient, CURLE_OPERATION_TIMEOUTED, 0, [], 0));
}

public function testShouldRetryOnConnectionFailure()
Expand All @@ -139,7 +139,7 @@ public function testShouldRetryOnConnectionFailure()

$curlClient = new CurlClient();

$this->assertTrue($this->shouldRetryMethod->invoke($curlClient, CURLE_COULDNT_CONNECT, true, 0, "", 0));
$this->assertTrue($this->shouldRetryMethod->invoke($curlClient, CURLE_COULDNT_CONNECT, 0, [], 0));
}

public function testShouldRetryOnConflict()
Expand All @@ -148,61 +148,55 @@ public function testShouldRetryOnConflict()

$curlClient = new CurlClient();

$this->assertTrue($this->shouldRetryMethod->invoke($curlClient, 0, true, 409, "", 0));
$this->assertTrue($this->shouldRetryMethod->invoke($curlClient, 0, 409, [], 0));
}

public function testShouldRetryOn429WhenLockTimeout()
{
Stripe::setMaxNetworkRetries(2);

$curlClient = new CurlClient();

$this->assertTrue($this->shouldRetryMethod->invoke($curlClient, 0, true, 429, '{"error": {"code": "lock_timeout"}}', 0));
}

public function testShouldNotRetryOn429WhenNotLockTimeout()
public function testShouldNotRetryOn429()
{
Stripe::setMaxNetworkRetries(2);

$curlClient = new CurlClient();

$this->assertFalse($this->shouldRetryMethod->invoke($curlClient, 0, true, 429, '{"error": {"code": "rate_limited"}}', 0));
$this->assertFalse($this->shouldRetryMethod->invoke($curlClient, 0, 429, [], 0));
}

public function testShouldNotRetryOn429WhenInvalidJson()
public function testShouldRetryOn500()
{
Stripe::setMaxNetworkRetries(2);

$curlClient = new CurlClient();

$this->assertFalse($this->shouldRetryMethod->invoke($curlClient, 0, true, 429, 'this is not valid JSON', 0));
$this->assertTrue($this->shouldRetryMethod->invoke($curlClient, 0, 500, [], 0));
}

public function testShouldRetryOn500AndNonPost()
public function testShouldRetryOn503()
{
Stripe::setMaxNetworkRetries(2);

$curlClient = new CurlClient();

$this->assertTrue($this->shouldRetryMethod->invoke($curlClient, 0, false, 500, "", 0));
$this->assertTrue($this->shouldRetryMethod->invoke($curlClient, 0, 503, [], 0));
}

public function testShouldNotRetryOn500AndPost()
public function testShouldRetryOnStripeShouldRetryTrue()
{
Stripe::setMaxNetworkRetries(2);

$curlClient = new CurlClient();

$this->assertFalse($this->shouldRetryMethod->invoke($curlClient, 0, true, 500, "", 0));
$this->assertFalse($this->shouldRetryMethod->invoke($curlClient, 0, 400, [], 0));
$this->assertTrue($this->shouldRetryMethod->invoke($curlClient, 0, 400, ['stripe-should-retry' => 'true'], 0));
}

public function testShouldRetryOn503()
public function testShouldNotRetryOnStripeShouldRetryFalse()
{
Stripe::setMaxNetworkRetries(2);

$curlClient = new CurlClient();

$this->assertTrue($this->shouldRetryMethod->invoke($curlClient, 0, true, 503, "", 0));
$this->assertTrue($this->shouldRetryMethod->invoke($curlClient, 0, 500, [], 0));
$this->assertFalse($this->shouldRetryMethod->invoke($curlClient, 0, 500, ['stripe-should-retry' => 'false'], 0));
}

public function testShouldNotRetryAtMaximumCount()
Expand All @@ -211,7 +205,7 @@ public function testShouldNotRetryAtMaximumCount()

$curlClient = new CurlClient();

$this->assertFalse($this->shouldRetryMethod->invoke($curlClient, 0, true, 0, "", Stripe::getMaxNetworkRetries()));
$this->assertFalse($this->shouldRetryMethod->invoke($curlClient, 0, 0, [], Stripe::getMaxNetworkRetries()));
}

public function testShouldNotRetryOnCertValidationError()
Expand All @@ -220,7 +214,7 @@ public function testShouldNotRetryOnCertValidationError()

$curlClient = new CurlClient();

$this->assertFalse($this->shouldRetryMethod->invoke($curlClient, CURLE_SSL_PEER_CERTIFICATE, true, -1, "", 0));
$this->assertFalse($this->shouldRetryMethod->invoke($curlClient, CURLE_SSL_PEER_CERTIFICATE, -1, [], 0));
}

public function testSleepTimeShouldGrowExponentially()
Expand All @@ -231,19 +225,19 @@ public function testSleepTimeShouldGrowExponentially()

$this->assertEquals(
Stripe::getInitialNetworkRetryDelay() * 1,
$this->sleepTimeMethod->invoke($curlClient, 1)
$this->sleepTimeMethod->invoke($curlClient, 1, [])
);
$this->assertEquals(
Stripe::getInitialNetworkRetryDelay() * 2,
$this->sleepTimeMethod->invoke($curlClient, 2)
$this->sleepTimeMethod->invoke($curlClient, 2, [])
);
$this->assertEquals(
Stripe::getInitialNetworkRetryDelay() * 4,
$this->sleepTimeMethod->invoke($curlClient, 3)
$this->sleepTimeMethod->invoke($curlClient, 3, [])
);
$this->assertEquals(
Stripe::getInitialNetworkRetryDelay() * 8,
$this->sleepTimeMethod->invoke($curlClient, 4)
$this->sleepTimeMethod->invoke($curlClient, 4, [])
);
}

Expand All @@ -254,10 +248,25 @@ public function testSleepTimeShouldEnforceMaxNetworkRetryDelay()

$curlClient = new CurlClient(null, $this->createFakeRandomGenerator());

$this->assertEquals(1, $this->sleepTimeMethod->invoke($curlClient, 1));
$this->assertEquals(2, $this->sleepTimeMethod->invoke($curlClient, 2));
$this->assertEquals(2, $this->sleepTimeMethod->invoke($curlClient, 3));
$this->assertEquals(2, $this->sleepTimeMethod->invoke($curlClient, 4));
$this->assertEquals(1, $this->sleepTimeMethod->invoke($curlClient, 1, []));
$this->assertEquals(2, $this->sleepTimeMethod->invoke($curlClient, 2, []));
$this->assertEquals(2, $this->sleepTimeMethod->invoke($curlClient, 3, []));
$this->assertEquals(2, $this->sleepTimeMethod->invoke($curlClient, 4, []));
}

public function testSleepTimeShouldRespectRetryAfter()
{
$this->setInitialNetworkRetryDelay(1);
$this->setMaxNetworkRetryDelay(2);

$curlClient = new CurlClient(null, $this->createFakeRandomGenerator());

// Uses max of default and header.
$this->assertEquals(10, $this->sleepTimeMethod->invoke($curlClient, 1, ['retry-after' => '10']));
$this->assertEquals(2, $this->sleepTimeMethod->invoke($curlClient, 2, ['retry-after' => '1']));

// Ignores excessively large values.
$this->assertEquals(2, $this->sleepTimeMethod->invoke($curlClient, 2, ['retry-after' => '100']));
}

public function testSleepTimeShouldAddSomeRandomness()
Expand All @@ -272,12 +281,12 @@ public function testSleepTimeShouldAddSomeRandomness()

// the initial value cannot be smaller than the base,
// so the randomness is ignored
$this->assertEquals(Stripe::getInitialNetworkRetryDelay(), $this->sleepTimeMethod->invoke($curlClient, 1));
$this->assertEquals(Stripe::getInitialNetworkRetryDelay(), $this->sleepTimeMethod->invoke($curlClient, 1, []));

// after the first one, the randomness is applied
$this->assertEquals($baseValue * 2, $this->sleepTimeMethod->invoke($curlClient, 2));
$this->assertEquals($baseValue * 4, $this->sleepTimeMethod->invoke($curlClient, 3));
$this->assertEquals($baseValue * 8, $this->sleepTimeMethod->invoke($curlClient, 4));
$this->assertEquals($baseValue * 2, $this->sleepTimeMethod->invoke($curlClient, 2, []));
$this->assertEquals($baseValue * 4, $this->sleepTimeMethod->invoke($curlClient, 3, []));
$this->assertEquals($baseValue * 8, $this->sleepTimeMethod->invoke($curlClient, 4, []));
}

public function testResponseHeadersCaseInsensitive()
Expand Down

0 comments on commit 1dc57c4

Please sign in to comment.