Merge pull request #1191 from kyleknap/s3-readtimeout

Add retry logic for read timeouts for s3 commands

CHANGELOG.rst

@@ -5,6 +5,8 @@ CHANGELOG
 Next Release (TBD)
 ==================
 
+* bugfix:``aws s3``: Fix issue where read timeouts were not retried.
+  (`issue 1191 <https://github.com/aws/aws-cli/pull/1191>`__)
 * feature:``aws cloudtrail``: Add support for regionalized policy templates
   for the ``create-subscription`` and ``update-subscription`` commands.
   (`issue 1167 <https://github.com/aws/aws-cli/pull/1167>`__)

awscli/customizations/s3/tasks.py

@@ -7,6 +7,8 @@
 
 from botocore.vendored import requests
 from botocore.exceptions import IncompleteReadError
+from botocore.vendored.requests.packages.urllib3.exceptions import \
+    ReadTimeoutError
 
 from awscli.customizations.s3.utils import find_bucket_key, MD5Error, \
     operate, ReadFileChunk, relative_path, IORequest, IOCloseRequest, \
@@ -383,8 +385,8 @@ def _download_part(self):
                 self._result_queue.put(PrintTask(**result))
                 LOGGER.debug("Task complete: %s", self)
                 return
-            except (socket.timeout, socket.error) as e:
-                LOGGER.debug("Socket timeout caught, retrying request, "
+            except (socket.timeout, socket.error, ReadTimeoutError) as e:
+                LOGGER.debug("Timeout error caught, retrying request, "
                              "(attempt %s / %s)", i, self.TOTAL_ATTEMPTS,
                              exc_info=True)
                 continue

tests/unit/customizations/s3/test_tasks.py

@@ -17,6 +17,8 @@
 import socket
 
 from botocore.exceptions import IncompleteReadError
+from botocore.vendored.requests.packages.urllib3.exceptions import \
+    ReadTimeoutError
 
 from awscli.customizations.s3 import transferconfig
 from awscli.customizations.s3.tasks import CreateLocalFileTask
@@ -396,6 +398,18 @@ def test_incomplete_read_is_retried(self):
         self.assertEqual(DownloadPartTask.TOTAL_ATTEMPTS,
                          self.service.get_operation.call_count)
 
+    def test_readtimeout_is_retried(self):
+        self.service.get_operation.return_value.call.side_effect = \
+            ReadTimeoutError(None, None, None)
+        task = DownloadPartTask(0, 1024 * 1024, self.result_queue,
+                                self.service, self.filename,
+                                self.context, self.io_queue)
+        with self.assertRaises(RetriesExeededError):
+            task()
+        self.context.cancel.assert_called_with()
+        self.assertEqual(DownloadPartTask.TOTAL_ATTEMPTS,
+                         self.service.get_operation.call_count)
+
     def test_retried_requests_dont_enqueue_writes_twice(self):
         error_body = mock.Mock()
         error_body.read.side_effect = socket.timeout