diff --git a/src/Storage/Device/S3.php b/src/Storage/Device/S3.php index 429543c6..b910e2be 100644 --- a/src/Storage/Device/S3.php +++ b/src/Storage/Device/S3.php @@ -891,8 +891,11 @@ protected function call(string $operation, string $method, string $uri, string $ $response->code = \curl_getinfo($curl, CURLINFO_HTTP_CODE); $attempt = 0; - while ($attempt < self::$retryAttempts && $response->code >= 500) { - usleep(self::$retryDelay * 1000); + while ( + $attempt < self::$retryAttempts && + $this->isTransientError($response->code, $response->body) + ) { + \usleep(self::$retryDelay * 1000); $attempt++; $result = \curl_exec($curl); $response->code = \curl_getinfo($curl, CURLINFO_HTTP_CODE); @@ -958,6 +961,39 @@ private function parseAndThrowS3Error(string $errorBody, int $statusCode): void throw new Exception($errorBody, $statusCode); } + /** + * Determine whether an S3 response indicates a transient rate-limiting error + * (e.g. SlowDown, ServiceUnavailable) that should be retried with exponential backoff. + * + * The XML body is parsed first so that specific S3 error codes are detected regardless + * of HTTP status. A 503/429 with a parseable but non-transient error code is NOT retried. + * Unparseable 429/503 responses fall back to status-code detection. + * + * @param int $statusCode HTTP response status code + * @param string $body Response body + * @return bool + */ + protected function isTransientError(int $statusCode, string $body): bool + { + $trimmed = \ltrim($body); + if (\str_starts_with($trimmed, 'Code ?? ''); + if (\in_array($code, ['SlowDown', 'ServiceUnavailable', 'Throttling', 'RequestThrottled'], true)) { + return true; + } + // Successfully parsed XML with a non-transient error code — do not retry. + if ($code !== '') { + return false; + } + } + } + + // Fall back to HTTP status code for responses that cannot be parsed as XML. + return $statusCode === 429 || $statusCode === 503; + } + /** * Sort compare for meta headers * diff --git a/tests/Storage/Device/S3SlowDownTest.php b/tests/Storage/Device/S3SlowDownTest.php new file mode 100644 index 00000000..634c5b1f --- /dev/null +++ b/tests/Storage/Device/S3SlowDownTest.php @@ -0,0 +1,71 @@ +isTransientError($statusCode, $body); + } +} + +class S3SlowDownTest extends TestCase +{ + private TestableS3 $s3; + + protected function setUp(): void + { + $this->s3 = new TestableS3( + root: '/root', + accessKey: 'test-key', + secretKey: 'test-secret', + host: 'https://s3.example.com', + region: 'us-east-1', + ); + } + + protected function tearDown(): void + { + S3::setRetryAttempts(3); + S3::setRetryDelay(500); + } + + public function testTransientXmlErrorIsRetried(): void + { + $body = 'SlowDownPlease reduce your request rate.'; + $this->assertTrue($this->s3->exposedIsTransientError(503, $body)); + } + + public function testNonTransientXmlErrorIsNotRetried(): void + { + $body = 'NoSuchKeyThe specified key does not exist.'; + $this->assertFalse($this->s3->exposedIsTransientError(404, $body)); + } + + public function testStatusFallbackIsTransient(): void + { + $this->assertTrue($this->s3->exposedIsTransientError(429, '')); + $this->assertTrue($this->s3->exposedIsTransientError(503, '')); + } + + /** XML error code takes precedence over HTTP status — 503 with non-transient XML must not be retried. */ + public function test503WithNonTransientXmlIsNotRetried(): void + { + $body = 'InternalErrorInternal server error.'; + $this->assertFalse($this->s3->exposedIsTransientError(503, $body)); + } + + public function testDefaultRetrySettings(): void + { + $prop = fn (string $name) => (new \ReflectionProperty(S3::class, $name))->getValue(); + $this->assertSame(3, $prop('retryAttempts')); + $this->assertSame(500, $prop('retryDelay')); + } +}