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/errorhandler.py b/awscli/errorhandler.py index 5453d1194deb..f22fbf671df6 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(): @@ -200,6 +204,20 @@ def _display_structured_error( ): try: error_format = self._resolve_error_format(parsed_globals) + modeled_fields = error_info.pop('_modeled_fields', None) + + if modeled_fields is not None: + modeled_lower = {f.lower() for f in modeled_fields} + for key in list(error_info.keys()): + if key.lower() not in modeled_lower: + del error_info[key] + else: + # No model info available (e.g. manually constructed + # ClientError). Remove all non-standard fields. + standard = {'code', 'message'} + for key in list(error_info.keys()): + if key.lower() not in standard: + del error_info[key] if error_format == 'legacy': return False @@ -275,7 +293,10 @@ def _get_formatted_message(self, error_info, exception): def _extract_error_info(self, exception): error_response = self._extract_error_response(exception) if error_response and 'Error' in error_response: - return error_response['Error'] + error_info = error_response['Error'] + modeled_fields = getattr(exception, 'modeled_fields', None) + error_info['_modeled_fields'] = modeled_fields + return error_info return None @staticmethod 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()