Ensure all networking errors are retried on idempotent s3 commands

casualuser · Sep 23, 2015 · f439966 · f439966
1 parent 98c4130
commit f439966
Show file tree

Hide file tree

Showing 2 changed files with 48 additions and 18 deletions.
diff --git a/src/S3/S3Client.php b/src/S3/S3Client.php
@@ -489,15 +489,11 @@ public static function _applyRetryConfig($value, $_, HandlerList $list)
                         $error->getResponse()->getBody(),
                         'Your socket connection to the server'
                     ) !== false;
-                } elseif ($error->getPrevious() instanceof RequestException
-                    && defined('CURLE_RECV_ERROR')
-                ) {
-                    $context = $error->getPrevious()->getHandlerContext();
-                    if (isset($context['errno'])
-                        && $context['errno'] === CURLE_RECV_ERROR
-                    ) {
-                        return true;
-                    }
+                } elseif ($error->getPrevious() instanceof RequestException) {
+                    // All commands except CompleteMultipartUpload are
+                    // idempotent and may be retried without worry if a
+                    // networking error has occurred.
+                    return $command->getName() !== 'CompleteMultipartUpload';
                 }
             }
             return false;

diff --git a/tests/S3/S3ClientTest.php b/tests/S3/S3ClientTest.php
@@ -542,29 +542,26 @@ private function getSocketTimeoutResponse()
 EOXML;
     }
 
-    public function testConnectionResetErrorsAreRetried()
+    public function testNetworkingErrorsAreRetriedOnIdempotentCommands()
     {
-        $resetError = $this->getMockBuilder(RequestException::class)
+        $networkingError = $this->getMockBuilder(RequestException::class)
             ->disableOriginalConstructor()
-            ->setMethods(['getHandlerContext'])
+            ->setMethods([])
             ->getMock();
-        $resetError->expects($this->any())
-            ->method('getHandlerContext')
-            ->willReturn(['errno' => CURLE_RECV_ERROR]);
 
         $retries = 11;
         $client = new S3Client([
             'version' => 'latest',
             'region' => 'us-west-2',
             'retries' => $retries,
-            'http_handler' => function () use (&$retries, $resetError) {
+            'http_handler' => function () use (&$retries, $networkingError) {
                 if (0 === --$retries) {
                     return new FulfilledPromise(new Response);
                 }
 
                 return new RejectedPromise([
                     'connection_error' => false,
-                    'exception' => $resetError,
+                    'exception' => $networkingError,
                     'response' => null,
                 ]);
             },
@@ -573,7 +570,44 @@ public function testConnectionResetErrorsAreRetried()
         $client->putObject([
             'Bucket' => 'bucket',
             'Key' => 'key',
-            'Body' => Psr7\stream_for('x'),
+        ]);
+
+        $this->assertEquals(0, $retries);
+    }
+
+    /**
+     * @expectedException \Aws\S3\Exception\S3Exception
+     * @expectedExceptionMessageRegExp /CompleteMultipartUpload/
+     */
+    public function testNetworkingErrorsAreNotRetriedOnNonIdempotentCommands()
+    {
+        $networkingError = $this->getMockBuilder(RequestException::class)
+            ->disableOriginalConstructor()
+            ->setMethods([])
+            ->getMock();
+
+        $retries = 11;
+        $client = new S3Client([
+            'version' => 'latest',
+            'region' => 'us-west-2',
+            'retries' => $retries,
+            'http_handler' => function () use (&$retries, $networkingError) {
+                if (0 === --$retries) {
+                    return new FulfilledPromise(new Response);
+                }
+
+                return new RejectedPromise([
+                    'connection_error' => false,
+                    'exception' => $networkingError,
+                    'response' => null,
+                ]);
+            },
+        ]);
+
+        $client->completeMultipartUpload([
+            'Bucket' => 'bucket',
+            'Key' => 'key',
+            'UploadId' => 1,
         ]);
 
         $this->assertEquals(0, $retries);