closeio-api icon indicating copy to clipboard operation
closeio-api copied to clipboard

APIError message should contain the response's status code

Open wojcikstefan opened this issue 7 years ago • 5 comments

So that it's obvious at a glance what kind of an error we experienced. Right now it's possible to get an enigmatic traceback:

In [3]: api.get('me')
Traceback (most recent call last)
<ipython-input-3-842652bd220e> in <module>()
----> 1 api.get('me')

/Users/wojcikstefan/Repos/closeio-api/closeio_api/__init__.py in get(self, endpoint, params, **kwargs)
    132         """
    133         kwargs.update({'params': params})
--> 134         return self._dispatch('get', endpoint+'/', **kwargs)
    135
    136     def post(self, endpoint, data, **kwargs):

/Users/wojcikstefan/Repos/closeio-api/closeio_api/__init__.py in _dispatch(self, method_name, endpoint, api_key, data, debug, **kwargs)
    108             raise ValidationError(response)
    109         else:
--> 110             raise APIError(response)
    111
    112     def _get_rate_limit_sleep_time(self, response):

APIError:

wojcikstefan avatar May 19 '17 19:05 wojcikstefan

Note that this is a breaking change as unicode(exc) no longer contains just a stringified json for validation errors. That said, we should encourage people to use exc.errors and exc.field_errors anyway.

wojcikstefan avatar May 19 '17 19:05 wojcikstefan

@thomasst would you please do a quick review of this one?

wojcikstefan avatar Jun 03 '17 19:06 wojcikstefan

I'm not aware of any. However, to introduce this breaking change in a mature way, I think we should bump the major part of this package's version after this is merged, and note in the release that one should use json.loads(exc.response.text) from now on. Sounds good @philfreo @thomasst ?

wojcikstefan avatar Jan 09 '18 15:01 wojcikstefan

and note in the release that one should use json.loads(exc.response.text)

No, they shouldn't have to JSON-parse the text property. There's already exc.errors and exc.field_errors for ValidationError. Are there any other cases we should cover?

thomasst avatar Jan 09 '18 20:01 thomasst

@eengoron is this something we still want to do? If no, feel free to close this PR. If yes, but there's a better way to do it, please close this PR and open a new issue/PR.

wojcikstefan avatar Dec 09 '21 12:12 wojcikstefan