Xero-Java icon indicating copy to clipboard operation
Xero-Java copied to clipboard

SDK exception codes do not match API response codes

Open U3R1YXJ0 opened this issue 4 years ago • 4 comments

SDK you're using (please complete the following information):

  • Version 4.1.4

Describe the bug For background purposes, the specific thing I am working on is creating a retry solution for "Internal Server Error" messages. We see a lot of these in our logs.

We created a solution in which errors with a code == 500 are retried. I assumed this was the correct code to catch because the API response codes page says Internal Errors are a 500. However, this do not seem to work. On closer inspection, it appears "Internal Server Error" messages are actually error with code > 500.

The issue raised is that error codes greater than 500 from the API are not mapped well into Java exceptions.

I have 4 sources of information on error codes: XeroApiExceptionHandler.java README.md https://developer.xero.com/documentation/oauth2/limits https://developer.xero.com/documentation/api/http-response-codes

  1. The response codes page says rate limit exception is a 503, but OAuth2 limits, Java SDK code and readame say its a 429. This is likely an issues for the API team on the response codes page?

  2. The Java SDK maps all errors > 500 to the same message and the exception codes are not logged by default. 501 (not implemented), 503 (rate limit?), 503 (not available) and 503 (organsation offline) all get mapped to a static message "Internal Server Error". We are therefore losing valuable error information. This also explains why we're regularly seeing Internal Server Error from the API - they are most likely actually 503 not available messages from the Xero API, which are suitable to retry (unlike internal server errors).

  3. Mapping error codes > 500 to a static message "Internal Server Error" is particularly confusing because the the Xero API "Internal Error" is actually a 500 code. The 500 code then maps to a different message (no mention of internal server error) asking the client to check the Xero APi status page.

To Reproduce N/A

Expected behavior The Java SDK exception messages for codes > 500 should match the API exception messages. The Java SDK should not merge multiple API messages into one Java exception message. Perhaps the API code should be included in the Java exception message, as its not included by default.

Screenshots NA

Additional context NA

U3R1YXJ0 avatar Mar 08 '21 10:03 U3R1YXJ0

I can see a 503 from the accounting API is a rate limit exception, and a 429 from the oauth2 api is a rate limit exception. I'm now wondering whether the Java SDK differentiates these? Could I actually be getting 503 rate limits exception on the accounting API that are being converted into "Internal Server Error" in the SDK?

Is the accounting API rate limit still a valid response if I am using OAuth2? Or will I only get OAuth2 rate limits errors?

U3R1YXJ0 avatar Mar 08 '21 10:03 U3R1YXJ0

Hi @U3R1YXJ0

In your last comment you use the term "the accounting API" and the "ouath2 api". I believe you are confusing the API set and the authentication methods. OAuth 1.0a (scheduled for deprecation) and OAuth 2.0.

429 is the correct status code for OAuth 2.0 rate limit exceptions with header information about which rate limit was hit. 503 was the status code for OAuth 1.0a for rate limits - but is not true for OAuth 2.0

You call out two separate issues.

  1. The SDK should handle errors > 500 with more granularity. Def. something we can do - this is a feature request and will need to be prioritized against other work. Unless the amazing @lancedfr can lend a hand to get it done quicker 😄
  2. The API shouldn't return multiple error types with the same status code. - Is this accurate? If so, this is feedback for the API engineering team and can't be solved in the SDK.

SidneyAllen avatar Mar 16 '21 13:03 SidneyAllen

I raised the 429/503 issue with the API team and they have updated https://developer.xero.com/documentation/api/http-response-codes. The page now has both codes and specifies that one is relevant to OAuth2 and the other OAuth1.

The updates I suggest to the SDK (XeroApiExceptionHandler.java) are:

  1. Handle/Map 501 and 503 API codes
  2. Remove mapping of >500 codes to "Internal Server Error" message (this removes useful info from API).

U3R1YXJ0 avatar Mar 16 '21 16:03 U3R1YXJ0

I've submitted a pull request @SidneyAllen

evelinasmit avatar Mar 22 '21 10:03 evelinasmit