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

Proposal: Overhaul exception handling in this library #756

Closed
iMerica opened this issue Feb 14, 2019 · 3 comments
Closed

Proposal: Overhaul exception handling in this library #756

iMerica opened this issue Feb 14, 2019 · 3 comments
Labels
difficulty: unknown or n/a fix is unknown in difficulty type: community enhancement feature request not on Twilio's roadmap

Comments

@iMerica
Copy link

iMerica commented Feb 14, 2019

Issue Summary

The entire point of API client wrappers like this is to make it easier to communicate with REST APIs than just using only Python Requests and reading the official REST API Docs.

Currently if I need to get basic debugging details that tell me why a request failed, I have to know about some other lower level library called python_http_client (by the way, why not just use Requests as your http client?)

So the problem is:

  • Be consistent about the layer of abstraction. If people need to know about sendgrid/python-http-client to use this library, then why not just have them use that instead of this library?
  • If you are going to raise some exception from a third party library, at least include the HTTP response body in the exception itself instead of hiding it.

Steps to Reproduce

  1. Send an email with some kind of issue (like an invalid template ID)
  2. Wrap your send invocation in a try/catch following the instructions from the readme.
  3. Observe python_http_client.exceptions.BadRequestsError is raised, but there is no message included with details. If I wanted HTTP level exceptions, I'd just use Requests..

Expected Results

I expect API Client wrappers to consistently wrap HTTP clients. This includes the regular HTTP CRUD operations and exceptions. For example:

import sendgrid

try:
  sendgrid.someOperation(data=data)
except sendgrid.exceptions.SpecificErrorA as e:
   print(e)
except sendgrid.exceptions.SpecificErrorB as e:
   print(e)
except sendgrid.exceptions.SpecificErrorC as e:
   print(e)
except sendgrid.exceptions.SpecificErrorD as e:
   print(e)
except sendgrid.exceptions.SpecificErrorE as e:
   print(e)
except Exception as e:
  # catch all
   print(e)

Where SpecificErrorX handles a specific class of things that can go wrong and e contains the nitty gritty details about it.

Technical details:

  • sendgrid-python Version: 5.6.0
  • Python Version: Python 3.7
@iMerica
Copy link
Author

iMerica commented Feb 14, 2019

I've updated the description to include an Expected Results section

@thinkingserious
Copy link
Contributor

Hi @iMerica,

Thanks for taking the time to provide feedback!

This work is currently underway here, if you have a moment please have a look and let us know what you think. The WIP code is in the v4 branch in this repo.

On using requests, one decision we made was to remove as many dependencies as possible. python_http_client is simply a thin wrapper of the native Python http client. We separated that out thinking that others may find it useful independent of the SendGrid specific SDK.

With Best Regards,

Elmer

@thinkingserious thinkingserious added type: community enhancement feature request not on Twilio's roadmap difficulty: unknown or n/a fix is unknown in difficulty labels Feb 14, 2019
@iMerica
Copy link
Author

iMerica commented Feb 20, 2019

That issue you linked to looks like a pretty stale Github issue. Is there a branch or Pull Request?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: unknown or n/a fix is unknown in difficulty type: community enhancement feature request not on Twilio's roadmap
Projects
None yet
Development

No branches or pull requests

2 participants