Handling errors returned by gdax?

[duffar12]

How are people handling errors returned by gdax?
For example, lets say I enter an order and gdax returns a http error

401	Unauthorized – Invalid API Key
OR

500	Internal Server Error – We had a problem with our server

How is this handled cleanly?
I can see that all of the gdax authenticated client methods have got the line
r.raise_for_status()
commented out. These lines were all commented out in a change made to the code in may. But there were no comments in the change so I cannot see the reason why this was done.

[js931]

I would be interested in that, too. It's pretty dangerous that the no errors are raised...

[alex0527]

I would like to ask the same question, how to handling errors returned by gdax?

[bpross]

According to this: https://docs.gdax.com/?python#errors 'message' will be in the response body, so I just check for that key and raise a custom error. Something like this:

      def _raise_for_error(self, response):
          """
          Raises GdaxException if 'message' in response
          """
          if response.get('message'):
              raise GdaxException(response.get('message'))
          return response

[duffar12]

This is also what I have started to do. However, I don't love it as a solution and it does not explain why the line "r.raise_for_status()" is commented out in the API code.

Having your own error handler is a pragmatic fix for this. But, it means that you are re-writing functionality that already exists and has been tested millions of times in the requests library. I'm also not extremely confident that checking for 'message' will always give the desired behaviour. Gdax may change their spec in future. Or there may be valid responses, which contain a 'message' field.
The entire point of having status codes is so that you can classify the response.
That's just my opinion anyways. There may well be a very valid reason why the API ignores the status codes. But without comments in the code submissions I cannot tell why it was done

[duffar12]

Also, through testing I have discovered at least one instance in which the above error handling code suggested by @bpross won't work.
authenticated_client.get_orders()
wraps the response from gdax server in an array. In this instance we must use
if ('message' in response[0]):
raise GdaxException(response.get('message'))

This is a not a good situation. API should return the error.
I would offer to make the change to the API, but yet again, I have no idea why the error checking was removed...

[bpross]

I was just about to post that my sample method won't work. Not surprised that you ran into it too. I have no idea why the error checking was removed in cedc00e. Seems really strange to change functionality when adjusting formatting.

[duffar12]

@acontry could you please help us out with this?

[acontry]

There's been talk of implementing error handling for a while now (see Issue #27 from last year) but nobody (including me) has stepped up to provide a solution. The problem with raise_for_status (if I remember right) is that it clobbers any detail in the error, so if you want to handle different cases in different ways you simply can't. It should probably be used in building better error handling, but right now it hurts more than it helps. I think I also commented those out for consistency across the library since it had already been done elsewhere.

One difficulty in implementing error handling properly is discovering all of the errors possible from the API. Collaboration could help here. I'm thinking about forking this repo to fast-track improvements like error handling and make breaking changes that are (in my opinion) warranted - if there's interest in this, let me know!

[duffar12]

Thanks for the response @acontry that makes sense as the error details are very important.
 I may try and submit the following solution.
Bearing in mind that GDAX state that all successful responses will have a status code of 200...

def get_whatever():
        r = requests.get(some_url)
        if(r.status_code != 200) :
                 raise GDAXException(r.json())
        return r.json()

What do you guys think? @bpross @alex0527 @js931 Any reason that you can think of off hand why it wouldn't work?

[bpross]

I like it, but I think we should also save the response_code in the exception as well. Otherwise looks good.