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

Request exception handling #18

Closed
elyobo opened this issue Aug 3, 2015 · 1 comment
Closed

Request exception handling #18

elyobo opened this issue Aug 3, 2015 · 1 comment

Comments

@elyobo
Copy link
Contributor

elyobo commented Aug 3, 2015

The client __call catches AssertionError and raises a BadRequest instead. However most (or all?) methods that call __call themselves are trying to catch AssertionError, which will never actually be raised because __call already handles it.

The end result is that all of the "except AssertionError" blocks are never triggered and the exception is propagated instead of being caught, which is probably not intentional (I suspect that the custom BadRequest exception was implemented later).

However, I think it is actually useful to propagate the exceptions to that client code can handle them appropriately, instead of just returning a bool on an error, so I lean more towards removing all of those try/catch blocks entirely and changing the __call and __call_stream methods to raise something that allows users to determine the underlying problem and decide what they want to do.

@elyobo
Copy link
Contributor Author

elyobo commented Aug 4, 2015

Note that this limits the amount of test coverage that can be achieved as well; because __call catches all AssertionErrors and converts them to BadRequests, it's impossible to cover any of the except AssertionError lines in the functions that call __call, as no AssertionError will be raised (unless you mock __call to raise them, which is a bit pointless).

@toloco toloco closed this as completed Sep 21, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants