Skip to content
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changes/next-release/bugfix-errorformatting-83208.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"type": "bugfix",
"category": "error formatting",
"description": "Error output now only displays modeled error fields in the 'Additional error details' section."
}
11 changes: 10 additions & 1 deletion awscli/botocore/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -903,8 +903,17 @@ def _make_api_call(self, operation_name, api_params):

if http.status_code >= 300:
error_code = parsed_response.get("Error", {}).get("Code")
# Lookup is a cached dict access; only runs on error path.
error_shape = self._service_model.shape_for_error_code(
error_code
)
modeled_fields = {'Code', 'Message'}
if error_shape:
modeled_fields |= set(error_shape.members.keys())
error_class = self.exceptions.from_code(error_code)
raise error_class(parsed_response, operation_name)
error = error_class(parsed_response, operation_name)
error.modeled_fields = modeled_fields
raise error
else:
return parsed_response

Expand Down
27 changes: 24 additions & 3 deletions awscli/errorhandler.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,12 @@ def _format_inline(self, value):
return str(value)

def _get_additional_fields(self, error_info):
standard_keys = {'Code', 'Message'}
return {k: v for k, v in error_info.items() if k not in standard_keys}
standard_keys = {'code', 'message'}
return {
k: v
for k, v in error_info.items()
if k.lower() not in standard_keys
}


def construct_entry_point_handlers_chain():
Expand Down Expand Up @@ -200,6 +204,20 @@ def _display_structured_error(
):
try:
error_format = self._resolve_error_format(parsed_globals)
modeled_fields = error_info.pop('_modeled_fields', None)

if modeled_fields is not None:
modeled_lower = {f.lower() for f in modeled_fields}
for key in list(error_info.keys()):
if key.lower() not in modeled_lower:
del error_info[key]
else:
# No model info available (e.g. manually constructed
# ClientError). Remove all non-standard fields.
standard = {'code', 'message'}
for key in list(error_info.keys()):
if key.lower() not in standard:
del error_info[key]

if error_format == 'legacy':
return False
Expand Down Expand Up @@ -275,7 +293,10 @@ def _get_formatted_message(self, error_info, exception):
def _extract_error_info(self, exception):
error_response = self._extract_error_response(exception)
if error_response and 'Error' in error_response:
return error_response['Error']
error_info = error_response['Error']
modeled_fields = getattr(exception, 'modeled_fields', None)
error_info['_modeled_fields'] = modeled_fields
return error_info
return None

@staticmethod
Expand Down
121 changes: 121 additions & 0 deletions tests/unit/test_structured_error.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ def test_displays_structured_error_with_additional_members(self):
'ResponseMetadata': {'RequestId': '123'},
}
client_error = ClientError(error_response, 'GetObject')
client_error.modeled_fields = {'Code', 'Message', 'BucketName'}

stdout = io.StringIO()
stderr = io.StringIO()
Expand Down Expand Up @@ -129,6 +130,7 @@ def test_error_format_case_insensitive(self):
'ResponseMetadata': {'RequestId': '123'},
}
client_error = ClientError(error_response, 'GetObject')
client_error.modeled_fields = {'Code', 'Message', 'BucketName'}

self.session.config_store.set_config_provider(
'cli_error_format', mock.Mock(provide=lambda: 'Enhanced')
Expand All @@ -150,6 +152,91 @@ def test_error_format_case_insensitive(self):
)
assert stderr.getvalue() == expected

def test_modeled_fields_filters_unmodeled_from_display(self):
error_response = {
'Error': {
'Code': 'ExpiredToken',
'Message': 'Token expired',
'Token-0': 'AQoDYXdzEJr...sensitive...',
},
'ResponseMetadata': {'RequestId': '123'},
}
client_error = ClientError(error_response, 'ListBuckets')
client_error.modeled_fields = {'Code', 'Message'}

stdout = io.StringIO()
stderr = io.StringIO()

rc = self.handler.handle_exception(client_error, stdout, stderr)

assert rc == CLIENT_ERROR_RC
assert 'Token-0' not in stderr.getvalue()
assert 'sensitive' not in stderr.getvalue()

def test_modeled_fields_not_leaked_in_json_format(self):
error_response = {
'Error': {
'Code': 'ExpiredToken',
'Message': 'Token expired',
'Token-0': 'AQoDYXdzEJr...sensitive...',
},
'ResponseMetadata': {'RequestId': '123'},
}
client_error = ClientError(error_response, 'ListBuckets')
client_error.modeled_fields = {'Code', 'Message'}

self.session.session_vars['cli_error_format'] = 'json'

stdout = io.StringIO()
stderr = io.StringIO()

self.handler.handle_exception(client_error, stdout, stderr)

parsed = json.loads(stderr.getvalue())
assert 'Token-0' not in parsed
assert '_modeled_fields' not in parsed
assert 'modeled_fields' not in parsed

def test_no_modeled_fields_hides_additional_fields(self):
# ClientError without modeled_fields attribute (e.g. manually
# constructed in customizations) should not show additional fields.
error_response = {
'Error': {
'Code': 'CustomError',
'Message': 'Something broke',
'Detail': 'extra info',
},
'ResponseMetadata': {'RequestId': '123'},
}
client_error = ClientError(error_response, 'DoThing')

stdout = io.StringIO()
stderr = io.StringIO()

self.handler.handle_exception(client_error, stdout, stderr)

assert 'Detail' not in stderr.getvalue()

def test_modeled_fields_case_insensitive_match(self):
# Model has 'ErrorCode' but response has 'errorCode' (or vice versa)
error_response = {
'Error': {
'Code': 'SomeError',
'Message': 'msg',
'errorcode': 'detail',
},
'ResponseMetadata': {'RequestId': '123'},
}
client_error = ClientError(error_response, 'Op')
client_error.modeled_fields = {'Code', 'Message', 'ErrorCode'}

stdout = io.StringIO()
stderr = io.StringIO()

self.handler.handle_exception(client_error, stdout, stderr)

assert 'errorcode: detail' in stderr.getvalue()


class TestEnhancedErrorFormatter:
def setup_method(self):
Expand Down Expand Up @@ -343,6 +430,33 @@ def test_format_error_with_unicode_and_special_chars(self):
)
assert output == expected

def test_format_error_hides_unmodeled_fields(self):
# Unmodeled fields are now filtered before reaching the formatter.
# Formatter receives only modeled fields.
error_info = {
'Code': 'ExpiredToken',
'Message': 'Token expired',
}

stream = io.StringIO()
self.formatter.format_error(error_info, stream)

assert stream.getvalue() == ''

def test_format_error_shows_modeled_fields(self):
# Unmodeled fields are filtered before reaching the formatter.
error_info = {
'Code': 'FileSystemNotFound',
'Message': 'Not found',
'ErrorCode': 'FileSystemNotFound',
}

stream = io.StringIO()
self.formatter.format_error(error_info, stream)

output = stream.getvalue()
assert 'ErrorCode: FileSystemNotFound' in output

def test_format_error_with_large_list(self):
error_info = {
'Code': 'LargeList',
Expand Down Expand Up @@ -398,6 +512,11 @@ def test_dynamodb_transaction_cancelled_error(self):
},
}
client_error = ClientError(error_response, 'TransactWriteItems')
client_error.modeled_fields = {
'Code',
'Message',
'CancellationReasons',
}

stdout = io.StringIO()
stderr = io.StringIO()
Expand Down Expand Up @@ -441,6 +560,7 @@ def test_error_handler_receives_parsed_globals_from_clidriver(self):
'ResponseMetadata': {'RequestId': '123'},
}
client_error = ClientError(error_response, 'GetObject')
client_error.modeled_fields = {'Code', 'Message', 'BucketName'}

stdout = io.StringIO()
stderr = io.StringIO()
Expand Down Expand Up @@ -471,6 +591,7 @@ def test_error_handler_without_parsed_globals_uses_default(self):
'ResponseMetadata': {'RequestId': '123'},
}
client_error = ClientError(error_response, 'GetObject')
client_error.modeled_fields = {'Code', 'Message', 'BucketName'}

stdout = io.StringIO()
stderr = io.StringIO()
Expand Down
Loading