Fix s3 cp returning exit code 0 on upload failure

.changes/next-release/s3-cp-exit-code.json

@@ -0,0 +1,7 @@
+[
+  {
+    "type": "bugfix",
+    "category": "s3",
+    "description": "Fix s3 cp returning exit code 0 when upload fails with ReadTimeoutError"
+  }
+]

awscli/customizations/s3/results.py

@@ -150,11 +150,17 @@ def _on_failure(self, future, e):
 
     def _is_existing_object_skipped(self, future, exception):
         # --no-overwrite sends IfNoneMatch, which fails if the object exists.
+        response = getattr(exception, 'response', None)
+        if not isinstance(response, dict):
+            return False
+        error = response.get('Error', {})
+        if not isinstance(error, dict):
+            return False
+        call_args = getattr(getattr(future, 'meta', None), 'call_args', None)
+        extra_args = getattr(call_args, 'extra_args', {}) or {}
         return (
-            hasattr(exception, 'response')
-            and exception.response.get('Error', {}).get('Code')
-            == 'PreconditionFailed'
-            and 'IfNoneMatch' in future.meta.call_args.extra_args
+            error.get('Code') == 'PreconditionFailed'
+            and 'IfNoneMatch' in extra_args
         )
 
 

tests/functional/s3/test_cp_command.py

@@ -21,6 +21,7 @@
     S3RequestType,
     S3ResponseError,
 )
+from botocore.exceptions import ReadTimeoutError
 from botocore.response import StreamingBody
 
 from awscli.compat import BytesIO, OrderedDict
@@ -32,6 +33,7 @@
     mock,
     skip_if_windows,
 )
+from tests import BaseResponse
 from tests.functional.s3 import (
     BaseCRTTransferClientTest,
     BaseS3CLIRunnerTest,
@@ -3986,3 +3988,21 @@ def test_s3_express_warn_emits_warning(self):
 
         _, stderr, _ = self.run_cmd(cmd, expected_rc=0)
         assert "warning: Recursive copies/moves" in stderr
+
+
+class TestCpFailureExitCode(BaseS3CLIRunnerTest):
+    def test_upload_fails_with_read_timeout_error_returns_exit_code_1(self):
+        class ReadTimeoutResponse(BaseResponse):
+            def on_http_request_sent(self, request):
+                raise ReadTimeoutError(endpoint_url='https://s3.amazonaws.com')
+
+        self.cli_runner.env['AWS_MAX_ATTEMPTS'] = '1'
+        self.session_stubber.add_response(ReadTimeoutResponse())
+        filename = os.path.join(self.config_files.rootdir, 'test.txt')
+        with open(filename, 'w') as f:
+            f.write('hello')
+        cmdline = ['s3', 'cp', filename, 's3://bucket/key']
+        result = self.cli_runner.run(cmdline)
+        self.assertEqual(result.rc, 1)
+        self.assertIn('upload failed', result.stderr)
+        self.assertIn('Read timeout on endpoint URL', result.stderr)

tests/unit/customizations/s3/test_results.py

@@ -13,7 +13,7 @@
 
 import time
 
-from botocore.exceptions import HTTPClientError
+from botocore.exceptions import HTTPClientError, ReadTimeoutError
 from s3transfer.exceptions import CancelledError, FatalError
 
 from awscli.compat import StringIO, queue
@@ -213,6 +213,45 @@ def test_on_done_precondition_failed_without_if_none_match(self):
             ),
         )
 
+    def test_on_done_failure_with_none_response(self):
+        subscriber = self.get_result_subscriber(DoneResultSubscriber)
+        read_timeout_error = ReadTimeoutError(
+            endpoint_url='https://s3.amazonaws.com'
+        )
+        failure_future = self.get_failed_transfer_future(read_timeout_error)
+        subscriber.on_done(failure_future)
+        result = self.get_queued_result()
+        self.assert_result_queue_is_empty()
+        self.assertEqual(
+            result,
+            FailureResult(
+                transfer_type=self.transfer_type,
+                src=self.src,
+                dest=self.dest,
+                exception=read_timeout_error,
+            ),
+        )
+
+    def test_on_done_failure_with_malformed_error_field(self):
+        subscriber = self.get_result_subscriber(DoneResultSubscriber)
+        malformed_error = HTTPClientError(
+            request=mock.Mock(),
+            response={'Error': 'not-a-dict'},
+            error='not-a-dict',
+        )
+        failure_future = self.get_failed_transfer_future(malformed_error)
+        subscriber.on_done(failure_future)
+        result = self.get_queued_result()
+        self.assert_result_queue_is_empty()
+        self.assertEqual(
+            result,
+            FailureResult(
+                transfer_type=self.transfer_type,
+                src=self.src,
+                dest=self.dest,
+                exception=malformed_error,
+            ),
+        )
 
     def test_on_done_unexpected_cancelled(self):
         subscriber = self.get_result_subscriber(DoneResultSubscriber)