diff --git a/.changes/next-release/bugfix-errorformatting-83208.json b/.changes/next-release/bugfix-errorformatting-83208.json new file mode 100644 index 000000000000..7225afce4b5d --- /dev/null +++ b/.changes/next-release/bugfix-errorformatting-83208.json @@ -0,0 +1,5 @@ +{ + "type": "bugfix", + "category": "error formatting", + "description": "Error output now only displays modeled error fields in the 'Additional error details' section." +} diff --git a/awscli/botocore/client.py b/awscli/botocore/client.py index b04b3f5c70c5..ed78f75661af 100644 --- a/awscli/botocore/client.py +++ b/awscli/botocore/client.py @@ -903,8 +903,17 @@ def _make_api_call(self, operation_name, api_params): if http.status_code >= 300: error_code = parsed_response.get("Error", {}).get("Code") + # Lookup is a cached dict access; only runs on error path. + error_shape = self._service_model.shape_for_error_code( + error_code + ) + modeled_fields = {'Code', 'Message'} + if error_shape: + modeled_fields |= set(error_shape.members.keys()) error_class = self.exceptions.from_code(error_code) - raise error_class(parsed_response, operation_name) + error = error_class(parsed_response, operation_name) + error.modeled_fields = modeled_fields + raise error else: return parsed_response diff --git a/awscli/botocore/exceptions.py b/awscli/botocore/exceptions.py index 326074ed72db..83d409867bcc 100644 --- a/awscli/botocore/exceptions.py +++ b/awscli/botocore/exceptions.py @@ -524,6 +524,7 @@ def __init__(self, error_response, operation_name): super().__init__(msg) self.response = error_response self.operation_name = operation_name + self.modeled_fields = None def _get_retry_info(self, response): retry_info = '' diff --git a/awscli/errorhandler.py b/awscli/errorhandler.py index 5453d1194deb..0411f77972c3 100644 --- a/awscli/errorhandler.py +++ b/awscli/errorhandler.py @@ -95,8 +95,12 @@ def _format_inline(self, value): return str(value) def _get_additional_fields(self, error_info): - standard_keys = {'Code', 'Message'} - return {k: v for k, v in error_info.items() if k not in standard_keys} + standard_keys = {'code', 'message'} + return { + k: v + for k, v in error_info.items() + if k.lower() not in standard_keys + } def construct_entry_point_handlers_chain(): @@ -286,15 +290,23 @@ def _extract_error_response(exception): if hasattr(exception, 'response') and 'Error' in exception.response: error_dict = dict(exception.response['Error']) - # AWS services return modeled error fields - # at the top level of the error response, - # not nested under an Error key. Botocore preserves this structure. - # Include these fields to provide complete error information. - # Exclude response metadata and avoid duplicates. + modeled_fields = exception.modeled_fields + if modeled_fields is not None: + modeled_lower = {f.lower() for f in modeled_fields} + else: + modeled_lower = {'code', 'message'} + + # Only include fields present in the error shape model. excluded_keys = {'Error', 'ResponseMetadata', 'Code', 'Message'} for key, value in exception.response.items(): if key not in excluded_keys and key not in error_dict: - error_dict[key] = value + if key.lower() in modeled_lower: + error_dict[key] = value + + # Filter fields inside Error dict as well. + for key in list(error_dict.keys()): + if key.lower() not in modeled_lower: + del error_dict[key] return {'Error': error_dict} diff --git a/tests/unit/test_structured_error.py b/tests/unit/test_structured_error.py index edd1241c4674..6b113ca1b56c 100644 --- a/tests/unit/test_structured_error.py +++ b/tests/unit/test_structured_error.py @@ -53,6 +53,7 @@ def test_displays_structured_error_with_additional_members(self): 'ResponseMetadata': {'RequestId': '123'}, } client_error = ClientError(error_response, 'GetObject') + client_error.modeled_fields = {'Code', 'Message', 'BucketName'} stdout = io.StringIO() stderr = io.StringIO() @@ -129,6 +130,7 @@ def test_error_format_case_insensitive(self): 'ResponseMetadata': {'RequestId': '123'}, } client_error = ClientError(error_response, 'GetObject') + client_error.modeled_fields = {'Code', 'Message', 'BucketName'} self.session.config_store.set_config_provider( 'cli_error_format', mock.Mock(provide=lambda: 'Enhanced') @@ -150,6 +152,91 @@ def test_error_format_case_insensitive(self): ) assert stderr.getvalue() == expected + def test_modeled_fields_filters_unmodeled_from_display(self): + error_response = { + 'Error': { + 'Code': 'ExpiredToken', + 'Message': 'Token expired', + 'Token-0': 'AQoDYXdzEJr...sensitive...', + }, + 'ResponseMetadata': {'RequestId': '123'}, + } + client_error = ClientError(error_response, 'ListBuckets') + client_error.modeled_fields = {'Code', 'Message'} + + stdout = io.StringIO() + stderr = io.StringIO() + + rc = self.handler.handle_exception(client_error, stdout, stderr) + + assert rc == CLIENT_ERROR_RC + assert 'Token-0' not in stderr.getvalue() + assert 'sensitive' not in stderr.getvalue() + + def test_modeled_fields_not_leaked_in_json_format(self): + error_response = { + 'Error': { + 'Code': 'ExpiredToken', + 'Message': 'Token expired', + 'Token-0': 'AQoDYXdzEJr...sensitive...', + }, + 'ResponseMetadata': {'RequestId': '123'}, + } + client_error = ClientError(error_response, 'ListBuckets') + client_error.modeled_fields = {'Code', 'Message'} + + self.session.session_vars['cli_error_format'] = 'json' + + stdout = io.StringIO() + stderr = io.StringIO() + + self.handler.handle_exception(client_error, stdout, stderr) + + parsed = json.loads(stderr.getvalue()) + assert 'Token-0' not in parsed + assert '_modeled_fields' not in parsed + assert 'modeled_fields' not in parsed + + def test_no_modeled_fields_hides_additional_fields(self): + # ClientError without modeled_fields attribute (e.g. manually + # constructed in customizations) should not show additional fields. + error_response = { + 'Error': { + 'Code': 'CustomError', + 'Message': 'Something broke', + 'Detail': 'extra info', + }, + 'ResponseMetadata': {'RequestId': '123'}, + } + client_error = ClientError(error_response, 'DoThing') + + stdout = io.StringIO() + stderr = io.StringIO() + + self.handler.handle_exception(client_error, stdout, stderr) + + assert 'Detail' not in stderr.getvalue() + + def test_modeled_fields_case_insensitive_match(self): + # Model has 'ErrorCode' but response has 'errorCode' (or vice versa) + error_response = { + 'Error': { + 'Code': 'SomeError', + 'Message': 'msg', + 'errorcode': 'detail', + }, + 'ResponseMetadata': {'RequestId': '123'}, + } + client_error = ClientError(error_response, 'Op') + client_error.modeled_fields = {'Code', 'Message', 'ErrorCode'} + + stdout = io.StringIO() + stderr = io.StringIO() + + self.handler.handle_exception(client_error, stdout, stderr) + + assert 'errorcode: detail' in stderr.getvalue() + class TestEnhancedErrorFormatter: def setup_method(self): @@ -343,6 +430,33 @@ def test_format_error_with_unicode_and_special_chars(self): ) assert output == expected + def test_format_error_hides_unmodeled_fields(self): + # Unmodeled fields are now filtered before reaching the formatter. + # Formatter receives only modeled fields. + error_info = { + 'Code': 'ExpiredToken', + 'Message': 'Token expired', + } + + stream = io.StringIO() + self.formatter.format_error(error_info, stream) + + assert stream.getvalue() == '' + + def test_format_error_shows_modeled_fields(self): + # Unmodeled fields are filtered before reaching the formatter. + error_info = { + 'Code': 'FileSystemNotFound', + 'Message': 'Not found', + 'ErrorCode': 'FileSystemNotFound', + } + + stream = io.StringIO() + self.formatter.format_error(error_info, stream) + + output = stream.getvalue() + assert 'ErrorCode: FileSystemNotFound' in output + def test_format_error_with_large_list(self): error_info = { 'Code': 'LargeList', @@ -398,6 +512,11 @@ def test_dynamodb_transaction_cancelled_error(self): }, } client_error = ClientError(error_response, 'TransactWriteItems') + client_error.modeled_fields = { + 'Code', + 'Message', + 'CancellationReasons', + } stdout = io.StringIO() stderr = io.StringIO() @@ -441,6 +560,7 @@ def test_error_handler_receives_parsed_globals_from_clidriver(self): 'ResponseMetadata': {'RequestId': '123'}, } client_error = ClientError(error_response, 'GetObject') + client_error.modeled_fields = {'Code', 'Message', 'BucketName'} stdout = io.StringIO() stderr = io.StringIO() @@ -471,6 +591,7 @@ def test_error_handler_without_parsed_globals_uses_default(self): 'ResponseMetadata': {'RequestId': '123'}, } client_error = ClientError(error_response, 'GetObject') + client_error.modeled_fields = {'Code', 'Message', 'BucketName'} stdout = io.StringIO() stderr = io.StringIO()