diff --git a/.changes/next-release/feature-Output-59989.json b/.changes/next-release/feature-Output-59989.json new file mode 100644 index 000000000000..520f437b86ce --- /dev/null +++ b/.changes/next-release/feature-Output-59989.json @@ -0,0 +1,5 @@ +{ + "type": "feature", + "category": "Output", + "description": "AWS service errors containing modeled fields beyond Code and Message now write structured output to stdout in the configured output format. Set cli_error_format=LEGACY to disable or use --output off to suppress stdout." +} diff --git a/awscli/clidriver.py b/awscli/clidriver.py index ae8a3af68845..109118f631e1 100644 --- a/awscli/clidriver.py +++ b/awscli/clidriver.py @@ -22,6 +22,7 @@ import distro from botocore import xform_name from botocore.compat import OrderedDict, copy_kwargs +from botocore.exceptions import ClientError from botocore.configprovider import ( ChainProvider, ConstantProvider, @@ -76,6 +77,7 @@ ) from awscli.plugin import load_plugins from awscli.telemetry import add_session_id_component_to_user_agent_extra +from awscli.structured_error import StructuredErrorHandler from awscli.utils import ( IMDSRegionProvider, OutputStreamFactory, @@ -275,6 +277,9 @@ def _update_config_chain(self): config_store.set_config_provider( 'cli_help_output', self._construct_cli_help_output_chain() ) + config_store.set_config_provider( + 'cli_error_format', self._construct_cli_error_format_chain() + ) def _construct_cli_region_chain(self): providers = [ @@ -368,6 +373,20 @@ def _construct_cli_auto_prompt_chain(self): ] return ChainProvider(providers=providers) + def _construct_cli_error_format_chain(self): + providers = [ + EnvironmentProvider( + name='AWS_CLI_ERROR_FORMAT', + env=os.environ, + ), + ScopedConfigProvider( + config_var_name='cli_error_format', + session=self.session, + ), + ConstantProvider(value='STANDARD'), + ] + return ChainProvider(providers=providers) + @property def subcommand_table(self): return self._get_command_table() @@ -983,6 +1002,9 @@ class CLIOperationCaller: def __init__(self, session): self._session = session self._output_stream_factory = OutputStreamFactory(session) + self._structured_error_handler = StructuredErrorHandler( + session, self._output_stream_factory + ) def invoke(self, service_name, operation_name, parameters, parsed_globals): """Invoke an operation and format the response. @@ -1023,15 +1045,44 @@ def invoke(self, service_name, operation_name, parameters, parsed_globals): def _make_client_call( self, client, operation_name, parameters, parsed_globals ): - py_operation_name = xform_name(operation_name) - if client.can_paginate(py_operation_name) and parsed_globals.paginate: - paginator = client.get_paginator(py_operation_name) - response = paginator.paginate(**parameters) - else: - response = getattr(client, xform_name(operation_name))( - **parameters + try: + py_operation_name = xform_name(operation_name) + if ( + client.can_paginate(py_operation_name) + and parsed_globals.paginate + ): + paginator = client.get_paginator(py_operation_name) + response = paginator.paginate(**parameters) + else: + response = getattr(client, py_operation_name)(**parameters) + return response + except ClientError as e: + # Display structured error output before re-raising + self._display_structured_error_for_exception( + e, parsed_globals + ) + raise + + def _display_structured_error_for_exception( + self, exception, parsed_globals + ): + try: + error_response = ( + self._structured_error_handler.extract_error_response( + exception + ) + ) + if error_response: + self._structured_error_handler.handle_error( + error_response, parsed_globals + ) + except Exception as e: + # Don't let structured error display break error handling + LOG.debug( + 'Failed to display structured error: %s', + e, + exc_info=True, ) - return response def _display_response(self, command_name, response, parsed_globals): output = parsed_globals.output @@ -1039,5 +1090,9 @@ def _display_response(self, command_name, response, parsed_globals): output = self._session.get_config_variable('output') formatter = get_formatter(output, parsed_globals) - with self._output_stream_factory.get_output_stream() as stream: - formatter(command_name, response, stream) + try: + with self._output_stream_factory.get_output_stream() as stream: + formatter(command_name, response, stream) + except ClientError as e: + self._display_structured_error_for_exception(e, parsed_globals) + raise diff --git a/awscli/data/cli.json b/awscli/data/cli.json index 27d32410393f..7183b5219e4f 100644 --- a/awscli/data/cli.json +++ b/awscli/data/cli.json @@ -26,7 +26,8 @@ "text", "table", "yaml", - "yaml-stream" + "yaml-stream", + "off" ], "help": "

The formatting style for command output.

" }, diff --git a/awscli/examples/global_options.rst b/awscli/examples/global_options.rst index cec1f5529736..e8ba11090991 100644 --- a/awscli/examples/global_options.rst +++ b/awscli/examples/global_options.rst @@ -29,6 +29,8 @@ * yaml-stream + * off + ``--query`` (string) diff --git a/awscli/formatter.py b/awscli/formatter.py index 60d80d8db0c6..0f0f7f002611 100644 --- a/awscli/formatter.py +++ b/awscli/formatter.py @@ -227,9 +227,9 @@ def _format_response(self, command_name, response, stream): try: self.table.render(stream) except OSError: - # If they're piping stdout to another process which exits before - # we're done writing all of our output, we'll get an error about a - # closed pipe which we can safely ignore. + # If they're piping stdout to another process which exits + # before we're done writing all of our output, we'll get an + # error about a closed pipe which we can safely ignore. pass def _build_table(self, title, current, indent_level=0): @@ -368,12 +368,23 @@ def _format_response(self, response, stream): text.format_text(response, stream) +class OffFormatter(Formatter): + """Formatter that suppresses all output. + Only stdout is suppressed; stderr (error messages) remains visible. + """ + + def __call__(self, command_name, response, stream=None): + # Suppress all output + pass + + CLI_OUTPUT_FORMATS = { 'json': JSONFormatter, 'text': TextFormatter, 'table': TableFormatter, 'yaml': YAMLFormatter, 'yaml-stream': StreamedYAMLFormatter, + 'off': OffFormatter, } diff --git a/awscli/structured_error.py b/awscli/structured_error.py new file mode 100644 index 000000000000..d7897cfa11a3 --- /dev/null +++ b/awscli/structured_error.py @@ -0,0 +1,94 @@ +# Copyright 2012-2013 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 logging + +from botocore.exceptions import ClientError + +from awscli.formatter import get_formatter + + +LOG = logging.getLogger('awscli.structured_error') + + +class StructuredErrorHandler: + """Handles display of structured error information from AWS services. + + This class is responsible for determining when to display structured + error output and formatting it appropriately based on user configuration + and output format settings. + """ + + def __init__(self, session, output_stream_factory): + self._session = session + self._output_stream_factory = output_stream_factory + + def handle_error(self, error_response, parsed_globals): + error_info = error_response.get('Error', {}) + + if self.should_display(error_info, parsed_globals): + self.display(error_info, parsed_globals) + + def should_display(self, error_info, parsed_globals): + if not self._has_additional_error_members(error_info): + return False + + config_store = self._session.get_component('config_store') + error_format = config_store.get_config_variable('cli_error_format') + if error_format == 'LEGACY': + return False + + output = self._get_output_format(parsed_globals) + if output == 'off': + return False + + return True + + def display(self, error_response, parsed_globals): + output = self._get_output_format(parsed_globals) + + try: + formatter = get_formatter(output, parsed_globals) + + with self._output_stream_factory.get_output_stream() as stream: + formatter('error', error_response, stream) + except Exception as e: + # Log the error but don't let it prevent the normal error handling + LOG.debug( + "Failed to display structured error output: %s", + e, + exc_info=True, + ) + + def _get_output_format(self, parsed_globals): + output = parsed_globals.output + if output is None: + output = self._session.get_config_variable('output') + return output + + def _has_additional_error_members(self, error_response): + if not error_response: + return False + + standard_keys = {'Code', 'Message'} + error_keys = set(error_response.keys()) + return len(error_keys - standard_keys) > 0 + + @staticmethod + def extract_error_response(exception): + if not isinstance(exception, ClientError): + return None + + if hasattr(exception, 'response') and 'Error' in exception.response: + return {'Error': exception.response['Error']} + + return None diff --git a/tests/functional/autocomplete/test_completer.py b/tests/functional/autocomplete/test_completer.py index 0e9e5dbc2a4c..568d38566509 100644 --- a/tests/functional/autocomplete/test_completer.py +++ b/tests/functional/autocomplete/test_completer.py @@ -483,7 +483,7 @@ def test_return_suggestions_for_global_arg_with_choices(self): suggestions = self.completer.complete(parsed) names = [s.name for s in suggestions] self.assertEqual( - names, ['json', 'text', 'table', 'yaml', 'yaml-stream'] + names, ['json', 'text', 'table', 'yaml', 'yaml-stream', 'off'] ) def test_not_return_suggestions_for_global_arg_wo_trailing_space(self): diff --git a/tests/unit/test_formatter.py b/tests/unit/test_formatter.py index db24f1946754..f4f35a5677c5 100644 --- a/tests/unit/test_formatter.py +++ b/tests/unit/test_formatter.py @@ -19,7 +19,12 @@ from botocore.paginate import PageIterator from awscli.compat import StringIO, contextlib -from awscli.formatter import JSONFormatter, StreamedYAMLFormatter, YAMLDumper +from awscli.formatter import ( + JSONFormatter, + OffFormatter, + StreamedYAMLFormatter, + YAMLDumper, +) from awscli.testutils import mock, unittest @@ -180,3 +185,44 @@ def test_encoding_override(self, env_vars): '}\n' ).encode() ) + + +class TestOffFormatter: + def setup_method(self): + self.args = Namespace(query=None) + self.formatter = OffFormatter(self.args) + self.output = StringIO() + + def test_suppresses_simple_response(self): + response = {'Key': 'Value'} + self.formatter('test-command', response, self.output) + assert self.output.getvalue() == '' + + def test_suppresses_complex_response(self): + response = { + 'Items': [ + {'Name': 'Item1', 'Value': 'data'}, + {'Name': 'Item2', 'Value': 'more-data'} + ], + 'Count': 2 + } + self.formatter('test-command', response, self.output) + assert self.output.getvalue() == '' + + def test_suppresses_empty_response(self): + response = {} + self.formatter('test-command', response, self.output) + assert self.output.getvalue() == '' + + def test_suppresses_paginated_response(self): + response = FakePageIterator([ + {'Items': ['Item1']}, + {'Items': ['Item2']} + ]) + self.formatter('test-command', response, self.output) + assert self.output.getvalue() == '' + + def test_works_without_stream(self): + response = {'Key': 'Value'} + # Should not raise an exception + self.formatter('test-command', response, None) diff --git a/tests/unit/test_structured_error.py b/tests/unit/test_structured_error.py new file mode 100644 index 000000000000..4ed610ba9640 --- /dev/null +++ b/tests/unit/test_structured_error.py @@ -0,0 +1,234 @@ +# Copyright 2012-2013 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 io +from unittest import mock + +from botocore.exceptions import ClientError + +from awscli.structured_error import StructuredErrorHandler +from tests.unit.test_clidriver import FakeSession +from awscli.utils import OutputStreamFactory +from awscli.clidriver import CLIOperationCaller + + +class TestStructuredErrorHandler: + def setup_method(self): + self.session = FakeSession() + + self.output_stream_factory = OutputStreamFactory(self.session) + self.handler = StructuredErrorHandler( + self.session, self.output_stream_factory + ) + + def test_extract_error_response_from_client_error(self): + error_response = { + 'Error': { + 'Code': 'NoSuchBucket', + 'Message': 'The specified bucket does not exist', + 'BucketName': 'my-bucket', + }, + 'ResponseMetadata': {'RequestId': '123'}, + } + client_error = ClientError(error_response, 'GetObject') + + result = StructuredErrorHandler.extract_error_response(client_error) + + assert result is not None + assert 'Error' in result + assert result['Error']['Code'] == 'NoSuchBucket' + assert result['Error']['BucketName'] == 'my-bucket' + + def test_extract_error_response_from_non_client_error(self): + result = StructuredErrorHandler.extract_error_response( + ValueError('Some error') + ) + assert result is None + + def test_has_additional_error_members(self): + assert self.handler._has_additional_error_members( + {'Code': 'NoSuchBucket', 'Message': 'Error', 'BucketName': 'test'} + ) + + assert not self.handler._has_additional_error_members( + {'Code': 'AccessDenied', 'Message': 'Access Denied'} + ) + + assert not self.handler._has_additional_error_members({}) + assert not self.handler._has_additional_error_members(None) + + def test_should_display_with_additional_members(self): + error_response = { + 'Code': 'NoSuchBucket', + 'Message': 'Error', + 'BucketName': 'my-bucket', + } + parsed_globals = mock.Mock() + parsed_globals.output = 'json' + + assert self.handler.should_display(error_response, parsed_globals) + + def test_should_display_without_additional_members(self): + error_response = {'Code': 'AccessDenied', 'Message': 'Access Denied'} + parsed_globals = mock.Mock() + parsed_globals.output = 'json' + + assert not self.handler.should_display(error_response, parsed_globals) + + def test_should_display_respects_legacy_format(self): + error_response = { + 'Code': 'NoSuchBucket', + 'Message': 'Error', + 'BucketName': 'test', + } + parsed_globals = mock.Mock() + parsed_globals.output = 'json' + + self.session.config_store.set_config_provider( + 'cli_error_format', mock.Mock(provide=lambda: 'LEGACY') + ) + + assert not self.handler.should_display(error_response, parsed_globals) + + def test_should_display_respects_output_off(self): + error_response = { + 'Code': 'NoSuchBucket', + 'Message': 'Error', + 'BucketName': 'test', + } + parsed_globals = mock.Mock() + parsed_globals.output = 'off' + + assert not self.handler.should_display(error_response, parsed_globals) + + def test_display_json_format(self): + error_response = { + 'Code': 'NoSuchBucket', + 'Message': 'The specified bucket does not exist', + 'BucketName': 'my-bucket', + } + parsed_globals = mock.Mock() + parsed_globals.output = 'json' + parsed_globals.query = None + + mock_stream = io.StringIO() + mock_context_manager = mock.MagicMock() + mock_context_manager.__enter__.return_value = mock_stream + mock_context_manager.__exit__.return_value = False + + mock_stream_factory = mock.Mock() + mock_stream_factory.get_output_stream.return_value = ( + mock_context_manager + ) + self.handler._output_stream_factory = mock_stream_factory + + self.handler.display(error_response, parsed_globals) + + output = mock_stream.getvalue() + assert 'NoSuchBucket' in output + assert 'my-bucket' in output + + def test_display_handles_exceptions_gracefully(self): + error_response = {'Code': 'SomeError', 'Message': 'An error occurred'} + parsed_globals = mock.Mock() + parsed_globals.output = 'json' + + mock_context_manager = mock.MagicMock() + mock_context_manager.__enter__.side_effect = Exception('Stream error') + + mock_stream_factory = mock.Mock() + mock_stream_factory.get_output_stream.return_value = ( + mock_context_manager + ) + self.handler._output_stream_factory = mock_stream_factory + + self.handler.display(error_response, parsed_globals) + + def test_should_display_with_parsed_globals_output_none(self): + error_response = { + 'Code': 'NoSuchBucket', + 'Message': 'Error', + 'BucketName': 'test', + } + parsed_globals = mock.Mock() + parsed_globals.output = None + + with mock.patch.object( + self.session, 'get_config_variable', return_value='off' + ): + assert not self.handler.should_display( + error_response, parsed_globals + ) + + def test_should_display_with_parsed_globals_output_none_json(self): + error_response = { + 'Code': 'NoSuchBucket', + 'Message': 'Error', + 'BucketName': 'test', + } + parsed_globals = mock.Mock() + parsed_globals.output = None + + with mock.patch.object( + self.session, 'get_config_variable', return_value='json' + ): + assert self.handler.should_display(error_response, parsed_globals) + + +class TestStructuredErrorWithPagination: + def setup_method(self): + self.session = FakeSession() + self.caller = CLIOperationCaller(self.session) + + def test_formatter_error_displays_structured_error(self): + error_response = { + 'Error': { + 'Code': 'AccessDenied', + 'Message': 'Access Denied', + 'BucketName': 'my-bucket', + }, + 'ResponseMetadata': {'RequestId': '123'}, + } + + client_error = ClientError(error_response, 'ListObjects') + + parsed_globals = mock.Mock() + parsed_globals.output = 'json' + parsed_globals.query = None + + mock_formatter = mock.Mock() + mock_formatter.side_effect = client_error + + mock_stream = io.StringIO() + mock_context_manager = mock.MagicMock() + mock_context_manager.__enter__.return_value = mock_stream + mock_context_manager.__exit__.return_value = False + + with mock.patch( + 'awscli.clidriver.get_formatter', return_value=mock_formatter + ): + with mock.patch.object( + self.caller._output_stream_factory, + 'get_output_stream', + return_value=mock_context_manager, + ): + try: + self.caller._display_response( + 'list-objects', {}, parsed_globals + ) + assert False, "Expected ClientError to be raised" + except ClientError: + pass + + output = mock_stream.getvalue() + assert 'AccessDenied' in output + assert 'my-bucket' in output