pycrest / PyCrest

Python library for accessing the EVE Online CREST API
MIT License
35 stars 22 forks source link

Add HTTP status code to APIException #19

Closed Acidity closed 8 years ago

coveralls commented 8 years ago

Coverage Status

Coverage increased (+0.02%) to 96.981% when pulling 3656b828fbf6c09496e9d486dec8a9e6223842ba on Acidity:feature/error_status_codes into a5e537523d3e9a8bda1c0df2a76759aec8e76b89 on Dreae:master.

wtfrank commented 8 years ago

OK so with the HTTP methods that CCP have started using, there's at least one example where a POST call returns status 201 instead of the more usual 200. Do we also need a method of communicating the return value after a successful call? Or do we only care about it in the failure case? In either case this pull is definitely useful.

jonobrien commented 8 years ago

Would be useful for debugging if we knew about return codes, but probably just for failure case currently?

I know there are different messages in the response body for invalid scope vs invalid callback uri that both return 401. Sometimes parsing the body on an error code could be useful too, but as far as return code, unless you've found it necessary on successful calls, probably not too important.

hkraal commented 8 years ago

There is no reason to hide the actual response code from the user, if ones wants to it should be accessible. That's however something which should be covered with #7 along other possible return values.

In case of exceptions I really would like the HTTP code and content (if any) which is being returned if possible the URL + parameters could prove helpful as well. The Fittings endpoint has some special error messages which should be getting to the user.

jonobrien commented 8 years ago

Yes I saw that Fittings error as well, good point. :cake:

wtfrank commented 8 years ago

So the exception needs to be augmented with any response text as well as just the response code.

I wonder if there is an exception in the requests library already? or maybe its better to have a pycrest specific exception class that does just what pycrest needs

regarding URL and parameters - you surely know what they are as you just made the call that is throwing an exception? Unless you mean for debugging, in which case maybe there's a case for using the python logging facility.

hkraal commented 8 years ago

Closing as the improved version in #41 is now merged into master