diff --git a/lib/HttpClient/CurlClient.php b/lib/HttpClient/CurlClient.php index 17448d003..0c077e9c0 100644 --- a/lib/HttpClient/CurlClient.php +++ b/lib/HttpClient/CurlClient.php @@ -258,6 +258,7 @@ public function request($method, $absUrl, $headers, $params, $hasFile) private function executeRequestWithRetries($opts, $absUrl) { $numRetries = 0; + $isPost = array_key_exists(CURLOPT_POST, $opts) && $opts[CURLOPT_POST] == 1; while (true) { $rcode = 0; @@ -277,7 +278,7 @@ private function executeRequestWithRetries($opts, $absUrl) $this->closeCurlHandle(); } - if ($this->shouldRetry($errno, $rcode, $numRetries)) { + if ($this->shouldRetry($errno, $isPost, $rcode, $numRetries)) { $numRetries += 1; $sleepSeconds = $this->sleepTime($numRetries); usleep(intval($sleepSeconds * 1000000)); @@ -338,11 +339,12 @@ private function handleCurlError($url, $errno, $message, $numRetries) * socket errors that may represent an intermittent problem and some special * HTTP statuses. * @param int $errno + * @param bool $isPost * @param int $rcode * @param int $numRetries * @return bool */ - private function shouldRetry($errno, $rcode, $numRetries) + private function shouldRetry($errno, $isPost, $rcode, $numRetries) { if ($numRetries >= Stripe::getMaxNetworkRetries()) { return false; @@ -360,11 +362,25 @@ private function shouldRetry($errno, $rcode, $numRetries) return true; } - // 409 conflict + // 409 Conflict if ($rcode === 409) { 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) { + return true; + } + + // 503 Service Unavailable + if ($rcode == 503) { + return true; + } + return false; } diff --git a/tests/Stripe/HttpClient/CurlClientTest.php b/tests/Stripe/HttpClient/CurlClientTest.php index 25c24cede..0900e3c18 100644 --- a/tests/Stripe/HttpClient/CurlClientTest.php +++ b/tests/Stripe/HttpClient/CurlClientTest.php @@ -130,7 +130,7 @@ public function testShouldRetryOnTimeout() $curlClient = new CurlClient(); - $this->assertTrue($this->shouldRetryMethod->invoke($curlClient, CURLE_OPERATION_TIMEOUTED, 0, 0)); + $this->assertTrue($this->shouldRetryMethod->invoke($curlClient, CURLE_OPERATION_TIMEOUTED, true, 0, 0)); } public function testShouldRetryOnConnectionFailure() @@ -139,7 +139,7 @@ public function testShouldRetryOnConnectionFailure() $curlClient = new CurlClient(); - $this->assertTrue($this->shouldRetryMethod->invoke($curlClient, CURLE_COULDNT_CONNECT, 0, 0)); + $this->assertTrue($this->shouldRetryMethod->invoke($curlClient, CURLE_COULDNT_CONNECT, true, 0, 0)); } public function testShouldRetryOnConflict() @@ -148,7 +148,34 @@ public function testShouldRetryOnConflict() $curlClient = new CurlClient(); - $this->assertTrue($this->shouldRetryMethod->invoke($curlClient, 0, 409, 0)); + $this->assertTrue($this->shouldRetryMethod->invoke($curlClient, 0, true, 409, 0)); + } + + public function testShouldRetryOn500AndNonPost() + { + Stripe::setMaxNetworkRetries(2); + + $curlClient = new CurlClient(); + + $this->assertTrue($this->shouldRetryMethod->invoke($curlClient, 0, false, 500, 0)); + } + + public function testShouldNotRetryOn500AndPost() + { + Stripe::setMaxNetworkRetries(2); + + $curlClient = new CurlClient(); + + $this->assertFalse($this->shouldRetryMethod->invoke($curlClient, 0, true, 500, 0)); + } + + public function testShouldRetryOn503() + { + Stripe::setMaxNetworkRetries(2); + + $curlClient = new CurlClient(); + + $this->assertTrue($this->shouldRetryMethod->invoke($curlClient, 0, true, 503, 0)); } public function testShouldNotRetryAtMaximumCount() @@ -157,7 +184,7 @@ public function testShouldNotRetryAtMaximumCount() $curlClient = new CurlClient(); - $this->assertFalse($this->shouldRetryMethod->invoke($curlClient, 0, 0, Stripe::getMaxNetworkRetries())); + $this->assertFalse($this->shouldRetryMethod->invoke($curlClient, 0, true, 0, Stripe::getMaxNetworkRetries())); } public function testShouldNotRetryOnCertValidationError() @@ -166,7 +193,7 @@ public function testShouldNotRetryOnCertValidationError() $curlClient = new CurlClient(); - $this->assertFalse($this->shouldRetryMethod->invoke($curlClient, CURLE_SSL_PEER_CERTIFICATE, -1, 0)); + $this->assertFalse($this->shouldRetryMethod->invoke($curlClient, CURLE_SSL_PEER_CERTIFICATE, true, -1, 0)); } public function testSleepTimeShouldGrowExponentially()