Repository: boto/botocore
Pull request #3054 (2023-11-11 05:50:22)

Fix Sqs ErrorType not being parsed properly
Customer reporting exception error type is not properly handled after the AwsJson-1.0 migration when calling SQS. 

For example, when calling 
```
import boto3
import botocore

sqs = boto3.client('sqs')

try:
    sqs.delete_queue( QueueUrl='foo-nonexistant-queue' )
except sqs.exceptions.QueueDoesNotExist as e:
    print("Successfully caught QueueDoesNotExist")
```
Message does not printed, but instead exception dump does.

This is because Client is listening to an ErrorCode to throw a modelled factory exception. In the above case, `self.client.exceptions.QueueDoesNotExist` `ErrorType` should be thrown. However, as a prerequisite of the AwsJson migration, we modified the `ErrorCode` to ensure a backwards compatibility. In the above case, `ErrorCode` should be `AWS.SimpleQueueService.NonExistentQueue`. 

This PR should fix this.

codecov-commenter: ## [Codecov](https://app.codecov.io/gh/boto/botocore/pull/3054?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boto) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Comparison is base [(`01dabf4`)](https://app.codecov.io/gh/boto/botocore/commit/01dabf4d3dd645dcd921939a057429f0647cd2ec?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boto) 93.46% compared to head [(`dec0412`)](https://app.codecov.io/gh/boto/botocore/pull/3054?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boto) 93.43%.

:exclamation: Your organization needs to install the [Codecov GitHub app](https://github.com/apps/codecov/installations/select_target) to enable full functionality.

<details><summary>Additional details and impacted files</summary>


```diff
@@             Coverage Diff             @@
##           develop    #3054      +/-   ##
===========================================
- Coverage    93.46%   93.43%   -0.03%     
===========================================
  Files           66       66              
  Lines        14007    14007              
===========================================
- Hits         13091    13087       -4     
- Misses         916      920       +4     
```


| [Files](https://app.codecov.io/gh/boto/botocore/pull/3054?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boto) | Coverage Δ | |
|---|---|---|
| [botocore/client.py](https://app.codecov.io/gh/boto/botocore/pull/3054?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boto#diff-Ym90b2NvcmUvY2xpZW50LnB5) | `97.68% <100.00%> (+<0.01%)` | :arrow_up: |
| [botocore/parsers.py](https://app.codecov.io/gh/boto/botocore/pull/3054?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boto#diff-Ym90b2NvcmUvcGFyc2Vycy5weQ==) | `98.61% <100.00%> (-0.01%)` | :arrow_down: |

... and [3 files with indirect coverage changes](https://app.codecov.io/gh/boto/botocore/pull/3054/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boto)


</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/boto/botocore/pull/3054?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boto).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boto).


Patch:
diff --git a/botocore/client.py b/botocore/client.py
index 5becb3788..8bd63f07a 100644
--- a/botocore/client.py
+++ b/botocore/client.py
@@ -975,7 +975,10 @@ class BaseClient:
         )
 
         if http.status_code >= 300:
-            error_code = parsed_response.get("Error", {}).get("Code")
+            error_info = parsed_response.get("Error", {})
+            error_code = error_info.get("QueryErrorCode") or error_info.get(
+                "Code"
+            )
             error_class = self.exceptions.from_code(error_code)
             raise error_class(parsed_response, operation_name)
         else:
diff --git a/botocore/parsers.py b/botocore/parsers.py
index 09e362b85..3905757c8 100644
--- a/botocore/parsers.py
+++ b/botocore/parsers.py
@@ -701,26 +701,37 @@ class BaseJSONParser(ResponseParser):
         # if the message did not contain an error code
         # include the response status code
         response_code = response.get('status_code')
-        # Error response may contain an x-amzn-query-error header for json
-        # we need to fetch the error code from this header in that case
-        query_error = headers.get('x-amzn-query-error', '')
-        query_error_components = query_error.split(';')
-        code = None
-        if len(query_error_components) == 2 and query_error_components[0]:
-            code = query_error_components[0]
-            error['Error']['Type'] = query_error_components[1]
-        if code is None:
-            code = body.get('__type', response_code and str(response_code))
+
+        code = body.get('__type', response_code and str(response_code))
         if code is not None:
             # code has a couple forms as well:
             # * "com.aws.dynamodb.vAPI#ProvisionedThroughputExceededException"
             # * "ResourceNotFoundException"
             if '#' in code:
                 code = code.rsplit('#', 1)[1]
+            if 'x-amzn-query-error' in headers:
+                code = self._do_query_compatible_error_parse(
+                    code, headers, error
+                )
             error['Error']['Code'] = code
         self._inject_response_metadata(error, response['headers'])
         return error
 
+    def _do_query_compatible_error_parse(self, code, headers, error):
+        """
+        Error response may contain an x-amzn-query-error header to translate
+        errors codes from former `query` services into `json`. We use this to
+        do our lookup in the errorfactory for modeled errors.
+        """
+        query_error = headers['x-amzn-query-error']
+        query_error_components = query_error.split(';')
+
+        if len(query_error_components) == 2 and query_error_components[0]:
+            error['Error']['QueryErrorCode'] = code
+            error['Error']['Type'] = query_error_components[1]
+            return query_error_components[0]
+        return code
+
     def _inject_response_metadata(self, parsed, headers):
         if 'x-amzn-requestid' in headers:
             parsed.setdefault('ResponseMetadata', {})['RequestId'] = headers[


Test patch:
diff --git a/tests/functional/test_sqs.py b/tests/functional/test_sqs.py
new file mode 100644
index 000000000..bde70c86e
--- /dev/null
+++ b/tests/functional/test_sqs.py
@@ -0,0 +1,48 @@
+# Copyright 2023 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"). You
+# may not use this file except in compliance with the License. A copy of
+# the License is located at
+#
+# http://aws.amazon.com/apache2.0/
+#
+# or in the "license" file accompanying this file. This file is
+# distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF
+# ANY KIND, either express or implied. See the License for the specific
+# language governing permissions and limitations under the License.
+
+import pytest
+
+from tests import BaseSessionTest, ClientHTTPStubber
+
+
+class BaseSQSOperationTest(BaseSessionTest):
+    def setUp(self):
+        super().setUp()
+        self.region = "us-west-2"
+        self.client = self.session.create_client("sqs", self.region)
+        self.http_stubber = ClientHTTPStubber(self.client)
+
+
+class SQSQueryCompatibleTest(BaseSQSOperationTest):
+    def test_query_compatible_error_parsing(self):
+        """When migrating SQS from the ``query`` protocol to ``json``,
+        we unintentionally moved from modeled errors to a general ``ClientError``.
+        This ensures we're not silently regressing that behavior.
+        """
+
+        error_body = (
+            b'{"__type":"com.amazonaws.sqs#QueueDoesNotExist",'
+            b'"message":"The specified queue does not exist."}'
+        )
+        error_headers = {
+            "x-amzn-query-error": "AWS.SimpleQueueService.NonExistentQueue;Sender",
+        }
+        with self.http_stubber as stub:
+            stub.add_response(
+                status=400, body=error_body, headers=error_headers
+            )
+            with pytest.raises(self.client.exceptions.QueueDoesNotExist):
+                self.client.delete_queue(
+                    QueueUrl="not-a-real-queue-botocore",
+                )
diff --git a/tests/unit/test_parsers.py b/tests/unit/test_parsers.py
index d5f55a29e..979a542f3 100644
--- a/tests/unit/test_parsers.py
+++ b/tests/unit/test_parsers.py
@@ -1133,6 +1133,9 @@ class TestParseErrorResponses(unittest.TestCase):
         self.assertEqual(
             parsed['Error']['Code'], 'AWS.SimpleQueueService.NonExistentQueue'
         )
+        self.assertEqual(
+            parsed['Error']['QueryErrorCode'], "ValidationException"
+        )
         self.assertEqual(parsed['Error']['Type'], 'Sender')
 
     def test_response_with_invalid_query_error_for_json_protocol(self):
@@ -1155,6 +1158,7 @@ class TestParseErrorResponses(unittest.TestCase):
         self.assertIn('Error', parsed)
         self.assertEqual(parsed['Error']['Message'], 'this is a message')
         self.assertEqual(parsed['Error']['Code'], 'ValidationException')
+        self.assertNotIn('QueryErrorCode', parsed['Error'])
         self.assertNotIn('Type', parsed['Error'])
 
     def test_response_with_incomplete_query_error_for_json_protocol(self):
@@ -1177,6 +1181,7 @@ class TestParseErrorResponses(unittest.TestCase):
         self.assertIn('Error', parsed)
         self.assertEqual(parsed['Error']['Message'], 'this is a message')
         self.assertEqual(parsed['Error']['Code'], 'ValidationException')
+        self.assertNotIn('QueryErrorCode', parsed['Error'])
         self.assertNotIn('Type', parsed['Error'])
 
     def test_response_with_empty_query_errors_for_json_protocol(self):
@@ -1199,6 +1204,7 @@ class TestParseErrorResponses(unittest.TestCase):
         self.assertIn('Error', parsed)
         self.assertEqual(parsed['Error']['Message'], 'this is a message')
         self.assertEqual(parsed['Error']['Code'], 'ValidationException')
+        self.assertNotIn('QueryErrorCode', parsed['Error'])
         self.assertNotIn('Type', parsed['Error'])
 
     def test_parse_error_response_for_query_protocol(self):