-
Notifications
You must be signed in to change notification settings - Fork 4.5k
Structured Error Output #9890
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Structured Error Output #9890
Changes from 33 commits
1afe452
c46cf3e
6842e4e
3fe5630
bcc33fc
829968b
00213ca
181b94a
6958d85
9eef83f
e678480
329d167
5700443
073c28d
60b04ee
b82ef33
4b34b62
7f511a2
b7d7ebe
614b09a
5b0ce61
47b6129
9f04b48
c7f1a50
0493e92
ae6987d
dd7974e
3c5af8d
9db425e
e62ce3b
a31fb33
79e5322
6f0f981
a7e07b7
fa72088
e6347d1
7972caf
7ceeaa1
2cb1a04
8ac8385
1d0341a
f9c1256
4989c22
b6dddb4
5e8313d
28ab15f
1f517fa
e2598bf
6d9e747
439b186
bc25515
ce86991
ec56d0e
558c9f6
41c4e27
6e36a98
926c63a
af6a93c
0d754e9
29ee23c
e003f09
4f87cfa
64a79eb
a7548f6
3c14ea4
20923d1
408c0c6
b6a806a
dd8b2fc
08c42f9
8c77925
1800d25
8a4c74b
aa19e40
b4db2ae
5ad8a9f
4232a97
4f56944
3943d63
1df3abf
d5294d8
f83e308
aad20f6
5bc5025
dd99339
5aa7f84
4ed2eac
0c7a1d0
b7ccb23
4ddf51e
36d2350
bb51f3b
46ae82a
d6274b5
a9d62b4
af4ff69
12e3109
9a99e6b
c460a71
40a7626
e73a257
b21394b
60937ba
0adb050
00330f3
5fd6d51
4d427da
b04e3df
18b00bd
12aea15
f1d9604
9144bc4
55d333b
adcfb9e
71bc63a
cbfe40a
44df686
832392e
ff901d4
afcbf9a
31fbeeb
bd629ff
0a415e9
1047303
c527bd9
36f6e59
34bb9a7
a2cd4a7
57eec7c
39c4790
7aaac09
90df436
19e3c1b
8ad7287
0cdaf45
4b01307
099a3b7
5ca1c7d
d935f1d
6baf1ff
79aa0dd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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." | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| { | ||
| "type": "feature", | ||
| "category": "Output", | ||
| "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." | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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,10 +37,64 @@ | |
| ConfigurationError, | ||
| ParamValidationError, | ||
| ) | ||
| from awscli.formatter import get_formatter | ||
| from awscli.utils import PagerInitializationException | ||
|
|
||
| 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: | ||
| def format_error(self, error_info, formatted_message, stream): | ||
| stream.write(f'{formatted_message}\n') | ||
|
|
||
| additional_fields = self._get_additional_fields(error_info) | ||
|
|
||
| if not additional_fields: | ||
| return | ||
|
|
||
| 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') | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wondering if we should indent these under the "Additional error details header" Something like: I'm leaning towards yes, but not a strong opinion. Can maybe check with the internal team quickly.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good call, I debated this earlier on and went against indentation because I thought it didn't look as clean when there's only a single additional field. But I agree it's worth revisiting. I'll bring it up with the team internally and see what they think.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm okay either way, and it'd be safe to change post-launch too. |
||
| elif self._is_small_collection(value): | ||
| stream.write(f'{key}: {self._format_inline(value)}\n') | ||
| else: | ||
| stream.write( | ||
| f'{key}: <complex value>\n' | ||
AndrewAsseily marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| 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))) | ||
|
|
||
| def _is_small_collection(self, value): | ||
| if isinstance(value, list): | ||
| return len(value) < MAX_INLINE_ITEMS and all( | ||
| self._is_simple_value(item) for item in value | ||
| ) | ||
| elif isinstance(value, dict): | ||
| return len(value) < MAX_INLINE_ITEMS and all( | ||
| self._is_simple_value(v) for v in value.values() | ||
| ) | ||
| return False | ||
|
|
||
| def _format_inline(self, value): | ||
| if isinstance(value, list): | ||
| return f"[{', '.join(str(item) for item in value)}]" | ||
| elif isinstance(value, dict): | ||
| items = ', '.join(f'{k}: {v}' for k, v in value.items()) | ||
| return f'{{{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 = [ | ||
|
|
@@ -51,7 +106,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 +115,7 @@ def construct_cli_error_handlers_chain(): | |
| NoCredentialsErrorHandler(), | ||
| PagerErrorHandler(), | ||
| InterruptExceptionHandler(), | ||
| ClientErrorHandler(), | ||
| ClientErrorHandler(session), | ||
AndrewAsseily marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| GeneralExceptionHandler(), | ||
| ] | ||
| return ChainedExceptionHandler(exception_handlers=handlers) | ||
|
|
@@ -75,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") | ||
|
|
@@ -108,6 +165,115 @@ class ClientErrorHandler(FilteredExceptionHandler): | |
| EXCEPTIONS_TO_HANDLE = ClientError | ||
| RC = CLIENT_ERROR_RC | ||
|
|
||
| def __init__(self, session=None): | ||
| self._session = session | ||
| self._enhanced_formatter = None | ||
|
|
||
| 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, parsed_globals | ||
| ) | ||
|
|
||
| if not displayed_structured: | ||
| return super()._do_handle_exception( | ||
| exception, stdout, stderr, **kwargs | ||
| ) | ||
|
|
||
| return self.RC | ||
|
|
||
| def _resolve_error_format(self, parsed_globals): | ||
| 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) as e: | ||
| LOG.debug( | ||
| 'Failed to get cli_error_format from config: %s', e | ||
| ) | ||
|
|
||
| return 'enhanced' | ||
|
|
||
| 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(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' | ||
|
|
||
| if error_format == 'legacy': | ||
| return False | ||
|
|
||
| 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(parsed_globals, 'color', 'auto') | ||
| if parsed_globals | ||
| else 'auto' | ||
| ) | ||
AndrewAsseily marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| 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 | ||
|
|
||
| @staticmethod | ||
| def _extract_error_response(exception): | ||
| if not isinstance(exception, ClientError): | ||
| return None | ||
|
|
||
| if hasattr(exception, 'response') and 'Error' in exception.response: | ||
| error_dict = dict(exception.response['Error']) | ||
|
|
||
| # AWS services return modeled error fields | ||
| # at the top level of the error response, | ||
| # not nested under an Error key. Botocore preserves this structure. | ||
| # Include these fields to provide complete error information. | ||
| # Exclude response metadata and avoid duplicates. | ||
| 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 | ||
|
|
||
|
|
||
| class ConfigurationErrorHandler(FilteredExceptionHandler): | ||
| EXCEPTIONS_TO_HANDLE = ConfigurationError | ||
|
|
@@ -180,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 | ||
Uh oh!
There was an error while loading. Please reload this page.