From 1afe452136b4e67162a2cd3684507a3392946ffd Mon Sep 17 00:00:00 2001 From: Andrew Asseily Date: Mon, 6 Oct 2025 11:19:26 -0400 Subject: [PATCH 01/26] Structured errors draft - failing tests --- awscli/clidriver.py | 21 +++++++++- awscli/data/cli.json | 11 +++++- awscli/errorhandler.py | 61 ++++++++++++++++++++++++++++- awscli/examples/global_options.rst | 12 ++++++ awscli/examples/global_synopsis.rst | 1 + tests/unit/test_clidriver.py | 5 +++ 6 files changed, 107 insertions(+), 4 deletions(-) diff --git a/awscli/clidriver.py b/awscli/clidriver.py index ae8a3af68845..bd2f8f824ae0 100644 --- a/awscli/clidriver.py +++ b/awscli/clidriver.py @@ -119,7 +119,7 @@ def create_clidriver(args=None): session.full_config.get('plugins', {}), event_hooks=session.get_component('event_emitter'), ) - error_handlers_chain = construct_cli_error_handlers_chain() + error_handlers_chain = construct_cli_error_handlers_chain(session=session) driver = CLIDriver( session=session, error_handler=error_handlers_chain, debug=debug ) @@ -275,6 +275,10 @@ 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 +372,20 @@ def _construct_cli_auto_prompt_chain(self): ] return ChainProvider(providers=providers) + def _construct_cli_error_format_chain(self): + providers = [ + InstanceVarProvider( + instance_var='cli_error_format', + session=self.session, + ), + 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() @@ -386,6 +404,7 @@ def _get_cli_data(self): # we load it here once. if self._cli_data is None: self._cli_data = self.session.get_data('cli') + print("CLI Data:", self._cli_data) return self._cli_data def _get_command_table(self): diff --git a/awscli/data/cli.json b/awscli/data/cli.json index 27d32410393f..d3ea070c2c53 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.

" }, @@ -85,6 +86,14 @@ "no-cli-auto-prompt": { "action": "store_true", "help": "

Disable automatically prompt for CLI input parameters.

" + }, + "cli-error-format": { + "choices": [ + "standard", + "legacy" + ], + "default": "standard", + "help": "

The formatting style for error output. 'standard' displays modeled error details, 'legacy' shows only the traditional error message.

" } } } diff --git a/awscli/errorhandler.py b/awscli/errorhandler.py index 954b005f8a0e..2d89ffa479de 100644 --- a/awscli/errorhandler.py +++ b/awscli/errorhandler.py @@ -10,6 +10,7 @@ # 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 json import logging import signal @@ -51,7 +52,7 @@ def construct_entry_point_handlers_chain(): return ChainedExceptionHandler(exception_handlers=handlers) -def construct_cli_error_handlers_chain(): +def construct_cli_error_handlers_chain(session=None): handlers = [ ParamValidationErrorsHandler(), UnknownArgumentErrorHandler(), @@ -60,7 +61,7 @@ def construct_cli_error_handlers_chain(): NoCredentialsErrorHandler(), PagerErrorHandler(), InterruptExceptionHandler(), - ClientErrorHandler(), + ClientErrorHandler(session=session), GeneralExceptionHandler(), ] return ChainedExceptionHandler(exception_handlers=handlers) @@ -108,6 +109,62 @@ class ClientErrorHandler(FilteredExceptionHandler): EXCEPTIONS_TO_HANDLE = ClientError RC = CLIENT_ERROR_RC + def __init__(self, session=None): + self._session = session + + def _do_handle_exception(self, exception, stdout, stderr): + if self._should_display_error_details(): + self._display_error_details(exception, stdout) + + stderr.write("\n") + stderr.write(str(exception)) + stderr.write("\n") + + return self.RC + + def _should_display_error_details(self): + if not self._session: + return True + + output_format = self._session.get_config_variable('output') + if output_format == 'off': + return False + + error_format = self._session.get_config_variable('cli_error_format') + return error_format != 'legacy' + + def _display_error_details(self, exception, stdout): + if not hasattr(exception, 'response') or not exception.response: + return + + error_details = exception.response.get('Error', {}) + if not error_details: + return + + output_format = 'json' + if self._session: + output_format = self._session.get_config_variable('output') or 'json' + + try: + if output_format == 'yaml': + self._display_yaml_error(error_details, stdout) + else: + json.dump(error_details, stdout, indent=4, ensure_ascii=False) + stdout.write('\n') + except (OSError, UnicodeError, TypeError) as e: + LOG.debug("Error formatting failed: %s", e) + + def _display_yaml_error(self, error_details, stdout): + try: + from ruamel.yaml import YAML + yaml = YAML(typ='safe') + yaml.encoding = None + yaml.representer.default_flow_style = False + yaml.dump(error_details, stdout) + except ImportError: + json.dump(error_details, stdout, indent=4, ensure_ascii=False) + stdout.write('\n') + class ConfigurationErrorHandler(FilteredExceptionHandler): EXCEPTIONS_TO_HANDLE = ConfigurationError diff --git a/awscli/examples/global_options.rst b/awscli/examples/global_options.rst index cec1f5529736..b673edff3539 100644 --- a/awscli/examples/global_options.rst +++ b/awscli/examples/global_options.rst @@ -29,6 +29,8 @@ * yaml-stream + * off + ``--query`` (string) @@ -96,3 +98,13 @@ Disable automatically prompt for CLI input parameters. +``--cli-error-format`` (string) + + The formatting style for error output. 'standard' displays modeled error details, 'legacy' shows only the traditional error message. + + + * standard + + * legacy + + diff --git a/awscli/examples/global_synopsis.rst b/awscli/examples/global_synopsis.rst index 1ca332c717ff..3e603348debb 100644 --- a/awscli/examples/global_synopsis.rst +++ b/awscli/examples/global_synopsis.rst @@ -16,3 +16,4 @@ [--no-cli-pager] [--cli-auto-prompt] [--no-cli-auto-prompt] +[--cli-error-format ] diff --git a/tests/unit/test_clidriver.py b/tests/unit/test_clidriver.py index 2140afae02f1..6214d07b1ce6 100644 --- a/tests/unit/test_clidriver.py +++ b/tests/unit/test_clidriver.py @@ -79,6 +79,11 @@ }, "read-timeout": {"type": "int", "help": ""}, "connect-timeout": {"type": "int", "help": ""}, + "cli-error-format": { + "choices": ["standard", "legacy"], + "default": "standard", + "help": "The formatting style for error output." + } }, }, } From 6842e4ef5809033db873b502676938473a78951c Mon Sep 17 00:00:00 2001 From: Andrew Asseily Date: Fri, 24 Oct 2025 14:38:42 -0400 Subject: [PATCH 02/26] Revert "Structured errors draft - failing tests" This reverts commit 1afe452136b4e67162a2cd3684507a3392946ffd. --- awscli/clidriver.py | 21 +--------- awscli/data/cli.json | 11 +----- awscli/errorhandler.py | 61 +---------------------------- awscli/examples/global_options.rst | 12 ------ awscli/examples/global_synopsis.rst | 1 - tests/unit/test_clidriver.py | 5 --- 6 files changed, 4 insertions(+), 107 deletions(-) diff --git a/awscli/clidriver.py b/awscli/clidriver.py index bd2f8f824ae0..ae8a3af68845 100644 --- a/awscli/clidriver.py +++ b/awscli/clidriver.py @@ -119,7 +119,7 @@ def create_clidriver(args=None): session.full_config.get('plugins', {}), event_hooks=session.get_component('event_emitter'), ) - error_handlers_chain = construct_cli_error_handlers_chain(session=session) + error_handlers_chain = construct_cli_error_handlers_chain() driver = CLIDriver( session=session, error_handler=error_handlers_chain, debug=debug ) @@ -275,10 +275,6 @@ 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 = [ @@ -372,20 +368,6 @@ def _construct_cli_auto_prompt_chain(self): ] return ChainProvider(providers=providers) - def _construct_cli_error_format_chain(self): - providers = [ - InstanceVarProvider( - instance_var='cli_error_format', - session=self.session, - ), - 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() @@ -404,7 +386,6 @@ def _get_cli_data(self): # we load it here once. if self._cli_data is None: self._cli_data = self.session.get_data('cli') - print("CLI Data:", self._cli_data) return self._cli_data def _get_command_table(self): diff --git a/awscli/data/cli.json b/awscli/data/cli.json index d3ea070c2c53..27d32410393f 100644 --- a/awscli/data/cli.json +++ b/awscli/data/cli.json @@ -26,8 +26,7 @@ "text", "table", "yaml", - "yaml-stream", - "off" + "yaml-stream" ], "help": "

The formatting style for command output.

" }, @@ -86,14 +85,6 @@ "no-cli-auto-prompt": { "action": "store_true", "help": "

Disable automatically prompt for CLI input parameters.

" - }, - "cli-error-format": { - "choices": [ - "standard", - "legacy" - ], - "default": "standard", - "help": "

The formatting style for error output. 'standard' displays modeled error details, 'legacy' shows only the traditional error message.

" } } } diff --git a/awscli/errorhandler.py b/awscli/errorhandler.py index 2d89ffa479de..954b005f8a0e 100644 --- a/awscli/errorhandler.py +++ b/awscli/errorhandler.py @@ -10,7 +10,6 @@ # 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 json import logging import signal @@ -52,7 +51,7 @@ def construct_entry_point_handlers_chain(): return ChainedExceptionHandler(exception_handlers=handlers) -def construct_cli_error_handlers_chain(session=None): +def construct_cli_error_handlers_chain(): handlers = [ ParamValidationErrorsHandler(), UnknownArgumentErrorHandler(), @@ -61,7 +60,7 @@ def construct_cli_error_handlers_chain(session=None): NoCredentialsErrorHandler(), PagerErrorHandler(), InterruptExceptionHandler(), - ClientErrorHandler(session=session), + ClientErrorHandler(), GeneralExceptionHandler(), ] return ChainedExceptionHandler(exception_handlers=handlers) @@ -109,62 +108,6 @@ class ClientErrorHandler(FilteredExceptionHandler): EXCEPTIONS_TO_HANDLE = ClientError RC = CLIENT_ERROR_RC - def __init__(self, session=None): - self._session = session - - def _do_handle_exception(self, exception, stdout, stderr): - if self._should_display_error_details(): - self._display_error_details(exception, stdout) - - stderr.write("\n") - stderr.write(str(exception)) - stderr.write("\n") - - return self.RC - - def _should_display_error_details(self): - if not self._session: - return True - - output_format = self._session.get_config_variable('output') - if output_format == 'off': - return False - - error_format = self._session.get_config_variable('cli_error_format') - return error_format != 'legacy' - - def _display_error_details(self, exception, stdout): - if not hasattr(exception, 'response') or not exception.response: - return - - error_details = exception.response.get('Error', {}) - if not error_details: - return - - output_format = 'json' - if self._session: - output_format = self._session.get_config_variable('output') or 'json' - - try: - if output_format == 'yaml': - self._display_yaml_error(error_details, stdout) - else: - json.dump(error_details, stdout, indent=4, ensure_ascii=False) - stdout.write('\n') - except (OSError, UnicodeError, TypeError) as e: - LOG.debug("Error formatting failed: %s", e) - - def _display_yaml_error(self, error_details, stdout): - try: - from ruamel.yaml import YAML - yaml = YAML(typ='safe') - yaml.encoding = None - yaml.representer.default_flow_style = False - yaml.dump(error_details, stdout) - except ImportError: - json.dump(error_details, stdout, indent=4, ensure_ascii=False) - stdout.write('\n') - class ConfigurationErrorHandler(FilteredExceptionHandler): EXCEPTIONS_TO_HANDLE = ConfigurationError diff --git a/awscli/examples/global_options.rst b/awscli/examples/global_options.rst index b673edff3539..cec1f5529736 100644 --- a/awscli/examples/global_options.rst +++ b/awscli/examples/global_options.rst @@ -29,8 +29,6 @@ * yaml-stream - * off - ``--query`` (string) @@ -98,13 +96,3 @@ Disable automatically prompt for CLI input parameters. -``--cli-error-format`` (string) - - The formatting style for error output. 'standard' displays modeled error details, 'legacy' shows only the traditional error message. - - - * standard - - * legacy - - diff --git a/awscli/examples/global_synopsis.rst b/awscli/examples/global_synopsis.rst index 3e603348debb..1ca332c717ff 100644 --- a/awscli/examples/global_synopsis.rst +++ b/awscli/examples/global_synopsis.rst @@ -16,4 +16,3 @@ [--no-cli-pager] [--cli-auto-prompt] [--no-cli-auto-prompt] -[--cli-error-format ] diff --git a/tests/unit/test_clidriver.py b/tests/unit/test_clidriver.py index 6214d07b1ce6..2140afae02f1 100644 --- a/tests/unit/test_clidriver.py +++ b/tests/unit/test_clidriver.py @@ -79,11 +79,6 @@ }, "read-timeout": {"type": "int", "help": ""}, "connect-timeout": {"type": "int", "help": ""}, - "cli-error-format": { - "choices": ["standard", "legacy"], - "default": "standard", - "help": "The formatting style for error output." - } }, }, } From bcc33fcb3f940a6431b10545826baae22f6c480a Mon Sep 17 00:00:00 2001 From: Andrew Asseily Date: Fri, 14 Nov 2025 13:51:26 -0500 Subject: [PATCH 03/26] Structured Errors implementation Draft 1 --- awscli/clidriver.py | 91 +++++++++++++-- awscli/structured_error.py | 117 +++++++++++++++++++ tests/unit/test_structured_error.py | 168 ++++++++++++++++++++++++++++ 3 files changed, 367 insertions(+), 9 deletions(-) create mode 100644 awscli/structured_error.py create mode 100644 tests/unit/test_structured_error.py diff --git a/awscli/clidriver.py b/awscli/clidriver.py index ae8a3af68845..b319864d80d8 100644 --- a/awscli/clidriver.py +++ b/awscli/clidriver.py @@ -76,6 +76,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 +276,12 @@ 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() + ) + config_store.set_config_provider( + 'cli_hide_error_details', self._construct_cli_hide_error_details_chain() + ) def _construct_cli_region_chain(self): providers = [ @@ -368,6 +375,34 @@ 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) + + def _construct_cli_hide_error_details_chain(self): + providers = [ + EnvironmentProvider( + name='AWS_CLI_HIDE_ERROR_DETAILS', + env=os.environ, + ), + ScopedConfigProvider( + config_var_name='cli_hide_error_details', + session=self.session, + ), + ConstantProvider(value='false'), + ] + return ChainProvider(providers=providers) + @property def subcommand_table(self): return self._get_command_table() @@ -983,6 +1018,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 +1061,50 @@ 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 - ) - return response + service_id = client._service_model.service_id.hyphenize() + operation_model = client._service_model.operation_model( + operation_name + ) + + event_name = f'after-call-error.{service_id}.{operation_model.name}' + + def error_handler(**kwargs): + try: + exception = kwargs.get('exception') + if exception: + handler = self._structured_error_handler + error_response = handler.extract_error_response( + exception + ) + if error_response: + 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, + ) + + client.meta.events.register(event_name, error_handler) + + 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, xform_name(operation_name))( + **parameters + ) + return response + finally: + client.meta.events.unregister(event_name, error_handler) def _display_response(self, command_name, response, parsed_globals): output = parsed_globals.output diff --git a/awscli/structured_error.py b/awscli/structured_error.py new file mode 100644 index 000000000000..4be6136825fe --- /dev/null +++ b/awscli/structured_error.py @@ -0,0 +1,117 @@ +# 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): + filtered_error_info = self._filter_sensitive_fields(error_info) + self.display(filtered_error_info, parsed_globals) + + def should_display(self, error_response, parsed_globals): + if not self._has_additional_error_members(error_response): + return False + + config_store = self._session.get_component('config_store') + hide_details = config_store.get_config_variable( + 'cli_hide_error_details' + ) + try: + if isinstance(hide_details, str): + hide_details = hide_details.lower() != 'false' + else: + hide_details = bool(hide_details) if hide_details else False + except (AttributeError, ValueError): + hide_details = False + + if hide_details: + return False + + error_format = config_store.get_config_variable('cli_error_format') + if error_format == 'LEGACY': + return False + + output = parsed_globals.output + if output is None: + output = self._session.get_config_variable('output') + if output == 'off': + return False + + return True + + def display(self, error_response, parsed_globals): + output = parsed_globals.output + if output is None: + output = self._session.get_config_variable('output') + + 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 _filter_sensitive_fields(self, error_info): + """Filter sensitive fields from error response before display. + + TODO: Implement sensitive output mitigation to filter fields + marked with the sensitive trait according to AWS CLI sensitive + output mitigation design. + """ + # Currently returns unfiltered + return error_info + + 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/unit/test_structured_error.py b/tests/unit/test_structured_error.py new file mode 100644 index 000000000000..65312b797475 --- /dev/null +++ b/tests/unit/test_structured_error.py @@ -0,0 +1,168 @@ +# 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 + + +class TestStructuredErrorHandler: + def setup_method(self): + self.session = FakeSession() + from awscli.utils import OutputStreamFactory + + 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_hide_details(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_hide_error_details', mock.Mock(provide=lambda: True) + ) + + 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) From 829968bf852ae2310f93240448bd4a046bf8c4ac Mon Sep 17 00:00:00 2001 From: Andrew Asseily Date: Mon, 17 Nov 2025 10:47:35 -0500 Subject: [PATCH 04/26] Refactor structured error --- awscli/clidriver.py | 63 +++++++++++++---------------- awscli/structured_error.py | 29 +++++++------ tests/unit/test_structured_error.py | 30 ++++++++++++++ 3 files changed, 73 insertions(+), 49 deletions(-) diff --git a/awscli/clidriver.py b/awscli/clidriver.py index b319864d80d8..5a9f5cbb7d94 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, @@ -1061,35 +1062,6 @@ def invoke(self, service_name, operation_name, parameters, parsed_globals): def _make_client_call( self, client, operation_name, parameters, parsed_globals ): - service_id = client._service_model.service_id.hyphenize() - operation_model = client._service_model.operation_model( - operation_name - ) - - event_name = f'after-call-error.{service_id}.{operation_model.name}' - - def error_handler(**kwargs): - try: - exception = kwargs.get('exception') - if exception: - handler = self._structured_error_handler - error_response = handler.extract_error_response( - exception - ) - if error_response: - 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, - ) - - client.meta.events.register(event_name, error_handler) - try: py_operation_name = xform_name(operation_name) if ( @@ -1099,12 +1071,35 @@ def error_handler(**kwargs): paginator = client.get_paginator(py_operation_name) response = paginator.paginate(**parameters) else: - response = getattr(client, xform_name(operation_name))( - **parameters - ) + response = getattr(client, py_operation_name)(**parameters) return response - finally: - client.meta.events.unregister(event_name, error_handler) + 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, + ) def _display_response(self, command_name, response, parsed_globals): output = parsed_globals.output diff --git a/awscli/structured_error.py b/awscli/structured_error.py index 4be6136825fe..4e3b5a4ae3f1 100644 --- a/awscli/structured_error.py +++ b/awscli/structured_error.py @@ -39,21 +39,18 @@ def handle_error(self, error_response, parsed_globals): filtered_error_info = self._filter_sensitive_fields(error_info) self.display(filtered_error_info, parsed_globals) - def should_display(self, error_response, parsed_globals): - if not self._has_additional_error_members(error_response): + 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') hide_details = config_store.get_config_variable( 'cli_hide_error_details' ) - try: - if isinstance(hide_details, str): - hide_details = hide_details.lower() != 'false' - else: - hide_details = bool(hide_details) if hide_details else False - except (AttributeError, ValueError): - hide_details = False + if isinstance(hide_details, str): + hide_details = hide_details.lower() == 'true' + else: + hide_details = bool(hide_details) if hide_details else False if hide_details: return False @@ -62,18 +59,14 @@ def should_display(self, error_response, parsed_globals): if error_format == 'LEGACY': return False - output = parsed_globals.output - if output is None: - output = self._session.get_config_variable('output') + output = self._get_output_format(parsed_globals) if output == 'off': return False return True def display(self, error_response, parsed_globals): - output = parsed_globals.output - if output is None: - output = self._session.get_config_variable('output') + output = self._get_output_format(parsed_globals) try: formatter = get_formatter(output, parsed_globals) @@ -88,6 +81,12 @@ def display(self, error_response, parsed_globals): 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 _filter_sensitive_fields(self, error_info): """Filter sensitive fields from error response before display. diff --git a/tests/unit/test_structured_error.py b/tests/unit/test_structured_error.py index 65312b797475..8bc53c76f000 100644 --- a/tests/unit/test_structured_error.py +++ b/tests/unit/test_structured_error.py @@ -166,3 +166,33 @@ def test_display_handles_exceptions_gracefully(self): 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) From 6958d85dfec12a24f97f6019c6a77fbb92c90c8f Mon Sep 17 00:00:00 2001 From: Andrew Asseily Date: Mon, 17 Nov 2025 16:48:44 -0500 Subject: [PATCH 05/26] Remove cli_hide_error_details --- awscli/clidriver.py | 17 ----------------- awscli/structured_error.py | 13 +------------ tests/unit/test_structured_error.py | 15 --------------- 3 files changed, 1 insertion(+), 44 deletions(-) diff --git a/awscli/clidriver.py b/awscli/clidriver.py index 5a9f5cbb7d94..77313a93fc5b 100644 --- a/awscli/clidriver.py +++ b/awscli/clidriver.py @@ -280,9 +280,6 @@ def _update_config_chain(self): config_store.set_config_provider( 'cli_error_format', self._construct_cli_error_format_chain() ) - config_store.set_config_provider( - 'cli_hide_error_details', self._construct_cli_hide_error_details_chain() - ) def _construct_cli_region_chain(self): providers = [ @@ -390,20 +387,6 @@ def _construct_cli_error_format_chain(self): ] return ChainProvider(providers=providers) - def _construct_cli_hide_error_details_chain(self): - providers = [ - EnvironmentProvider( - name='AWS_CLI_HIDE_ERROR_DETAILS', - env=os.environ, - ), - ScopedConfigProvider( - config_var_name='cli_hide_error_details', - session=self.session, - ), - ConstantProvider(value='false'), - ] - return ChainProvider(providers=providers) - @property def subcommand_table(self): return self._get_command_table() diff --git a/awscli/structured_error.py b/awscli/structured_error.py index 4e3b5a4ae3f1..f5263bd07cab 100644 --- a/awscli/structured_error.py +++ b/awscli/structured_error.py @@ -44,17 +44,6 @@ def should_display(self, error_info, parsed_globals): return False config_store = self._session.get_component('config_store') - hide_details = config_store.get_config_variable( - 'cli_hide_error_details' - ) - if isinstance(hide_details, str): - hide_details = hide_details.lower() == 'true' - else: - hide_details = bool(hide_details) if hide_details else False - - if hide_details: - return False - error_format = config_store.get_config_variable('cli_error_format') if error_format == 'LEGACY': return False @@ -92,7 +81,7 @@ def _filter_sensitive_fields(self, error_info): TODO: Implement sensitive output mitigation to filter fields marked with the sensitive trait according to AWS CLI sensitive - output mitigation design. + output mitigation design when finalized. """ # Currently returns unfiltered return error_info diff --git a/tests/unit/test_structured_error.py b/tests/unit/test_structured_error.py index 8bc53c76f000..b198f2f4fb68 100644 --- a/tests/unit/test_structured_error.py +++ b/tests/unit/test_structured_error.py @@ -83,21 +83,6 @@ def test_should_display_without_additional_members(self): assert not self.handler.should_display(error_response, parsed_globals) - def test_should_display_respects_hide_details(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_hide_error_details', mock.Mock(provide=lambda: True) - ) - - assert not self.handler.should_display(error_response, parsed_globals) - def test_should_display_respects_legacy_format(self): error_response = { 'Code': 'NoSuchBucket', From 329d1671c1afce5983ed001764f371323327d76f Mon Sep 17 00:00:00 2001 From: Andrew Asseily Date: Tue, 25 Nov 2025 14:32:44 -0500 Subject: [PATCH 06/26] Implement --output off format to suppress stdout --- awscli/formatter.py | 17 ++++++++++--- tests/unit/test_formatter.py | 48 +++++++++++++++++++++++++++++++++++- 2 files changed, 61 insertions(+), 4 deletions(-) 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/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) From 073c28dd0e184076ff3f962674f532163f127177 Mon Sep 17 00:00:00 2001 From: Andrew Asseily Date: Wed, 26 Nov 2025 15:22:48 -0500 Subject: [PATCH 07/26] Add missing off status to output choices --- awscli/data/cli.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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.

" }, From b82ef33a8bf8065723cd5990158274ac1d88e2ed Mon Sep 17 00:00:00 2001 From: Andrew Asseily Date: Mon, 1 Dec 2025 04:05:20 -0500 Subject: [PATCH 08/26] Catch and format ClientError exceptions raised during pagination --- awscli/clidriver.py | 8 +++-- tests/unit/test_structured_error.py | 53 ++++++++++++++++++++++++++++- 2 files changed, 58 insertions(+), 3 deletions(-) diff --git a/awscli/clidriver.py b/awscli/clidriver.py index 77313a93fc5b..109118f631e1 100644 --- a/awscli/clidriver.py +++ b/awscli/clidriver.py @@ -1090,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/tests/unit/test_structured_error.py b/tests/unit/test_structured_error.py index b198f2f4fb68..4ed610ba9640 100644 --- a/tests/unit/test_structured_error.py +++ b/tests/unit/test_structured_error.py @@ -17,12 +17,13 @@ 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() - from awscli.utils import OutputStreamFactory self.output_stream_factory = OutputStreamFactory(self.session) self.handler = StructuredErrorHandler( @@ -181,3 +182,53 @@ def test_should_display_with_parsed_globals_output_none_json(self): 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 From 7f511a2d5213bd8f8584b18db6a09ca7329847a1 Mon Sep 17 00:00:00 2001 From: Andrew Asseily Date: Tue, 2 Dec 2025 10:04:08 -0500 Subject: [PATCH 09/26] Update test fixtures and documentation for 'off' output format option --- awscli/examples/global_options.rst | 2 ++ tests/functional/autocomplete/test_completer.py | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) 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/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): From ae6987d66d41af223b25b21fd846ba0b1def9401 Mon Sep 17 00:00:00 2001 From: Andrew Asseily Date: Thu, 4 Dec 2025 14:17:45 -0500 Subject: [PATCH 10/26] Remove TODO for filtering; use --output off for sensitive data instead --- awscli/structured_error.py | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/awscli/structured_error.py b/awscli/structured_error.py index f5263bd07cab..d7897cfa11a3 100644 --- a/awscli/structured_error.py +++ b/awscli/structured_error.py @@ -36,8 +36,7 @@ def handle_error(self, error_response, parsed_globals): error_info = error_response.get('Error', {}) if self.should_display(error_info, parsed_globals): - filtered_error_info = self._filter_sensitive_fields(error_info) - self.display(filtered_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): @@ -76,16 +75,6 @@ def _get_output_format(self, parsed_globals): output = self._session.get_config_variable('output') return output - def _filter_sensitive_fields(self, error_info): - """Filter sensitive fields from error response before display. - - TODO: Implement sensitive output mitigation to filter fields - marked with the sensitive trait according to AWS CLI sensitive - output mitigation design when finalized. - """ - # Currently returns unfiltered - return error_info - def _has_additional_error_members(self, error_response): if not error_response: return False From 9db425e7bd4a47553b199218f070169c1895fdfb Mon Sep 17 00:00:00 2001 From: Andrew Asseily Date: Mon, 8 Dec 2025 10:20:16 -0500 Subject: [PATCH 11/26] Add changelog entry for structured error output --- .changes/next-release/feature-Output-59989.json | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changes/next-release/feature-Output-59989.json 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." +} From bc255155ffc71581b2da6ef23c5bc5e48a681f2c Mon Sep 17 00:00:00 2001 From: Andrew Asseily Date: Sun, 14 Dec 2025 13:04:51 -0500 Subject: [PATCH 12/26] Moves structured error handling into ClientErrorHandler to write errors with extra fields to stderr --- .../enhancement-Output-51826.json | 5 + .../next-release/feature-Output-59989.json | 2 +- awscli/clidriver.py | 47 +--- awscli/errorhandler.py | 76 +++++- awscli/examples/global_options.rst | 10 + awscli/formatter.py | 4 +- awscli/structured_error.py | 94 ------- awscli/topics/config-vars.rst | 15 ++ tests/unit/test_structured_error.py | 237 +++++++++--------- 9 files changed, 238 insertions(+), 252 deletions(-) create mode 100644 .changes/next-release/enhancement-Output-51826.json delete mode 100644 awscli/structured_error.py diff --git a/.changes/next-release/enhancement-Output-51826.json b/.changes/next-release/enhancement-Output-51826.json new file mode 100644 index 000000000000..685f6a4c976c --- /dev/null +++ b/.changes/next-release/enhancement-Output-51826.json @@ -0,0 +1,5 @@ +{ + "type": "enhancement", + "category": "Output", + "description": "Add support for ``--output off`` to suppress all stdout output while preserving stderr for errors and warnings." +} diff --git a/.changes/next-release/feature-Output-59989.json b/.changes/next-release/feature-Output-59989.json index 520f437b86ce..135bfc0ad6c8 100644 --- a/.changes/next-release/feature-Output-59989.json +++ b/.changes/next-release/feature-Output-59989.json @@ -1,5 +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." + "description": "AWS service errors containing modeled fields beyond Code and Message now write structured output to stderr in the configured output format. Set cli_error_format=LEGACY to disable." } diff --git a/awscli/clidriver.py b/awscli/clidriver.py index 109118f631e1..1d954d4dc0e5 100644 --- a/awscli/clidriver.py +++ b/awscli/clidriver.py @@ -22,7 +22,6 @@ 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, @@ -31,6 +30,7 @@ ScopedConfigProvider, ) from botocore.context import start_as_current_context +from botocore.exceptions import ClientError from botocore.history import get_global_history_recorder from awscli import __version__ @@ -77,7 +77,6 @@ ) 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, @@ -121,7 +120,7 @@ def create_clidriver(args=None): session.full_config.get('plugins', {}), event_hooks=session.get_component('event_emitter'), ) - error_handlers_chain = construct_cli_error_handlers_chain() + error_handlers_chain = construct_cli_error_handlers_chain(session) driver = CLIDriver( session=session, error_handler=error_handlers_chain, debug=debug ) @@ -248,7 +247,9 @@ def __init__(self, session=None, error_handler=None, debug=False): self.session = session self._error_handler = error_handler if self._error_handler is None: - self._error_handler = construct_cli_error_handlers_chain() + self._error_handler = construct_cli_error_handlers_chain( + self.session + ) if debug: self._set_logging(debug) self._update_config_chain() @@ -1002,9 +1003,6 @@ 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. @@ -1056,43 +1054,14 @@ def _make_client_call( 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 - ) + except ClientError: 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, - ) - def _display_response(self, command_name, response, parsed_globals): output = parsed_globals.output if output is None: output = self._session.get_config_variable('output') formatter = get_formatter(output, parsed_globals) - 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 + with self._output_stream_factory.get_output_stream() as stream: + formatter(command_name, response, stream) diff --git a/awscli/errorhandler.py b/awscli/errorhandler.py index 51d1c31f9514..573990fee676 100644 --- a/awscli/errorhandler.py +++ b/awscli/errorhandler.py @@ -10,6 +10,7 @@ # 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 argparse import logging import signal @@ -36,6 +37,7 @@ ConfigurationError, ParamValidationError, ) +from awscli.formatter import get_formatter from awscli.utils import PagerInitializationException LOG = logging.getLogger(__name__) @@ -51,7 +53,7 @@ def construct_entry_point_handlers_chain(): return ChainedExceptionHandler(exception_handlers=handlers) -def construct_cli_error_handlers_chain(): +def construct_cli_error_handlers_chain(session=None): handlers = [ ParamValidationErrorsHandler(), UnknownArgumentErrorHandler(), @@ -60,7 +62,7 @@ def construct_cli_error_handlers_chain(): NoCredentialsErrorHandler(), PagerErrorHandler(), InterruptExceptionHandler(), - ClientErrorHandler(), + ClientErrorHandler(session), GeneralExceptionHandler(), ] return ChainedExceptionHandler(exception_handlers=handlers) @@ -108,6 +110,76 @@ class ClientErrorHandler(FilteredExceptionHandler): EXCEPTIONS_TO_HANDLE = ClientError RC = CLIENT_ERROR_RC + def __init__(self, session=None): + self._session = session + + def _do_handle_exception(self, exception, stdout, stderr): + if self._session: + self._try_display_structured_error(exception, stderr) + + return super()._do_handle_exception(exception, stdout, stderr) + + def _try_display_structured_error(self, exception, stderr): + try: + error_response = self._extract_error_response(exception) + + if error_response and 'Error' in error_response: + error_info = error_response['Error'] + + if self._should_display_structured_error(error_info): + output = self._session.get_config_variable('output') + + parsed_globals = argparse.Namespace() + parsed_globals.output = output + parsed_globals.query = None + + formatter = get_formatter(output, parsed_globals) + formatter('error', error_info, stderr) + except Exception as e: + LOG.debug( + 'Failed to display structured error: %s', e, exc_info=True + ) + + def _should_display_structured_error(self, error_info): + 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: + error_format = error_format.upper() + + valid_formats = ['STANDARD', 'LEGACY'] + if error_format and error_format not in valid_formats: + raise ValueError( + f"Invalid cli_error_format: {error_format}. " + f"Valid values are: {', '.join(valid_formats)}" + ) + + if error_format == 'LEGACY': + return False + + return True + + 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 + class ConfigurationErrorHandler(FilteredExceptionHandler): EXCEPTIONS_TO_HANDLE = ConfigurationError diff --git a/awscli/examples/global_options.rst b/awscli/examples/global_options.rst index e8ba11090991..c60291999349 100644 --- a/awscli/examples/global_options.rst +++ b/awscli/examples/global_options.rst @@ -86,6 +86,16 @@ * raw-in-base64-out +``--cli-error-format`` (string) + + The formatting style for AWS service errors. When set to STANDARD, AWS service errors containing modeled fields beyond Code and Message are written to stderr in the configured output format. When set to LEGACY, errors are written to stderr as unstructured text. + + + * STANDARD + + * LEGACY + + ``--no-cli-pager`` (boolean) Disable cli pager for output. diff --git a/awscli/formatter.py b/awscli/formatter.py index 0f0f7f002611..903109b07afa 100644 --- a/awscli/formatter.py +++ b/awscli/formatter.py @@ -370,9 +370,9 @@ def _format_response(self, response, stream): class OffFormatter(Formatter): """Formatter that suppresses all output. - Only stdout is suppressed; stderr (error messages) remains visible. + Only stdout is suppressed; stderr (error messages) remains visible. """ - + def __call__(self, command_name, response, stream=None): # Suppress all output pass diff --git a/awscli/structured_error.py b/awscli/structured_error.py deleted file mode 100644 index d7897cfa11a3..000000000000 --- a/awscli/structured_error.py +++ /dev/null @@ -1,94 +0,0 @@ -# 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/awscli/topics/config-vars.rst b/awscli/topics/config-vars.rst index 8e291725c166..192c848cfca1 100644 --- a/awscli/topics/config-vars.rst +++ b/awscli/topics/config-vars.rst @@ -94,6 +94,21 @@ The valid values of the ``output`` configuration variable are: * json * table * text +* yaml +* yaml-stream +* off + +The ``off`` value suppresses all stdout output while preserving stderr for +errors and warnings. + +``cli_error_format`` controls how AWS service errors are displayed. The valid +values of the ``cli_error_format`` configuration variable are: + +* STANDARD - AWS service errors containing modeled fields beyond Code and + Message are written to stderr in the configured output format. This is the + default behavior. +* LEGACY - AWS service errors are written to stderr as unstructured text, + displaying only the error code and message. ``cli_timestamp_format`` controls the format of timestamps displayed by the AWS CLI. The valid values of the ``cli_timestamp_format`` configuration variable are: diff --git a/tests/unit/test_structured_error.py b/tests/unit/test_structured_error.py index 4ed610ba9640..afad6849add7 100644 --- a/tests/unit/test_structured_error.py +++ b/tests/unit/test_structured_error.py @@ -15,20 +15,15 @@ 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 +from awscli.errorhandler import ClientErrorHandler +from tests.unit.test_clidriver import FakeSession -class TestStructuredErrorHandler: +class TestClientErrorHandler: def setup_method(self): self.session = FakeSession() - - self.output_stream_factory = OutputStreamFactory(self.session) - self.handler = StructuredErrorHandler( - self.session, self.output_stream_factory - ) + self.handler = ClientErrorHandler(self.session) def test_extract_error_response_from_client_error(self): error_response = { @@ -41,7 +36,7 @@ def test_extract_error_response_from_client_error(self): } client_error = ClientError(error_response, 'GetObject') - result = StructuredErrorHandler.extract_error_response(client_error) + result = ClientErrorHandler._extract_error_response(client_error) assert result is not None assert 'Error' in result @@ -49,7 +44,7 @@ def test_extract_error_response_from_client_error(self): assert result['Error']['BucketName'] == 'my-bucket' def test_extract_error_response_from_non_client_error(self): - result = StructuredErrorHandler.extract_error_response( + result = ClientErrorHandler._extract_error_response( ValueError('Some error') ) assert result is None @@ -67,121 +62,73 @@ def test_has_additional_error_members(self): assert not self.handler._has_additional_error_members(None) def test_should_display_with_additional_members(self): - error_response = { + error_info = { 'Code': 'NoSuchBucket', 'Message': 'Error', 'BucketName': 'my-bucket', } - parsed_globals = mock.Mock() - parsed_globals.output = 'json' - assert self.handler.should_display(error_response, parsed_globals) + assert self.handler._should_display_structured_error(error_info) def test_should_display_without_additional_members(self): - error_response = {'Code': 'AccessDenied', 'Message': 'Access Denied'} - parsed_globals = mock.Mock() - parsed_globals.output = 'json' + error_info = {'Code': 'AccessDenied', 'Message': 'Access Denied'} - assert not self.handler.should_display(error_response, parsed_globals) + assert not self.handler._should_display_structured_error(error_info) def test_should_display_respects_legacy_format(self): - error_response = { + error_info = { '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) + assert not self.handler._should_display_structured_error(error_info) - def test_should_display_respects_output_off(self): - error_response = { + def test_should_display_validates_error_format(self): + error_info = { '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.session.config_store.set_config_provider( + 'cli_error_format', mock.Mock(provide=lambda: 'INVALID') ) - 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 = { + try: + self.handler._should_display_structured_error(error_info) + assert False, "Expected ValueError to be raised" + except ValueError as e: + assert 'Invalid cli_error_format' in str(e) + assert 'INVALID' in str(e) + assert 'STANDARD' in str(e) + assert 'LEGACY' in str(e) + + def test_should_display_case_insensitive(self): + error_info = { '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 - ) + self.session.config_store.set_config_provider( + 'cli_error_format', mock.Mock(provide=lambda: 'standard') + ) + assert self.handler._should_display_structured_error(error_info) - 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 + self.session.config_store.set_config_provider( + 'cli_error_format', mock.Mock(provide=lambda: 'legacy') + ) + assert not self.handler._should_display_structured_error(error_info) - with mock.patch.object( - self.session, 'get_config_variable', return_value='json' - ): - assert self.handler.should_display(error_response, parsed_globals) + self.session.config_store.set_config_provider( + 'cli_error_format', mock.Mock(provide=lambda: 'Standard') + ) + assert self.handler._should_display_structured_error(error_info) class TestStructuredErrorWithPagination: @@ -189,7 +136,8 @@ def setup_method(self): self.session = FakeSession() self.caller = CLIOperationCaller(self.session) - def test_formatter_error_displays_structured_error(self): + def test_formatter_error_propagates_to_error_handler(self): + """Test ClientError during formatting propagates to handler.""" error_response = { 'Error': { 'Code': 'AccessDenied', @@ -208,27 +156,88 @@ def test_formatter_error_displays_structured_error(self): 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 + # The error should propagate without being caught + try: + self.caller._display_response( + 'list-objects', {}, parsed_globals + ) + assert False, "Expected ClientError to be raised" + except ClientError as e: + # Verify the error propagated correctly + assert e.response['Error']['Code'] == 'AccessDenied' + assert e.response['Error']['BucketName'] == 'my-bucket' + + +class TestClientErrorHandlerWithStructuredErrors: + def setup_method(self): + self.session = FakeSession() + + def test_client_error_handler_displays_structured_error(self): + from awscli.errorhandler import ClientErrorHandler + + error_response = { + 'Error': { + 'Code': 'NoSuchBucket', + 'Message': 'The specified bucket does not exist', + 'BucketName': 'my-bucket', + }, + 'ResponseMetadata': {'RequestId': '123'}, + } + client_error = ClientError(error_response, 'GetObject') + + handler = ClientErrorHandler(self.session) + + stdout = io.StringIO() + stderr = io.StringIO() + + rc = handler.handle_exception(client_error, stdout, stderr) + + # Should return CLIENT_ERROR_RC + from awscli.constants import CLIENT_ERROR_RC + + assert rc == CLIENT_ERROR_RC + + stderr_output = stderr.getvalue() + assert 'NoSuchBucket' in stderr_output + assert 'my-bucket' in stderr_output + assert 'ClientError' in stderr_output or '"Code"' in stderr_output + + stdout_output = stdout.getvalue() + assert stdout_output == '' + + def test_client_error_handler_without_additional_fields(self): + from awscli.errorhandler import ClientErrorHandler + + error_response = { + 'Error': { + 'Code': 'AccessDenied', + 'Message': 'Access Denied', + }, + 'ResponseMetadata': {'RequestId': '123'}, + } + client_error = ClientError(error_response, 'GetObject') + + handler = ClientErrorHandler(self.session) + + stdout = io.StringIO() + stderr = io.StringIO() + + rc = handler.handle_exception(client_error, stdout, stderr) + + # Should return CLIENT_ERROR_RC + from awscli.constants import CLIENT_ERROR_RC + + assert rc == CLIENT_ERROR_RC + + stdout_output = stdout.getvalue() + assert stdout_output == '' + + stderr_output = stderr.getvalue() + assert ( + 'ClientError' in stderr_output + or 'AccessDenied' in stderr_output + ) + assert '"Code"' not in stderr_output From ce86991e59cf894753d7b6fe8c2fa89f4a2ec47e Mon Sep 17 00:00:00 2001 From: Andrew Asseily Date: Sun, 14 Dec 2025 14:09:08 -0500 Subject: [PATCH 13/26] Remove cli_error_format from global options docs (config-only variable) --- awscli/examples/global_options.rst | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/awscli/examples/global_options.rst b/awscli/examples/global_options.rst index c60291999349..e8ba11090991 100644 --- a/awscli/examples/global_options.rst +++ b/awscli/examples/global_options.rst @@ -86,16 +86,6 @@ * raw-in-base64-out -``--cli-error-format`` (string) - - The formatting style for AWS service errors. When set to STANDARD, AWS service errors containing modeled fields beyond Code and Message are written to stderr in the configured output format. When set to LEGACY, errors are written to stderr as unstructured text. - - - * STANDARD - - * LEGACY - - ``--no-cli-pager`` (boolean) Disable cli pager for output. From 29ee23cec9acd4a6c67c00d79d016fbf5c3b3e4e Mon Sep 17 00:00:00 2001 From: Andrew Asseily Date: Wed, 17 Dec 2025 10:45:06 -0500 Subject: [PATCH 14/26] Add structured error output with configurable formats (json, yaml, enhanced, text, table, legacy) --- awscli/clidriver.py | 5 +- awscli/data/cli.json | 11 + awscli/errorhandler.py | 181 ++++++++++---- awscli/examples/global_options.rst | 16 ++ tests/unit/test_structured_error.py | 370 +++++++++++++++++++--------- 5 files changed, 420 insertions(+), 163 deletions(-) diff --git a/awscli/clidriver.py b/awscli/clidriver.py index 1d954d4dc0e5..1c5f7553bdda 100644 --- a/awscli/clidriver.py +++ b/awscli/clidriver.py @@ -384,7 +384,7 @@ def _construct_cli_error_format_chain(self): config_var_name='cli_error_format', session=self.session, ), - ConstantProvider(value='STANDARD'), + ConstantProvider(value='enhanced'), ] return ChainProvider(providers=providers) @@ -542,6 +542,9 @@ def main(self, args=None): # general exception handling logic as calling into the # command table. This is why it's in the try/except clause. parsed_args, remaining = parser.parse_known_args(args) + self._error_handler = construct_cli_error_handlers_chain( + self.session, parsed_args + ) self._handle_top_level_args(parsed_args) validate_preferred_output_encoding() self._emit_session_event(parsed_args) diff --git a/awscli/data/cli.json b/awscli/data/cli.json index 7183b5219e4f..312d46b9e192 100644 --- a/awscli/data/cli.json +++ b/awscli/data/cli.json @@ -86,6 +86,17 @@ "no-cli-auto-prompt": { "action": "store_true", "help": "

Disable automatically prompt for CLI input parameters.

" + }, + "cli-error-format": { + "choices": [ + "legacy", + "json", + "yaml", + "text", + "table", + "enhanced" + ], + "help": "

The formatting style for error output. By default, errors are displayed in enhanced format.

" } } } diff --git a/awscli/errorhandler.py b/awscli/errorhandler.py index 573990fee676..c4ae5ee31fbe 100644 --- a/awscli/errorhandler.py +++ b/awscli/errorhandler.py @@ -42,6 +42,78 @@ LOG = logging.getLogger(__name__) +VALID_ERROR_FORMATS = ['legacy', 'json', 'yaml', 'text', 'table', 'enhanced'] + + +class EnhancedErrorFormatter: + def format_error(self, error_info, formatted_message, stream): + stream.write(formatted_message) + stream.write('\n') + + additional_fields = self._get_additional_fields(error_info) + + if not additional_fields: + return + + if len(additional_fields) == 1: + stream.write('\n') + for key, value in additional_fields.items(): + if self._is_simple_value(value): + stream.write(f'{key}: {value}\n') + elif self._is_small_collection(value): + stream.write(f'{key}: {self._format_inline(value)}\n') + else: + stream.write( + f'{key}: \n' + f'(Use --error-format json or --error-format yaml ' + f'to see full details)\n' + ) + else: + stream.write('\nAdditional error details:\n') + for key, value in additional_fields.items(): + if self._is_simple_value(value): + stream.write(f' {key}: {value}\n') + elif self._is_small_collection(value): + stream.write(f' {key}: {self._format_inline(value)}\n') + else: + stream.write( + f' {key}: \n' + f' (Use --error-format json or --error-format yaml ' + f'to see full details)\n' + ) + + def _is_simple_value(self, value): + return isinstance(value, (str, int, float, bool, type(None))) + + def _is_small_collection(self, value): + if isinstance(value, list): + return ( + len(value) < 5 + and all(self._is_simple_value(item) for item in value) + ) + elif isinstance(value, dict): + return ( + len(value) < 5 + and all(self._is_simple_value(v) for v in value.values()) + ) + return False + + def _format_inline(self, value): + if isinstance(value, list): + return '[' + ', '.join(str(item) for item in value) + ']' + elif isinstance(value, dict): + items = [f'{k}: {v}' for k, v in value.items()] + return '{' + ', '.join(items) + '}' + 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 + } + def construct_entry_point_handlers_chain(): handlers = [ @@ -53,7 +125,7 @@ def construct_entry_point_handlers_chain(): return ChainedExceptionHandler(exception_handlers=handlers) -def construct_cli_error_handlers_chain(session=None): +def construct_cli_error_handlers_chain(session=None, parsed_globals=None): handlers = [ ParamValidationErrorsHandler(), UnknownArgumentErrorHandler(), @@ -62,7 +134,7 @@ def construct_cli_error_handlers_chain(session=None): NoCredentialsErrorHandler(), PagerErrorHandler(), InterruptExceptionHandler(), - ClientErrorHandler(session), + ClientErrorHandler(session, parsed_globals), GeneralExceptionHandler(), ] return ChainedExceptionHandler(exception_handlers=handlers) @@ -110,14 +182,42 @@ class ClientErrorHandler(FilteredExceptionHandler): EXCEPTIONS_TO_HANDLE = ClientError RC = CLIENT_ERROR_RC - def __init__(self, session=None): + def __init__(self, session=None, parsed_globals=None): self._session = session + self._parsed_globals = parsed_globals + self._enhanced_formatter = EnhancedErrorFormatter() def _do_handle_exception(self, exception, stdout, stderr): + displayed_structured = False if self._session: - self._try_display_structured_error(exception, stderr) + displayed_structured = self._try_display_structured_error( + exception, stderr + ) + + if not displayed_structured: + return super()._do_handle_exception(exception, stdout, stderr) + + return self.RC + + def _resolve_error_format(self, parsed_globals): + error_format = ( + getattr(parsed_globals, 'cli_error_format', None) + if parsed_globals + else None + ) + + if error_format is None: + try: + error_format = self._session.get_config_variable( + 'cli_error_format' + ) + except (KeyError, AttributeError): + error_format = None - return super()._do_handle_exception(exception, stdout, stderr) + if error_format is None: + error_format = 'enhanced' + + return error_format.lower() def _try_display_structured_error(self, exception, stderr): try: @@ -126,49 +226,46 @@ def _try_display_structured_error(self, exception, stderr): if error_response and 'Error' in error_response: error_info = error_response['Error'] - if self._should_display_structured_error(error_info): - output = self._session.get_config_variable('output') - - parsed_globals = argparse.Namespace() - parsed_globals.output = output - parsed_globals.query = None - - formatter = get_formatter(output, parsed_globals) + parsed_globals = self._parsed_globals + + error_format = self._resolve_error_format(parsed_globals) + + if error_format not in VALID_ERROR_FORMATS: + raise ValueError( + f"Invalid cli_error_format: {error_format}. " + f"Valid values are: {', '.join(VALID_ERROR_FORMATS)}" + ) + + formatted_message = str(exception) + + if error_format == 'legacy': + return False + elif error_format == 'enhanced': + self._enhanced_formatter.format_error( + error_info, formatted_message, stderr + ) + return True + else: + temp_parsed_globals = argparse.Namespace() + temp_parsed_globals.output = error_format + temp_parsed_globals.query = None + temp_parsed_globals.color = ( + getattr(parsed_globals, 'color', 'auto') + if parsed_globals + else 'auto' + ) + + formatter = get_formatter( + error_format, temp_parsed_globals + ) formatter('error', error_info, stderr) + return True except Exception as e: LOG.debug( 'Failed to display structured error: %s', e, exc_info=True ) - def _should_display_structured_error(self, error_info): - 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: - error_format = error_format.upper() - - valid_formats = ['STANDARD', 'LEGACY'] - if error_format and error_format not in valid_formats: - raise ValueError( - f"Invalid cli_error_format: {error_format}. " - f"Valid values are: {', '.join(valid_formats)}" - ) - - if error_format == 'LEGACY': - return False - - return True - - 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 + return False @staticmethod def _extract_error_response(exception): diff --git a/awscli/examples/global_options.rst b/awscli/examples/global_options.rst index e8ba11090991..1495b0bc7b8e 100644 --- a/awscli/examples/global_options.rst +++ b/awscli/examples/global_options.rst @@ -98,3 +98,19 @@ Disable automatically prompt for CLI input parameters. +``--cli-error-format`` (string) + + The formatting style for error output. By default, errors are displayed in enhanced format. + + + * legacy + + * json + + * yaml + + * text + + * table + + * enhanced diff --git a/tests/unit/test_structured_error.py b/tests/unit/test_structured_error.py index afad6849add7..c24b63d5db7a 100644 --- a/tests/unit/test_structured_error.py +++ b/tests/unit/test_structured_error.py @@ -1,4 +1,4 @@ -# Copyright 2012-2013 Amazon.com, Inc. or its affiliates. All Rights Reserved. +# Copyright 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 @@ -10,13 +10,19 @@ # 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 argparse import io from unittest import mock from botocore.exceptions import ClientError -from awscli.clidriver import CLIOperationCaller -from awscli.errorhandler import ClientErrorHandler +from awscli.clidriver import CLIDriver +from awscli.constants import CLIENT_ERROR_RC +from awscli.errorhandler import ( + ClientErrorHandler, + EnhancedErrorFormatter, + construct_cli_error_handlers_chain, +) from tests.unit.test_clidriver import FakeSession @@ -49,195 +55,319 @@ def test_extract_error_response_from_non_client_error(self): ) assert result is None - def test_has_additional_error_members(self): - assert self.handler._has_additional_error_members( - {'Code': 'NoSuchBucket', 'Message': 'Error', 'BucketName': 'test'} - ) + def test_displays_structured_error_with_additional_members(self): + error_response = { + 'Error': { + 'Code': 'NoSuchBucket', + 'Message': 'Error', + 'BucketName': 'my-bucket', + }, + 'ResponseMetadata': {'RequestId': '123'}, + } + client_error = ClientError(error_response, 'GetObject') - assert not self.handler._has_additional_error_members( - {'Code': 'AccessDenied', 'Message': 'Access Denied'} - ) + stdout = io.StringIO() + stderr = io.StringIO() - assert not self.handler._has_additional_error_members({}) - assert not self.handler._has_additional_error_members(None) + rc = self.handler.handle_exception(client_error, stdout, stderr) - def test_should_display_with_additional_members(self): - error_info = { - 'Code': 'NoSuchBucket', - 'Message': 'Error', - 'BucketName': 'my-bucket', + assert rc == CLIENT_ERROR_RC + stderr_output = stderr.getvalue() + assert 'NoSuchBucket' in stderr_output + assert 'my-bucket' in stderr_output + assert 'BucketName' in stderr_output + + def test_displays_standard_error_without_additional_members(self): + error_response = { + 'Error': { + 'Code': 'AccessDenied', + 'Message': 'Access Denied', + }, + 'ResponseMetadata': {'RequestId': '123'}, } + client_error = ClientError(error_response, 'GetObject') - assert self.handler._should_display_structured_error(error_info) + stdout = io.StringIO() + stderr = io.StringIO() - def test_should_display_without_additional_members(self): - error_info = {'Code': 'AccessDenied', 'Message': 'Access Denied'} + rc = self.handler.handle_exception(client_error, stdout, stderr) - assert not self.handler._should_display_structured_error(error_info) + assert rc == CLIENT_ERROR_RC + stderr_output = stderr.getvalue() + assert 'AccessDenied' in stderr_output + assert 'Additional error details' not in stderr_output - def test_should_display_respects_legacy_format(self): - error_info = { - 'Code': 'NoSuchBucket', - 'Message': 'Error', - 'BucketName': 'test', + def test_respects_legacy_format_config(self): + error_response = { + 'Error': { + 'Code': 'NoSuchBucket', + 'Message': 'Error', + 'BucketName': 'test', + }, + 'ResponseMetadata': {'RequestId': '123'}, } + client_error = ClientError(error_response, 'GetObject') self.session.config_store.set_config_provider( - 'cli_error_format', mock.Mock(provide=lambda: 'LEGACY') + 'cli_error_format', mock.Mock(provide=lambda: 'legacy') ) - assert not self.handler._should_display_structured_error(error_info) + stdout = io.StringIO() + stderr = io.StringIO() - def test_should_display_validates_error_format(self): - error_info = { - 'Code': 'NoSuchBucket', - 'Message': 'Error', - 'BucketName': 'test', + rc = self.handler.handle_exception(client_error, stdout, stderr) + + assert rc == CLIENT_ERROR_RC + stderr_output = stderr.getvalue() + # Legacy format should not show structured fields + assert 'NoSuchBucket' in stderr_output + + def test_error_format_case_insensitive(self): + """Test that error format config is case-insensitive.""" + error_response = { + 'Error': { + 'Code': 'NoSuchBucket', + 'Message': 'Error', + 'BucketName': 'test', + }, + 'ResponseMetadata': {'RequestId': '123'}, } + client_error = ClientError(error_response, 'GetObject') self.session.config_store.set_config_provider( - 'cli_error_format', mock.Mock(provide=lambda: 'INVALID') + 'cli_error_format', mock.Mock(provide=lambda: 'Enhanced') + ) + + stdout = io.StringIO() + stderr = io.StringIO() + + rc = self.handler.handle_exception(client_error, stdout, stderr) + + assert rc == CLIENT_ERROR_RC + stderr_output = stderr.getvalue() + assert 'NoSuchBucket' in stderr_output + assert 'test' in stderr_output + + +class TestEnhancedErrorFormatter: + def setup_method(self): + + self.formatter = EnhancedErrorFormatter() + + def test_is_simple_value(self): + assert self.formatter._is_simple_value('string') + assert self.formatter._is_simple_value(42) + assert self.formatter._is_simple_value(3.14) + assert self.formatter._is_simple_value(True) + assert self.formatter._is_simple_value(None) + assert not self.formatter._is_simple_value([1, 2, 3]) + assert not self.formatter._is_simple_value({'key': 'value'}) + + def test_is_small_collection_list(self): + assert self.formatter._is_small_collection(['a', 'b']) + assert self.formatter._is_small_collection([1, 2, 3, 4]) + + assert not self.formatter._is_small_collection([1, 2, 3, 4, 5]) + + assert not self.formatter._is_small_collection([1, [2, 3]]) + assert not self.formatter._is_small_collection([{'key': 'value'}]) + + def test_is_small_collection_dict(self): + assert self.formatter._is_small_collection({'key': 'value'}) + assert self.formatter._is_small_collection( + {'a': 1, 'b': 2, 'c': 3, 'd': 4} + ) + + assert not self.formatter._is_small_collection( + {'a': 1, 'b': 2, 'c': 3, 'd': 4, 'e': 5} ) - try: - self.handler._should_display_structured_error(error_info) - assert False, "Expected ValueError to be raised" - except ValueError as e: - assert 'Invalid cli_error_format' in str(e) - assert 'INVALID' in str(e) - assert 'STANDARD' in str(e) - assert 'LEGACY' in str(e) + assert not self.formatter._is_small_collection({'key': [1, 2]}) + assert not self.formatter._is_small_collection({'key': {'nested': 1}}) - def test_should_display_case_insensitive(self): + def test_format_inline_list(self): + result = self.formatter._format_inline([1, 2, 3]) + assert result == '[1, 2, 3]' + + result = self.formatter._format_inline(['a', 'b', 'c']) + assert result == '[a, b, c]' + + def test_format_inline_dict(self): + result = self.formatter._format_inline({'a': 1, 'b': 2}) + assert 'a: 1' in result + assert 'b: 2' in result + assert result.startswith('{') + assert result.endswith('}') + + def test_get_additional_fields(self): error_info = { 'Code': 'NoSuchBucket', - 'Message': 'Error', - 'BucketName': 'test', + 'Message': 'The bucket does not exist', + 'BucketName': 'my-bucket', + 'Region': 'us-east-1', } - self.session.config_store.set_config_provider( - 'cli_error_format', mock.Mock(provide=lambda: 'standard') + additional = self.formatter._get_additional_fields(error_info) + assert 'Code' not in additional + assert 'Message' not in additional + assert additional['BucketName'] == 'my-bucket' + assert additional['Region'] == 'us-east-1' + + def test_format_error_with_no_additional_fields(self): + error_info = { + 'Code': 'AccessDenied', + 'Message': 'Access Denied', + } + formatted_message = 'An error occurred (AccessDenied): Access Denied' + + stream = io.StringIO() + self.formatter.format_error(error_info, formatted_message, stream) + + output = stream.getvalue() + assert formatted_message in output + assert 'Additional error details' not in output + + def test_format_error_with_simple_fields(self): + error_info = { + 'Code': 'NoSuchBucket', + 'Message': 'The bucket does not exist', + 'BucketName': 'my-bucket', + 'Region': 'us-east-1', + } + formatted_message = ( + 'An error occurred (NoSuchBucket): The bucket does not exist' ) - assert self.handler._should_display_structured_error(error_info) - self.session.config_store.set_config_provider( - 'cli_error_format', mock.Mock(provide=lambda: 'legacy') + stream = io.StringIO() + self.formatter.format_error(error_info, formatted_message, stream) + + output = stream.getvalue() + assert formatted_message in output + assert 'Additional error details' in output + assert 'BucketName: my-bucket' in output + assert 'Region: us-east-1' in output + + def test_format_error_with_small_list(self): + error_info = { + 'Code': 'ValidationError', + 'Message': 'Validation failed', + 'AllowedValues': ['value1', 'value2', 'value3'], + } + formatted_message = ( + 'An error occurred (ValidationError): Validation failed' ) - assert not self.handler._should_display_structured_error(error_info) - self.session.config_store.set_config_provider( - 'cli_error_format', mock.Mock(provide=lambda: 'Standard') + stream = io.StringIO() + self.formatter.format_error(error_info, formatted_message, stream) + + output = stream.getvalue() + assert formatted_message in output + assert 'Additional error details' not in output + assert 'AllowedValues: [value1, value2, value3]' in output + + def test_format_error_with_small_dict(self): + error_info = { + 'Code': 'ValidationError', + 'Message': 'Validation failed', + 'Metadata': {'key1': 'value1', 'key2': 'value2'}, + } + formatted_message = ( + 'An error occurred (ValidationError): Validation failed' ) - assert self.handler._should_display_structured_error(error_info) + stream = io.StringIO() + self.formatter.format_error(error_info, formatted_message, stream) -class TestStructuredErrorWithPagination: - def setup_method(self): - self.session = FakeSession() - self.caller = CLIOperationCaller(self.session) + output = stream.getvalue() + assert formatted_message in output + assert 'Additional error details' not in output + assert 'Metadata:' in output + assert 'key1: value1' in output + assert 'key2: value2' in output - def test_formatter_error_propagates_to_error_handler(self): - """Test ClientError during formatting propagates to handler.""" - error_response = { - 'Error': { - 'Code': 'AccessDenied', - 'Message': 'Access Denied', - 'BucketName': 'my-bucket', - }, - 'ResponseMetadata': {'RequestId': '123'}, + def test_format_error_with_complex_object(self): + error_info = { + 'Code': 'ValidationError', + 'Message': 'Validation failed', + 'Details': [1, 2, 3, 4, 5, 6], } + formatted_message = ( + 'An error occurred (ValidationError): Validation failed' + ) - client_error = ClientError(error_response, 'ListObjects') + stream = io.StringIO() + self.formatter.format_error(error_info, formatted_message, stream) - parsed_globals = mock.Mock() - parsed_globals.output = 'json' - parsed_globals.query = None + output = stream.getvalue() + assert formatted_message in output + assert 'Additional error details' not in output + assert 'Details: ' in output + assert '--error-format json' in output + assert '--error-format yaml' in output - mock_formatter = mock.Mock() - mock_formatter.side_effect = client_error - with mock.patch( - 'awscli.clidriver.get_formatter', return_value=mock_formatter - ): - # The error should propagate without being caught - try: - self.caller._display_response( - 'list-objects', {}, parsed_globals - ) - assert False, "Expected ClientError to be raised" - except ClientError as e: - # Verify the error propagated correctly - assert e.response['Error']['Code'] == 'AccessDenied' - assert e.response['Error']['BucketName'] == 'my-bucket' +class TestParsedGlobalsPassthrough: + def test_error_handler_receives_parsed_globals_from_clidriver(self): -class TestClientErrorHandlerWithStructuredErrors: - def setup_method(self): - self.session = FakeSession() + session = FakeSession() - def test_client_error_handler_displays_structured_error(self): - from awscli.errorhandler import ClientErrorHandler + parsed_globals = argparse.Namespace() + parsed_globals.cli_error_format = 'json' + parsed_globals.command = 's3' + parsed_globals.color = 'auto' + + error_handler = construct_cli_error_handlers_chain( + session, parsed_globals + ) error_response = { 'Error': { 'Code': 'NoSuchBucket', 'Message': 'The specified bucket does not exist', - 'BucketName': 'my-bucket', + 'BucketName': 'test-bucket', }, 'ResponseMetadata': {'RequestId': '123'}, } client_error = ClientError(error_response, 'GetObject') - handler = ClientErrorHandler(self.session) - stdout = io.StringIO() stderr = io.StringIO() - rc = handler.handle_exception(client_error, stdout, stderr) - - # Should return CLIENT_ERROR_RC - from awscli.constants import CLIENT_ERROR_RC + rc = error_handler.handle_exception(client_error, stdout, stderr) assert rc == CLIENT_ERROR_RC stderr_output = stderr.getvalue() + assert ( + '"Code"' in stderr_output or '"code"' in stderr_output.lower() + ) assert 'NoSuchBucket' in stderr_output - assert 'my-bucket' in stderr_output - assert 'ClientError' in stderr_output or '"Code"' in stderr_output + assert 'test-bucket' in stderr_output - stdout_output = stdout.getvalue() - assert stdout_output == '' + def test_error_handler_without_parsed_globals_uses_default(self): + session = FakeSession() - def test_client_error_handler_without_additional_fields(self): - from awscli.errorhandler import ClientErrorHandler + error_handler = construct_cli_error_handlers_chain(session, None) error_response = { 'Error': { - 'Code': 'AccessDenied', - 'Message': 'Access Denied', + 'Code': 'NoSuchBucket', + 'Message': 'The specified bucket does not exist', + 'BucketName': 'test-bucket', }, 'ResponseMetadata': {'RequestId': '123'}, } client_error = ClientError(error_response, 'GetObject') - handler = ClientErrorHandler(self.session) - stdout = io.StringIO() stderr = io.StringIO() - rc = handler.handle_exception(client_error, stdout, stderr) - - # Should return CLIENT_ERROR_RC - from awscli.constants import CLIENT_ERROR_RC + rc = error_handler.handle_exception(client_error, stdout, stderr) assert rc == CLIENT_ERROR_RC - stdout_output = stdout.getvalue() - assert stdout_output == '' - stderr_output = stderr.getvalue() - assert ( - 'ClientError' in stderr_output - or 'AccessDenied' in stderr_output - ) - assert '"Code"' not in stderr_output + assert 'NoSuchBucket' in stderr_output + assert 'test-bucket' in stderr_output + assert 'BucketName' in stderr_output From e003f0937f97caa45411ecfffa9c92f80bf77ad8 Mon Sep 17 00:00:00 2001 From: Andrew Asseily Date: Wed, 17 Dec 2025 11:11:41 -0500 Subject: [PATCH 15/26] Update changelog entry to reflect new error format settings --- .changes/next-release/feature-Output-59989.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changes/next-release/feature-Output-59989.json b/.changes/next-release/feature-Output-59989.json index 135bfc0ad6c8..115a7ad09e17 100644 --- a/.changes/next-release/feature-Output-59989.json +++ b/.changes/next-release/feature-Output-59989.json @@ -1,5 +1,5 @@ { "type": "feature", "category": "Output", - "description": "AWS service errors containing modeled fields beyond Code and Message now write structured output to stderr in the configured output format. Set cli_error_format=LEGACY to disable." + "description": "Add structured error output with configurable formats. AWS service errors now display additional fields in the configured format (legacy, json, yaml, text, table, or enhanced). Configure via ``--cli-error-format``, ``cli_error_format`` config variable, or ``AWS_CLI_ERROR_FORMAT`` environment variable. The new enhanced format is the default. Set ``cli_error_format=legacy`` to preserve the original error format." } From 4f87cfa0913b16047c89729ff1b5e47acdfc6ded Mon Sep 17 00:00:00 2001 From: Andrew Asseily Date: Wed, 17 Dec 2025 11:21:40 -0500 Subject: [PATCH 16/26] Update validation text and format relevant files --- awscli/errorhandler.py | 20 +++++++------------- tests/unit/test_structured_error.py | 7 +------ 2 files changed, 8 insertions(+), 19 deletions(-) diff --git a/awscli/errorhandler.py b/awscli/errorhandler.py index c4ae5ee31fbe..6c46f819db91 100644 --- a/awscli/errorhandler.py +++ b/awscli/errorhandler.py @@ -87,14 +87,12 @@ def _is_simple_value(self, value): def _is_small_collection(self, value): if isinstance(value, list): - return ( - len(value) < 5 - and all(self._is_simple_value(item) for item in value) + return len(value) < 5 and all( + self._is_simple_value(item) for item in value ) elif isinstance(value, dict): - return ( - len(value) < 5 - and all(self._is_simple_value(v) for v in value.values()) + return len(value) < 5 and all( + self._is_simple_value(v) for v in value.values() ) return False @@ -108,11 +106,7 @@ def _format_inline(self, 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 - } + return {k: v for k, v in error_info.items() if k not in standard_keys} def construct_entry_point_handlers_chain(): @@ -232,8 +226,8 @@ def _try_display_structured_error(self, exception, stderr): if error_format not in VALID_ERROR_FORMATS: raise ValueError( - f"Invalid cli_error_format: {error_format}. " - f"Valid values are: {', '.join(VALID_ERROR_FORMATS)}" + f"Invalid value for cli_error_format: '{error_format}'\n" + f"Valid choices are: {', '.join(VALID_ERROR_FORMATS)}" ) formatted_message = str(exception) diff --git a/tests/unit/test_structured_error.py b/tests/unit/test_structured_error.py index c24b63d5db7a..2a2e113f3734 100644 --- a/tests/unit/test_structured_error.py +++ b/tests/unit/test_structured_error.py @@ -151,7 +151,6 @@ def test_error_format_case_insensitive(self): class TestEnhancedErrorFormatter: def setup_method(self): - self.formatter = EnhancedErrorFormatter() def test_is_simple_value(self): @@ -307,9 +306,7 @@ def test_format_error_with_complex_object(self): class TestParsedGlobalsPassthrough: - def test_error_handler_receives_parsed_globals_from_clidriver(self): - session = FakeSession() parsed_globals = argparse.Namespace() @@ -339,9 +336,7 @@ def test_error_handler_receives_parsed_globals_from_clidriver(self): assert rc == CLIENT_ERROR_RC stderr_output = stderr.getvalue() - assert ( - '"Code"' in stderr_output or '"code"' in stderr_output.lower() - ) + assert '"Code"' in stderr_output or '"code"' in stderr_output.lower() assert 'NoSuchBucket' in stderr_output assert 'test-bucket' in stderr_output From 64a79ebdc034a43e400ec51411764de5121bc692 Mon Sep 17 00:00:00 2001 From: Andrew Asseily Date: Wed, 17 Dec 2025 11:34:47 -0500 Subject: [PATCH 17/26] Refactor error handling: simplify logic, add constants --- awscli/clidriver.py | 20 +++----- awscli/errorhandler.py | 104 ++++++++++++++++++++--------------------- 2 files changed, 58 insertions(+), 66 deletions(-) diff --git a/awscli/clidriver.py b/awscli/clidriver.py index 1c5f7553bdda..5d14005fe344 100644 --- a/awscli/clidriver.py +++ b/awscli/clidriver.py @@ -1046,19 +1046,13 @@ def invoke(self, service_name, operation_name, parameters, parsed_globals): def _make_client_call( self, client, operation_name, parameters, parsed_globals ): - 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: - raise + 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 def _display_response(self, command_name, response, parsed_globals): output = parsed_globals.output diff --git a/awscli/errorhandler.py b/awscli/errorhandler.py index 6c46f819db91..3dd0f221ae49 100644 --- a/awscli/errorhandler.py +++ b/awscli/errorhandler.py @@ -43,6 +43,8 @@ LOG = logging.getLogger(__name__) VALID_ERROR_FORMATS = ['legacy', 'json', 'yaml', 'text', 'table', 'enhanced'] +# Maximum number of items to display inline for collections +MAX_INLINE_ITEMS = 5 class EnhancedErrorFormatter: @@ -87,11 +89,11 @@ def _is_simple_value(self, value): def _is_small_collection(self, value): if isinstance(value, list): - return len(value) < 5 and all( + return len(value) < MAX_INLINE_ITEMS and all( self._is_simple_value(item) for item in value ) elif isinstance(value, dict): - return len(value) < 5 and all( + return len(value) < MAX_INLINE_ITEMS and all( self._is_simple_value(v) for v in value.values() ) return False @@ -179,7 +181,7 @@ class ClientErrorHandler(FilteredExceptionHandler): def __init__(self, session=None, parsed_globals=None): self._session = session self._parsed_globals = parsed_globals - self._enhanced_formatter = EnhancedErrorFormatter() + self._enhanced_formatter = None def _do_handle_exception(self, exception, stdout, stderr): displayed_structured = False @@ -194,72 +196,68 @@ def _do_handle_exception(self, exception, stdout, stderr): return self.RC def _resolve_error_format(self, parsed_globals): - error_format = ( - getattr(parsed_globals, 'cli_error_format', None) - if parsed_globals - else None - ) - - if error_format is None: - try: - error_format = self._session.get_config_variable( - 'cli_error_format' - ) - except (KeyError, AttributeError): - error_format = None - - if error_format is None: - error_format = 'enhanced' + if parsed_globals: + error_format = getattr(parsed_globals, 'cli_error_format', None) + if error_format: + return error_format.lower() + try: + error_format = self._session.get_config_variable( + 'cli_error_format' + ) + if error_format: + return error_format.lower() + except (KeyError, AttributeError): + pass - return error_format.lower() + return 'enhanced' def _try_display_structured_error(self, exception, stderr): try: error_response = self._extract_error_response(exception) + if not error_response or 'Error' not in error_response: + return False - if error_response and 'Error' in error_response: - error_info = error_response['Error'] + error_info = error_response['Error'] + error_format = self._resolve_error_format(self._parsed_globals) - parsed_globals = self._parsed_globals + if error_format not in VALID_ERROR_FORMATS: + LOG.warning( + f"Invalid cli_error_format: '{error_format}'. " + f"Using 'enhanced' format." + ) + error_format = 'enhanced' - error_format = self._resolve_error_format(parsed_globals) + if error_format == 'legacy': + return False - if error_format not in VALID_ERROR_FORMATS: - raise ValueError( - f"Invalid value for cli_error_format: '{error_format}'\n" - f"Valid choices are: {', '.join(VALID_ERROR_FORMATS)}" - ) + formatted_message = str(exception) - formatted_message = str(exception) + if error_format == 'enhanced': + if self._enhanced_formatter is None: + self._enhanced_formatter = EnhancedErrorFormatter() + self._enhanced_formatter.format_error( + error_info, formatted_message, stderr + ) + return True + + temp_parsed_globals = argparse.Namespace() + temp_parsed_globals.output = error_format + temp_parsed_globals.query = None + temp_parsed_globals.color = ( + getattr(self._parsed_globals, 'color', 'auto') + if self._parsed_globals + else 'auto' + ) - if error_format == 'legacy': - return False - elif error_format == 'enhanced': - self._enhanced_formatter.format_error( - error_info, formatted_message, stderr - ) - return True - else: - temp_parsed_globals = argparse.Namespace() - temp_parsed_globals.output = error_format - temp_parsed_globals.query = None - temp_parsed_globals.color = ( - getattr(parsed_globals, 'color', 'auto') - if parsed_globals - else 'auto' - ) + formatter = get_formatter(error_format, temp_parsed_globals) + formatter('error', error_info, stderr) + return True - formatter = get_formatter( - error_format, temp_parsed_globals - ) - formatter('error', error_info, stderr) - return True except Exception as e: LOG.debug( 'Failed to display structured error: %s', e, exc_info=True ) - - return False + return False @staticmethod def _extract_error_response(exception): From a7548f6ddc49332a3c6721defab7ceb4dae3e9b1 Mon Sep 17 00:00:00 2001 From: Andrew Asseily Date: Wed, 17 Dec 2025 11:53:33 -0500 Subject: [PATCH 18/26] Update documentation to include --cli-error-format global option --- awscli/errorhandler.py | 9 +++++---- awscli/examples/global_options.rst | 2 ++ awscli/examples/global_synopsis.rst | 1 + awscli/formatter.py | 7 +++++-- 4 files changed, 13 insertions(+), 6 deletions(-) diff --git a/awscli/errorhandler.py b/awscli/errorhandler.py index 3dd0f221ae49..dba7057e2ae1 100644 --- a/awscli/errorhandler.py +++ b/awscli/errorhandler.py @@ -49,8 +49,7 @@ class EnhancedErrorFormatter: def format_error(self, error_info, formatted_message, stream): - stream.write(formatted_message) - stream.write('\n') + stream.write(f'{formatted_message}\n') additional_fields = self._get_additional_fields(error_info) @@ -100,10 +99,12 @@ def _is_small_collection(self, value): def _format_inline(self, value): if isinstance(value, list): - return '[' + ', '.join(str(item) for item in value) + ']' + items_str = ', '.join(str(item) for item in value) + return f'[{items_str}]' elif isinstance(value, dict): items = [f'{k}: {v}' for k, v in value.items()] - return '{' + ', '.join(items) + '}' + items_str = ', '.join(items) + return f'{{{items_str}}}' return str(value) def _get_additional_fields(self, error_info): diff --git a/awscli/examples/global_options.rst b/awscli/examples/global_options.rst index 1495b0bc7b8e..20354555fe3c 100644 --- a/awscli/examples/global_options.rst +++ b/awscli/examples/global_options.rst @@ -114,3 +114,5 @@ * table * enhanced + + diff --git a/awscli/examples/global_synopsis.rst b/awscli/examples/global_synopsis.rst index 1ca332c717ff..3e603348debb 100644 --- a/awscli/examples/global_synopsis.rst +++ b/awscli/examples/global_synopsis.rst @@ -16,3 +16,4 @@ [--no-cli-pager] [--cli-auto-prompt] [--no-cli-auto-prompt] +[--cli-error-format ] diff --git a/awscli/formatter.py b/awscli/formatter.py index 903109b07afa..6cbd46369081 100644 --- a/awscli/formatter.py +++ b/awscli/formatter.py @@ -374,8 +374,11 @@ class OffFormatter(Formatter): """ def __call__(self, command_name, response, stream=None): - # Suppress all output - pass + if is_response_paginated(response): + for page in response: + self._get_transformed_response_for_output(page) + else: + self._get_transformed_response_for_output(response) CLI_OUTPUT_FORMATS = { From 3c14ea44a7380af25dc1cc2014593f50adf1233c Mon Sep 17 00:00:00 2001 From: Andrew Asseily Date: Wed, 17 Dec 2025 12:09:04 -0500 Subject: [PATCH 19/26] Fix legacy format test to verify modeled fields are not displayed --- awscli/errorhandler.py | 4 ++-- tests/unit/test_structured_error.py | 10 ++++------ 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/awscli/errorhandler.py b/awscli/errorhandler.py index dba7057e2ae1..1b157ca85577 100644 --- a/awscli/errorhandler.py +++ b/awscli/errorhandler.py @@ -66,7 +66,7 @@ def format_error(self, error_info, formatted_message, stream): else: stream.write( f'{key}: \n' - f'(Use --error-format json or --error-format yaml ' + f'(Use --cli-error-format json or --cli-error-format yaml ' f'to see full details)\n' ) else: @@ -79,7 +79,7 @@ def format_error(self, error_info, formatted_message, stream): else: stream.write( f' {key}: \n' - f' (Use --error-format json or --error-format yaml ' + f' (Use --cli-error-format json or --cli-error-format yaml ' f'to see full details)\n' ) diff --git a/tests/unit/test_structured_error.py b/tests/unit/test_structured_error.py index 2a2e113f3734..7b9f97013cbe 100644 --- a/tests/unit/test_structured_error.py +++ b/tests/unit/test_structured_error.py @@ -108,9 +108,7 @@ def test_respects_legacy_format_config(self): } client_error = ClientError(error_response, 'GetObject') - self.session.config_store.set_config_provider( - 'cli_error_format', mock.Mock(provide=lambda: 'legacy') - ) + self.session.session_vars['cli_error_format'] = 'legacy' stdout = io.StringIO() stderr = io.StringIO() @@ -119,8 +117,8 @@ def test_respects_legacy_format_config(self): assert rc == CLIENT_ERROR_RC stderr_output = stderr.getvalue() - # Legacy format should not show structured fields assert 'NoSuchBucket' in stderr_output + assert 'BucketName' not in stderr_output def test_error_format_case_insensitive(self): """Test that error format config is case-insensitive.""" @@ -301,8 +299,8 @@ def test_format_error_with_complex_object(self): assert formatted_message in output assert 'Additional error details' not in output assert 'Details: ' in output - assert '--error-format json' in output - assert '--error-format yaml' in output + assert '--cli-error-format json' in output + assert '--cli-error-format yaml' in output class TestParsedGlobalsPassthrough: From 408c0c6775eb6aa0f188eb0155428c057cbe91fa Mon Sep 17 00:00:00 2001 From: Andrew Asseily Date: Wed, 17 Dec 2025 13:34:16 -0500 Subject: [PATCH 20/26] Fix error handler double construction and improve error format messaging --- awscli/clidriver.py | 10 ++++++---- awscli/errorhandler.py | 22 +++++++++++----------- awscli/topics/config-vars.rst | 18 +++++++++++++----- 3 files changed, 30 insertions(+), 20 deletions(-) diff --git a/awscli/clidriver.py b/awscli/clidriver.py index 5d14005fe344..f14e18aba8cd 100644 --- a/awscli/clidriver.py +++ b/awscli/clidriver.py @@ -246,10 +246,6 @@ def __init__(self, session=None, error_handler=None, debug=False): else: self.session = session self._error_handler = error_handler - if self._error_handler is None: - self._error_handler = construct_cli_error_handlers_chain( - self.session - ) if debug: self._set_logging(debug) self._update_config_chain() @@ -536,6 +532,12 @@ def main(self, args=None): command_table = self._get_command_table() parser = self.create_parser(command_table) self._add_aliases(command_table, parser) + + if self._error_handler is None: + self._error_handler = construct_cli_error_handlers_chain( + self.session + ) + try: # Because _handle_top_level_args emits events, it's possible # that exceptions can be raised, which should have the same diff --git a/awscli/errorhandler.py b/awscli/errorhandler.py index 1b157ca85577..549d9effecbc 100644 --- a/awscli/errorhandler.py +++ b/awscli/errorhandler.py @@ -66,8 +66,8 @@ def format_error(self, error_info, formatted_message, stream): else: stream.write( f'{key}: \n' - f'(Use --cli-error-format json or --cli-error-format yaml ' - f'to see full details)\n' + f'(Use --cli-error-format with json, yaml, text, ' + f'or table to see full details)\n' ) else: stream.write('\nAdditional error details:\n') @@ -79,8 +79,8 @@ def format_error(self, error_info, formatted_message, stream): else: stream.write( f' {key}: \n' - f' (Use --cli-error-format json or --cli-error-format yaml ' - f'to see full details)\n' + f' (Use --cli-error-format with json, yaml, ' + f'text, or table to see full details)\n' ) def _is_simple_value(self, value): @@ -99,12 +99,10 @@ def _is_small_collection(self, value): def _format_inline(self, value): if isinstance(value, list): - items_str = ', '.join(str(item) for item in value) - return f'[{items_str}]' + return f"[{', '.join(str(item) for item in value)}]" elif isinstance(value, dict): - items = [f'{k}: {v}' for k, v in value.items()] - items_str = ', '.join(items) - return f'{{{items_str}}}' + items = ', '.join(f'{k}: {v}' for k, v in value.items()) + return f'{{{items}}}' return str(value) def _get_additional_fields(self, error_info): @@ -207,8 +205,10 @@ def _resolve_error_format(self, parsed_globals): ) if error_format: return error_format.lower() - except (KeyError, AttributeError): - pass + except (KeyError, AttributeError) as e: + LOG.debug( + 'Failed to get cli_error_format from config: %s', e + ) return 'enhanced' diff --git a/awscli/topics/config-vars.rst b/awscli/topics/config-vars.rst index 192c848cfca1..24853b85aa53 100644 --- a/awscli/topics/config-vars.rst +++ b/awscli/topics/config-vars.rst @@ -104,11 +104,19 @@ errors and warnings. ``cli_error_format`` controls how AWS service errors are displayed. The valid values of the ``cli_error_format`` configuration variable are: -* STANDARD - AWS service errors containing modeled fields beyond Code and - Message are written to stderr in the configured output format. This is the - default behavior. -* LEGACY - AWS service errors are written to stderr as unstructured text, - displaying only the error code and message. +* enhanced - AWS service errors display additional modeled fields in a + human-readable format with inline display for simple values and small + collections. This is the default behavior. +* json - AWS service errors are formatted as JSON, showing all modeled fields + in the error response. +* yaml - AWS service errors are formatted as YAML, showing all modeled fields + in the error response. +* text - AWS service errors are formatted as text with key-value pairs, showing + all modeled fields in the error response. +* table - AWS service errors are formatted as a table, showing all modeled + fields in the error response. +* legacy - AWS service errors are written to stderr as unstructured text, + displaying only the error code and message without additional modeled fields. ``cli_timestamp_format`` controls the format of timestamps displayed by the AWS CLI. The valid values of the ``cli_timestamp_format`` configuration variable are: From 8c779252badaa54bc8188c692a3f18c1f3ac75b4 Mon Sep 17 00:00:00 2001 From: Andrew Asseily Date: Wed, 17 Dec 2025 17:41:05 -0500 Subject: [PATCH 21/26] Extract service-specific error fields from top-level response --- awscli/errorhandler.py | 20 ++- tests/unit/test_structured_error.py | 217 +++++++++++++++++++++++++++- 2 files changed, 230 insertions(+), 7 deletions(-) diff --git a/awscli/errorhandler.py b/awscli/errorhandler.py index 549d9effecbc..5ebc8734041d 100644 --- a/awscli/errorhandler.py +++ b/awscli/errorhandler.py @@ -66,8 +66,8 @@ def format_error(self, error_info, formatted_message, stream): else: stream.write( f'{key}: \n' - f'(Use --cli-error-format with json, yaml, text, ' - f'or table to see full details)\n' + f'(Use --cli-error-format with json, yaml, ' + f'or text to see full details)\n' ) else: stream.write('\nAdditional error details:\n') @@ -80,7 +80,7 @@ def format_error(self, error_info, formatted_message, stream): stream.write( f' {key}: \n' f' (Use --cli-error-format with json, yaml, ' - f'text, or table to see full details)\n' + f'or text to see full details)\n' ) def _is_simple_value(self, value): @@ -266,7 +266,19 @@ def _extract_error_response(exception): return None if hasattr(exception, 'response') and 'Error' in exception.response: - return {'Error': exception.response['Error']} + 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. + 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 + + return {'Error': error_dict} return None diff --git a/tests/unit/test_structured_error.py b/tests/unit/test_structured_error.py index 7b9f97013cbe..9f19ee9acaf3 100644 --- a/tests/unit/test_structured_error.py +++ b/tests/unit/test_structured_error.py @@ -16,7 +16,6 @@ from botocore.exceptions import ClientError -from awscli.clidriver import CLIDriver from awscli.constants import CLIENT_ERROR_RC from awscli.errorhandler import ( ClientErrorHandler, @@ -49,6 +48,35 @@ def test_extract_error_response_from_client_error(self): assert result['Error']['Code'] == 'NoSuchBucket' assert result['Error']['BucketName'] == 'my-bucket' + def test_extract_error_response_with_top_level_fields(self): + error_response = { + 'Error': { + 'Code': 'TransactionCanceledException', + 'Message': 'Transaction cancelled', + }, + 'CancellationReasons': [ + { + 'Code': 'ConditionalCheckFailed', + 'Message': 'The conditional request failed', + 'Item': {'id': {'S': '123'}}, + } + ], + 'ResponseMetadata': {'RequestId': '456'}, + } + client_error = ClientError(error_response, 'TransactWriteItems') + + result = ClientErrorHandler._extract_error_response(client_error) + + assert result is not None + assert 'Error' in result + assert result['Error']['Code'] == 'TransactionCanceledException' + assert 'CancellationReasons' in result['Error'] + assert len(result['Error']['CancellationReasons']) == 1 + assert ( + result['Error']['CancellationReasons'][0]['Code'] + == 'ConditionalCheckFailed' + ) + def test_extract_error_response_from_non_client_error(self): result = ClientErrorHandler._extract_error_response( ValueError('Some error') @@ -299,8 +327,191 @@ def test_format_error_with_complex_object(self): assert formatted_message in output assert 'Additional error details' not in output assert 'Details: ' in output - assert '--cli-error-format json' in output - assert '--cli-error-format yaml' in output + assert '--cli-error-format with json, yaml, or text' in output + + def test_format_error_with_nested_dict(self): + """Test formatting with nested dictionary structures.""" + error_info = { + 'Code': 'ValidationError', + 'Message': 'Validation failed', + 'FieldErrors': { + 'email': {'pattern': 'invalid', 'required': True}, + 'age': {'min': 0, 'max': 120}, + }, + } + formatted_message = ( + 'An error occurred (ValidationError): Validation failed' + ) + + stream = io.StringIO() + self.formatter.format_error(error_info, formatted_message, stream) + + output = stream.getvalue() + assert formatted_message in output + assert 'FieldErrors: ' in output + + def test_format_error_with_list_of_dicts(self): + """Test formatting with list containing dictionaries.""" + error_info = { + 'Code': 'TransactionCanceledException', + 'Message': 'Transaction cancelled', + 'CancellationReasons': [ + {'Code': 'ConditionalCheckFailed', 'Message': 'Check failed'}, + {'Code': 'ItemCollectionSizeLimitExceeded', 'Message': 'Too large'}, + ], + } + formatted_message = ( + 'An error occurred (TransactionCanceledException): ' + 'Transaction cancelled' + ) + + stream = io.StringIO() + self.formatter.format_error(error_info, formatted_message, stream) + + output = stream.getvalue() + assert formatted_message in output + assert 'CancellationReasons: ' in output + + def test_format_error_with_mixed_types(self): + """Test formatting with various data types.""" + error_info = { + 'Code': 'ComplexError', + 'Message': 'Complex error occurred', + 'StringField': 'test-value', + 'IntField': 42, + 'FloatField': 3.14, + 'BoolField': True, + 'NoneField': None, + } + formatted_message = ( + 'An error occurred (ComplexError): Complex error occurred' + ) + + stream = io.StringIO() + self.formatter.format_error(error_info, formatted_message, stream) + + output = stream.getvalue() + assert formatted_message in output + assert 'Additional error details' in output + assert 'StringField: test-value' in output + assert 'IntField: 42' in output + assert 'FloatField: 3.14' in output + assert 'BoolField: True' in output + assert 'NoneField: None' in output + + def test_format_error_with_unicode_and_special_chars(self): + """Test formatting with unicode and special characters.""" + error_info = { + 'Code': 'InvalidInput', + 'Message': 'Invalid input provided', + 'UserName': 'José García', + 'Description': 'Error with "quotes" and \'apostrophes\'', + 'Path': '/path/to/file.txt', + } + formatted_message = ( + 'An error occurred (InvalidInput): Invalid input provided' + ) + + stream = io.StringIO() + self.formatter.format_error(error_info, formatted_message, stream) + + output = stream.getvalue() + assert formatted_message in output + assert 'José García' in output + assert 'quotes' in output + assert 'apostrophes' in output + + def test_format_error_with_large_list(self): + """Test that large lists are marked as complex.""" + error_info = { + 'Code': 'LargeList', + 'Message': 'Large list error', + 'Items': list(range(10)), + } + formatted_message = ( + 'An error occurred (LargeList): Large list error' + ) + + stream = io.StringIO() + self.formatter.format_error(error_info, formatted_message, stream) + + output = stream.getvalue() + assert formatted_message in output + assert 'Items: ' in output + + +class TestRealWorldErrorScenarios: + """Test real-world AWS error response structures.""" + + def setup_method(self): + self.session = FakeSession() + self.handler = ClientErrorHandler(self.session) + + def test_dynamodb_transaction_cancelled_error(self): + """Test DynamoDB TransactionCanceledException with CancellationReasons.""" + error_response = { + 'Error': { + 'Code': 'TransactionCanceledException', + 'Message': ( + 'Transaction cancelled, please refer to ' + 'CancellationReasons for specific reasons' + ), + }, + 'CancellationReasons': [ + { + 'Code': 'ConditionalCheckFailed', + 'Message': 'The conditional request failed', + 'Item': { + 'id': {'S': 'item-123'}, + 'status': {'S': 'active'}, + }, + }, + { + 'Code': 'None', + 'Message': None, + }, + ], + 'ResponseMetadata': { + 'RequestId': 'abc-123', + 'HTTPStatusCode': 400, + }, + } + client_error = ClientError(error_response, 'TransactWriteItems') + + stdout = io.StringIO() + stderr = io.StringIO() + + rc = self.handler.handle_exception(client_error, stdout, stderr) + + assert rc == CLIENT_ERROR_RC + stderr_output = stderr.getvalue() + assert 'TransactionCanceledException' in stderr_output + assert 'CancellationReasons' in stderr_output + + def test_throttling_error_with_retry_info(self): + """Test throttling error with retry information.""" + error_response = { + 'Error': { + 'Code': 'ThrottlingException', + 'Message': 'Rate exceeded', + }, + 'RetryAfterSeconds': 30, + 'RequestsPerSecond': 100, + 'CurrentRate': 150, + 'ResponseMetadata': {'RequestId': 'throttle-123'}, + } + client_error = ClientError(error_response, 'DescribeInstances') + + stdout = io.StringIO() + stderr = io.StringIO() + + rc = self.handler.handle_exception(client_error, stdout, stderr) + + assert rc == CLIENT_ERROR_RC + stderr_output = stderr.getvalue() + assert 'ThrottlingException' in stderr_output + assert '30' in stderr_output + assert '100' in stderr_output class TestParsedGlobalsPassthrough: From 8a4c74b19aeca93cc004055738dbb12fdc6c1325 Mon Sep 17 00:00:00 2001 From: Andrew Asseily Date: Thu, 18 Dec 2025 11:16:11 -0500 Subject: [PATCH 22/26] Remove text format from complex value suggestions due to poor array handling --- awscli/errorhandler.py | 8 ++++---- tests/unit/test_structured_error.py | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/awscli/errorhandler.py b/awscli/errorhandler.py index 5ebc8734041d..81835355550f 100644 --- a/awscli/errorhandler.py +++ b/awscli/errorhandler.py @@ -66,8 +66,8 @@ def format_error(self, error_info, formatted_message, stream): else: stream.write( f'{key}: \n' - f'(Use --cli-error-format with json, yaml, ' - f'or text to see full details)\n' + f'(Use --cli-error-format with json or yaml ' + f'to see full details)\n' ) else: stream.write('\nAdditional error details:\n') @@ -79,8 +79,8 @@ def format_error(self, error_info, formatted_message, stream): else: stream.write( f' {key}: \n' - f' (Use --cli-error-format with json, yaml, ' - f'or text to see full details)\n' + f' (Use --cli-error-format with json or yaml ' + f'to see full details)\n' ) def _is_simple_value(self, value): diff --git a/tests/unit/test_structured_error.py b/tests/unit/test_structured_error.py index 9f19ee9acaf3..b80e8338ecd6 100644 --- a/tests/unit/test_structured_error.py +++ b/tests/unit/test_structured_error.py @@ -327,7 +327,7 @@ def test_format_error_with_complex_object(self): assert formatted_message in output assert 'Additional error details' not in output assert 'Details: ' in output - assert '--cli-error-format with json, yaml, or text' in output + assert '--cli-error-format with json or yaml' in output def test_format_error_with_nested_dict(self): """Test formatting with nested dictionary structures.""" From aa19e40919e966a170c1875daaacb41817e0ce17 Mon Sep 17 00:00:00 2001 From: Andrew Asseily Date: Thu, 18 Dec 2025 11:23:35 -0500 Subject: [PATCH 23/26] Simplified the format_error method to always use consistent formatting --- awscli/errorhandler.py | 38 +++++++++-------------------- tests/unit/test_structured_error.py | 6 ++--- 2 files changed, 15 insertions(+), 29 deletions(-) diff --git a/awscli/errorhandler.py b/awscli/errorhandler.py index 81835355550f..2cdf759b938f 100644 --- a/awscli/errorhandler.py +++ b/awscli/errorhandler.py @@ -56,32 +56,18 @@ def format_error(self, error_info, formatted_message, stream): if not additional_fields: return - if len(additional_fields) == 1: - stream.write('\n') - for key, value in additional_fields.items(): - if self._is_simple_value(value): - stream.write(f'{key}: {value}\n') - elif self._is_small_collection(value): - stream.write(f'{key}: {self._format_inline(value)}\n') - else: - stream.write( - f'{key}: \n' - f'(Use --cli-error-format with json or yaml ' - f'to see full details)\n' - ) - else: - stream.write('\nAdditional error details:\n') - for key, value in additional_fields.items(): - if self._is_simple_value(value): - stream.write(f' {key}: {value}\n') - elif self._is_small_collection(value): - stream.write(f' {key}: {self._format_inline(value)}\n') - else: - stream.write( - f' {key}: \n' - f' (Use --cli-error-format with json or yaml ' - f'to see full details)\n' - ) + stream.write('\nAdditional error details:\n') + for key, value in additional_fields.items(): + if self._is_simple_value(value): + stream.write(f'{key}: {value}\n') + elif self._is_small_collection(value): + stream.write(f'{key}: {self._format_inline(value)}\n') + else: + stream.write( + f'{key}: \n' + f'(Use --cli-error-format with json or yaml ' + f'to see full details)\n' + ) def _is_simple_value(self, value): return isinstance(value, (str, int, float, bool, type(None))) diff --git a/tests/unit/test_structured_error.py b/tests/unit/test_structured_error.py index b80e8338ecd6..cdd811daa550 100644 --- a/tests/unit/test_structured_error.py +++ b/tests/unit/test_structured_error.py @@ -287,7 +287,7 @@ def test_format_error_with_small_list(self): output = stream.getvalue() assert formatted_message in output - assert 'Additional error details' not in output + assert 'Additional error details' in output assert 'AllowedValues: [value1, value2, value3]' in output def test_format_error_with_small_dict(self): @@ -305,7 +305,7 @@ def test_format_error_with_small_dict(self): output = stream.getvalue() assert formatted_message in output - assert 'Additional error details' not in output + assert 'Additional error details' in output assert 'Metadata:' in output assert 'key1: value1' in output assert 'key2: value2' in output @@ -325,7 +325,7 @@ def test_format_error_with_complex_object(self): output = stream.getvalue() assert formatted_message in output - assert 'Additional error details' not in output + assert 'Additional error details' in output assert 'Details: ' in output assert '--cli-error-format with json or yaml' in output From b4db2aeed29388263efd78ae93f4238b3da02aef Mon Sep 17 00:00:00 2001 From: Andrew Asseily Date: Thu, 18 Dec 2025 11:30:58 -0500 Subject: [PATCH 24/26] Add the new error format to the general options table in config-vars --- awscli/topics/config-vars.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/awscli/topics/config-vars.rst b/awscli/topics/config-vars.rst index 24853b85aa53..28828bc88fba 100644 --- a/awscli/topics/config-vars.rst +++ b/awscli/topics/config-vars.rst @@ -82,6 +82,9 @@ max_attempts N/A max_attempts AWS_MAX_ATTEMPTS retry_mode N/A retry_mode AWS_RETRY_MODE Type of retries performed -------------------- -------------- --------------------- --------------------- -------------------------------- cli_pager --no-cli-pager cli_pager AWS_PAGER Redirect/Disable output to pager +-------------------- -------------- --------------------- --------------------- -------------------------------- +cli_error_format --cli-error- cli_error_format AWS_CLI_ERROR_FORMAT Format for error output + format ==================== ============== ===================== ===================== ================================ The third column, Config Entry, is the value you would specify in the AWS CLI From 4232a974269eb8a8fa004098f65d4705140bb1a8 Mon Sep 17 00:00:00 2001 From: Andrew Asseily Date: Thu, 18 Dec 2025 13:12:40 -0500 Subject: [PATCH 25/26] Restore error handler fallback --- awscli/clidriver.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/awscli/clidriver.py b/awscli/clidriver.py index f14e18aba8cd..a049f03e0c03 100644 --- a/awscli/clidriver.py +++ b/awscli/clidriver.py @@ -246,6 +246,10 @@ def __init__(self, session=None, error_handler=None, debug=False): else: self.session = session self._error_handler = error_handler + if self._error_handler is None: + self._error_handler = construct_cli_error_handlers_chain( + self.session + ) if debug: self._set_logging(debug) self._update_config_chain() @@ -533,11 +537,6 @@ def main(self, args=None): parser = self.create_parser(command_table) self._add_aliases(command_table, parser) - if self._error_handler is None: - self._error_handler = construct_cli_error_handlers_chain( - self.session - ) - try: # Because _handle_top_level_args emits events, it's possible # that exceptions can be raised, which should have the same From e73a257e8ed5606f233f8eb61dde15f8dcd12c85 Mon Sep 17 00:00:00 2001 From: Andrew Asseily Date: Tue, 30 Dec 2025 09:38:54 -0500 Subject: [PATCH 26/26] refactor: pass parsed_globals at exception handling time --- awscli/clidriver.py | 12 +++++---- awscli/errorhandler.py | 40 +++++++++++++++++------------ tests/unit/test_structured_error.py | 29 ++++++++++++++------- 3 files changed, 51 insertions(+), 30 deletions(-) diff --git a/awscli/clidriver.py b/awscli/clidriver.py index a049f03e0c03..bee766b2a614 100644 --- a/awscli/clidriver.py +++ b/awscli/clidriver.py @@ -536,16 +536,14 @@ def main(self, args=None): command_table = self._get_command_table() parser = self.create_parser(command_table) self._add_aliases(command_table, parser) - + parsed_globals = None try: # Because _handle_top_level_args emits events, it's possible # that exceptions can be raised, which should have the same # general exception handling logic as calling into the # command table. This is why it's in the try/except clause. parsed_args, remaining = parser.parse_known_args(args) - self._error_handler = construct_cli_error_handlers_chain( - self.session, parsed_args - ) + parsed_globals = parsed_args self._handle_top_level_args(parsed_args) validate_preferred_output_encoding() self._emit_session_event(parsed_args) @@ -562,6 +560,7 @@ def main(self, args=None): e, stdout=get_stdout_text_writer(), stderr=get_stderr_text_writer(), + parsed_globals=parsed_globals, ) def _emit_session_event(self, parsed_args): @@ -842,7 +841,10 @@ def _parse_potential_subcommand(self, args, subcommand_table): def __call__(self, args, parsed_globals): # Once we know we're trying to call a particular operation # of a service we can go ahead and load the parameters. - event = f'before-building-argument-table-parser.{self._parent_name}.{self._name}' + event = ( + f'before-building-argument-table-parser.' + f'{self._parent_name}.{self._name}' + ) self._emit( event, argument_table=self.arg_table, diff --git a/awscli/errorhandler.py b/awscli/errorhandler.py index 2cdf759b938f..5902f7e45dcc 100644 --- a/awscli/errorhandler.py +++ b/awscli/errorhandler.py @@ -106,7 +106,7 @@ def construct_entry_point_handlers_chain(): return ChainedExceptionHandler(exception_handlers=handlers) -def construct_cli_error_handlers_chain(session=None, parsed_globals=None): +def construct_cli_error_handlers_chain(session=None): handlers = [ ParamValidationErrorsHandler(), UnknownArgumentErrorHandler(), @@ -115,7 +115,7 @@ def construct_cli_error_handlers_chain(session=None, parsed_globals=None): NoCredentialsErrorHandler(), PagerErrorHandler(), InterruptExceptionHandler(), - ClientErrorHandler(session, parsed_globals), + ClientErrorHandler(session), GeneralExceptionHandler(), ] return ChainedExceptionHandler(exception_handlers=handlers) @@ -130,13 +130,15 @@ class FilteredExceptionHandler(BaseExceptionHandler): EXCEPTIONS_TO_HANDLE = () MESSAGE = '%s' - def handle_exception(self, exception, stdout, stderr): + def handle_exception(self, exception, stdout, stderr, **kwargs): if isinstance(exception, self.EXCEPTIONS_TO_HANDLE): - return_val = self._do_handle_exception(exception, stdout, stderr) + return_val = self._do_handle_exception( + exception, stdout, stderr, **kwargs + ) if return_val is not None: return return_val - def _do_handle_exception(self, exception, stdout, stderr): + def _do_handle_exception(self, exception, stdout, stderr, **kwargs): stderr.write("\n") stderr.write(self.MESSAGE % exception) stderr.write("\n") @@ -163,20 +165,22 @@ class ClientErrorHandler(FilteredExceptionHandler): EXCEPTIONS_TO_HANDLE = ClientError RC = CLIENT_ERROR_RC - def __init__(self, session=None, parsed_globals=None): + def __init__(self, session=None): self._session = session - self._parsed_globals = parsed_globals self._enhanced_formatter = None - def _do_handle_exception(self, exception, stdout, stderr): + def _do_handle_exception(self, exception, stdout, stderr, **kwargs): + parsed_globals = kwargs.get('parsed_globals') displayed_structured = False if self._session: displayed_structured = self._try_display_structured_error( - exception, stderr + exception, stderr, parsed_globals ) if not displayed_structured: - return super()._do_handle_exception(exception, stdout, stderr) + return super()._do_handle_exception( + exception, stdout, stderr, **kwargs + ) return self.RC @@ -198,14 +202,16 @@ def _resolve_error_format(self, parsed_globals): return 'enhanced' - def _try_display_structured_error(self, exception, stderr): + def _try_display_structured_error( + self, exception, stderr, parsed_globals=None + ): try: error_response = self._extract_error_response(exception) if not error_response or 'Error' not in error_response: return False error_info = error_response['Error'] - error_format = self._resolve_error_format(self._parsed_globals) + error_format = self._resolve_error_format(parsed_globals) if error_format not in VALID_ERROR_FORMATS: LOG.warning( @@ -231,8 +237,8 @@ def _try_display_structured_error(self, exception, stderr): temp_parsed_globals.output = error_format temp_parsed_globals.query = None temp_parsed_globals.color = ( - getattr(self._parsed_globals, 'color', 'auto') - if self._parsed_globals + getattr(parsed_globals, 'color', 'auto') + if parsed_globals else 'auto' ) @@ -340,8 +346,10 @@ def __init__(self, exception_handlers): def inject_handler(self, position, handler): self._exception_handlers.insert(position, handler) - def handle_exception(self, exception, stdout, stderr): + def handle_exception(self, exception, stdout, stderr, **kwargs): for handler in self._exception_handlers: - return_value = handler.handle_exception(exception, stdout, stderr) + return_value = handler.handle_exception( + exception, stdout, stderr, **kwargs + ) if return_value is not None: return return_value diff --git a/tests/unit/test_structured_error.py b/tests/unit/test_structured_error.py index cdd811daa550..594da647beda 100644 --- a/tests/unit/test_structured_error.py +++ b/tests/unit/test_structured_error.py @@ -356,8 +356,14 @@ def test_format_error_with_list_of_dicts(self): 'Code': 'TransactionCanceledException', 'Message': 'Transaction cancelled', 'CancellationReasons': [ - {'Code': 'ConditionalCheckFailed', 'Message': 'Check failed'}, - {'Code': 'ItemCollectionSizeLimitExceeded', 'Message': 'Too large'}, + { + 'Code': 'ConditionalCheckFailed', + 'Message': 'Check failed', + }, + { + 'Code': 'ItemCollectionSizeLimitExceeded', + 'Message': 'Too large', + }, ], } formatted_message = ( @@ -448,7 +454,10 @@ def setup_method(self): self.handler = ClientErrorHandler(self.session) def test_dynamodb_transaction_cancelled_error(self): - """Test DynamoDB TransactionCanceledException with CancellationReasons.""" + """ + Test DynamoDB TransactionCanceledException with + CancellationReasons. + """ error_response = { 'Error': { 'Code': 'TransactionCanceledException', @@ -523,9 +532,7 @@ def test_error_handler_receives_parsed_globals_from_clidriver(self): parsed_globals.command = 's3' parsed_globals.color = 'auto' - error_handler = construct_cli_error_handlers_chain( - session, parsed_globals - ) + error_handler = construct_cli_error_handlers_chain(session) error_response = { 'Error': { @@ -540,7 +547,9 @@ def test_error_handler_receives_parsed_globals_from_clidriver(self): stdout = io.StringIO() stderr = io.StringIO() - rc = error_handler.handle_exception(client_error, stdout, stderr) + rc = error_handler.handle_exception( + client_error, stdout, stderr, parsed_globals=parsed_globals + ) assert rc == CLIENT_ERROR_RC @@ -552,7 +561,7 @@ def test_error_handler_receives_parsed_globals_from_clidriver(self): def test_error_handler_without_parsed_globals_uses_default(self): session = FakeSession() - error_handler = construct_cli_error_handlers_chain(session, None) + error_handler = construct_cli_error_handlers_chain(session) error_response = { 'Error': { @@ -567,7 +576,9 @@ def test_error_handler_without_parsed_globals_uses_default(self): stdout = io.StringIO() stderr = io.StringIO() - rc = error_handler.handle_exception(client_error, stdout, stderr) + rc = error_handler.handle_exception( + client_error, stdout, stderr, parsed_globals=None + ) assert rc == CLIENT_ERROR_RC