Skip to content
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

Improve aiohttp error handling by converting response body to error #477

Open
jackwotherspoon opened this issue Sep 19, 2022 · 1 comment
Assignees
Labels
priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@jackwotherspoon
Copy link
Collaborator

The Python Connector currently uses raise_for_status=True when using aiohttp's ClientSession.

The default behaviour for this argument is to close http connections when response status code > 400 and raise generic error message (ex. 400 Bad Request), so the response body which may have a more helpful error message or instructions cannot be accessed.

It is recommended to not use raise for status if access to the errors response body is required. Because of this the Python Connector should do something along the following (pseudocode):

def convert_response(resp):
  # attempt to read response and format custom error
  try: 
    resp_body = await resp.json()
    # ... raise error using standard response format
    build_error(resp.status, resp_body)
  # if error occurs reading response body, fallback to raise_for_status 
  except:
    resp.raise_for_status()

# --------------------------------------------------------------------
# ... inside _get_metadata (refresh_utils.py)
async with client_session.get(url, headers=headers) as resp:
  # if not 2XX status code
  if not resp.ok:
    # handle standard response and format it into error
    convert_response(resp)
  # ... continue onwards
@jackwotherspoon jackwotherspoon added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. priority: p2 Moderately-important priority. Fix may not be included in next release. labels Sep 19, 2022
@jackwotherspoon
Copy link
Collaborator Author

Improve error messages around IAM as well

@jackwotherspoon jackwotherspoon added priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. and removed priority: p2 Moderately-important priority. Fix may not be included in next release. labels Jun 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

No branches or pull requests

1 participant