Respect retry-after header

stripe · Oct 5, 2019 · dd58835 · dd58835
1 parent c550dfd
commit dd58835
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 14 deletions.
diff --git a/lib/HttpClient/CurlClient.php b/lib/HttpClient/CurlClient.php
@@ -280,7 +280,7 @@ private function executeRequestWithRetries($opts, $absUrl)
 
             if ($this->shouldRetry($errno, $rcode, $rheaders, $numRetries)) {
                 $numRetries += 1;
-                $sleepSeconds = $this->sleepTime($numRetries);
+                $sleepSeconds = $this->sleepTime($numRetries, $rheaders);
                 usleep(intval($sleepSeconds * 1000000));
             } else {
                 break;
@@ -390,7 +390,15 @@ private function shouldRetry($errno, $rcode, $rheaders, $numRetries)
         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
@@ -407,6 +415,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 = floatval($rheaders['retry-after']);
+        if (floor($retryAfter) == $retryAfter && $retryAfter <= Stripe::getMaxRetryAfter()) {
+            $sleepSeconds = max($sleepSeconds, $retryAfter);
+        }
+
         return $sleepSeconds;
     }
 

diff --git a/lib/Stripe.php b/lib/Stripe.php
@@ -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;
 
@@ -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
      */

diff --git a/tests/Stripe/HttpClient/CurlClientTest.php b/tests/Stripe/HttpClient/CurlClientTest.php
@@ -225,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, [])
         );
     }
 
@@ -248,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()
@@ -266,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()