mirror of
https://github.com/saltstack/salt.git
synced 2025-04-17 10:10:20 +00:00
Log S3 API error message
Background: When the S3 API returns for example code 403, it is hard to guess what went wrong without looking at the more verbose message, as seen for example in #29912. When S3 API returns an error code in the 400 or 500 range, put message contents into debug log for easier troubleshooting. Requests doesn't raise HTTPError automatically - for that raise_for_status() is commonly used. Therefore the try clause never caught any exceptions. This change replaces the never activated except block with a response code check, as seen in utils/aws.py Q&A: Should we raise an error instead? Potentially Salt will generate a big amount of these log lines, for example when updating every single states file from S3. It is less scary to put it in debug log. Why did we lose the "return False"? It is inconsistent with the error handling below where return value does not matter. Why aren't we alerting the user that something went wrong? That requires a go-through through the whole s3.py to see which log.debug() should really be log.error() and is left for a future change.
This commit is contained in:
parent
941bcaed07
commit
8c4a101c8f
1 changed files with 9 additions and 11 deletions
|
@ -119,17 +119,15 @@ def query(key, keyid, method='GET', params=None, headers=None,
|
|||
if not data:
|
||||
data = None
|
||||
|
||||
try:
|
||||
result = requests.request(method, requesturl, headers=headers,
|
||||
data=data,
|
||||
verify=verify_ssl)
|
||||
response = result.content
|
||||
except requests.exceptions.HTTPError as exc:
|
||||
log.error('Failed to {0} {1}::'.format(method, requesturl))
|
||||
log.error(' Exception: {0}'.format(exc))
|
||||
if exc.response:
|
||||
log.error(' Response content: {0}'.format(exc.response.content))
|
||||
return False
|
||||
result = requests.request(method,
|
||||
requesturl,
|
||||
headers=headers,
|
||||
data=data,
|
||||
verify=verify_ssl)
|
||||
response = result.content
|
||||
if result.status_code >= 400:
|
||||
# On error the S3 API response should contain error message
|
||||
log.debug(' Response content: {0}'.format(response))
|
||||
|
||||
log.debug('S3 Response Status Code: {0}'.format(result.status_code))
|
||||
|
||||
|
|
Loading…
Add table
Reference in a new issue