fixes broken exception handling on http status code != ok.

[ms5]

the current handling of http error codes != ok is broken, so i fixed this with a new class explicit for this cases. the exception message contains the http status code, so does the exception object has an attribute for status_code.

[jmcarp]

Thanks for catching this issue. I'm not sure exactly which error brought this up, but a related fix in 8e69958206887af46727bdf9d83f5c5a65705317 may solve the issue. Could you check whether you still run into the same problem on master?

[ms5]

8e69958 solves the syntax error but does not handle http errors any better than before.

some issues i see with the current handling of http error:

• all http errors are handled as "UNKNOWN" by the current handler (my pr for example will log: "error http return code: 503")
• if response code is not ok, why should we check for a json response, there will never be one

why i added a new class:

• separate handling of http errors and actual api errors
• clear message about what happened. in case of:
  • api error: the error message
  • http error: the error code

I see several options to improve the error handling:

this pr

• pros:
  • distinct class (as written above) for api errors and http errors
  • simple/clear code (less complexity)
• cons:
  • another exception class
  • http error not handled together with api errors (yes could also be a con)

integrate http error code handling into api error exception

• pros:
  • one class for api and http error handling
• cons:
  • more complexity inside the exception class
  • no separate exception for http error
  • handling of response (json or text) is not necessary in case of status not ok response

there will probably be more pros/cons. but thats what just popped up in my head.

I'm looking for some input here. If the second option is the preferred way, I'm glad to write another pr for that case.

[jmcarp]

So is it the case that all error responses with JSON bodies have a status code of 200?

I don't have a strong preference about this. If we expect that users will want to catch 200 and non-200 errors separately, the extra exception class would be nice. On the other hand, if the more common use case is to handle both error categories in the same way, then the second exception type means lots of code like except (ApiError, ApiHttpError). To me, it's a tossup.

If you have a preference for your current solution, I'm happy to merge.

[ms5]

yes responses that carry a json (or any payload data) should have response code 200. according to the RFC code 203 is also possible, but I don't think betfair uses it. (I've not seen many company using it actually) see also http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html. Different codes are possible for example 3xx for redirect requests and 401 (unauthorised) but all those responses do not carry a json payload.

the main reason why I actually prefer two exception classes is, that in case of, for example error code 503 (Service Unavailable), I'd like to retry till the request is not of interest anymore for me or the server continues to respond. But in case of an API error, for example "market not found", I don't need to retry, the market will not come back just because I try more often ;)

[ms5]

I've created pr #51 with a proposal of handling this case.