Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[jOpenCage] Introduce dedicated exceptions for various potential errors #32

Open
tsamaya opened this issue Jan 20, 2024 · 3 comments
Open
Assignees

Comments

@tsamaya
Copy link
Member

tsamaya commented Jan 20, 2024

The package logs the error and returns null. Structuring the exceptions to distinguish between client-side and server-side errors from OpenCage API would help to react to HTTP errors.

code meaning
200 OK (zero or more results will be returned)
400 Invalid request (bad request; a required parameter is missing; invalid coordinates; invalid version; invalid format)
401 Unable to authenticate - missing, invalid, or unknown API key
402 Valid request but quota exceeded (payment required)
403 Forbidden (API key disabled or IP address rejected)
404 Invalid API endpoint
405 Method not allowed (non-GET request)
408 Timeout; you can try again
410 Request too long
426 Upgrade required (unsupported TLS)
429 Too many requests (too quickly, rate limiting)
503 Internal server error

Error code can be found at https://opencagedata.com/api#codes

@tsamaya
Copy link
Member Author

tsamaya commented Jan 20, 2024

An Idea from https://stackoverflow.com/a/47058425/1910637

Quick answer

In Spring you have exactly what you want:

HttpClientErrorException - Exception thrown when an HTTP 4xx is received.
HttpServerErrorException - Exception thrown when an HTTP 5xx is received.

And a recommended practice

Minimally, you should differentiate exceptions related to business logic (e.g., insufficient balance, email address is not valid) from other exceptions (e.g., server not available, unsupported media type, SQLException).

In our REST API, we have a library for Java clients that parses responses and throws only three different exceptions:

400, 401, 403, 404, 409, 422: throw MyBusinessException, which contains a message that can be shown to the end user. The message comes in the response body (exception handling on the service side), but if not present we have a default message specific to each status code.
405, 412, 415: throw HttpClientErrorException with a message that is specific to each status code.
other 4xx codes: throw HttpClientErrorException with a generic message.
5xx codes: throw HttpServerErrorException with a generic message.
All these exceptions are unchecked.

@tsamaya tsamaya self-assigned this Jan 20, 2024
@tsamaya
Copy link
Member Author

tsamaya commented Jan 22, 2024

Each HTTP Error code would have its own Exception, they all extend a base exception.

  • 400: throws a new HttpBadRequestException
  • 401: throws a new HttpUnauthenticatedException
  • 402: throws a new HttpQuotaExceededException
  • 403: throws a new HttpForbiddenException
  • 404: throws a new HttpNotFoundException
  • 405: throws a new HttpMethodNotAllowedException
  • 408: throws a new HttpTimeOutException
  • 410: throws a new HttpRequestTooLongException
  • 426: throws a new HttpUpgradeRequiredException
  • 429: throws new HttpTooManyRequestsException
  • 503: throws new HttpServerErrorException

@tsamaya
Copy link
Member Author

tsamaya commented Jan 31, 2024

Following a discussion with @lpellegr, the exception names won't always reflect the transport used.

  • 400: throws a new BadRequestException
  • 401: throws a new UnauthenticatedException
  • 402: throws a new QuotaExceededException
  • 403: throws a new ForbiddenException
  • 404: throws a new HttpNotFoundException
  • 405: throws a new HttpMethodNotAllowedException
  • 408: throws a new TimeOutException
  • 410: throws a new HttpRequestTooLongException
  • 426: throws a new HttpUpgradeRequiredException
  • 429: throws new TooManyRequestsException
  • 503: throws new HttpServerErrorException

The base exception is going to be JOpenCageException.

PR #33 is merged into the release-3.0.0 branch and a snapshot version is available for testing on https://oss.sonatype.org/content/repositories/snapshots/com/opencagedata/jopencage/3.0.0-SNAPSHOT/

NB: Keeping this issue open up to the v3 release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant