Extra exception info #156
Extra exception info #156
Conversation
|
Verified that @kelseymorris95 has signed the CLA. Thanks for the pull request! |
|
lgtm to push once the other PR is approved. 👍 |
kelseymorris95
force-pushed
the
extra_exception_info
branch
from
13a3816 to
b21613d
Compare
boxsdk/session/box_session.py
Outdated
| url=url, | ||
| method=method, | ||
| context_info=response_json.get('context_info', None), | ||
| request_response=network_response.request_response, |
There was a problem hiding this comment.
This is an unnecessary backwards incompatibility, since custom networks might not have request_response.
Instead, I would propose that this line be:
network_response=network_response
This has the benefits of:
- Is backwards compatible.
- Works with custom subclasses of
NetworkResponse. - Still allows the user to access
network_response.request_response, ifnetwork_responseis an instance ofDefaultNetworkResponse. - Not tying us to the
requestslibrary even further. - Probably never again needing to add more fields to the exception, since we now include a maximal amount of information.
We could also avoid adding access_token_used, since the user could use exception.network_response.access_token_used instead. It's a minor difference, we could do either. Having it directly on the object is slightly more clear and direct and less work for the user to access, but is another property that we would have to maintain forever for backwards compatibility.
|
👍 , waiting for tests to pass. |
Add
NetworkResponsetoBoxAPIException