Skip to content

Error naming in MeiliSearch #8

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

Closed
bidoubiwa opened this issue Apr 21, 2020 · 4 comments
Closed

Error naming in MeiliSearch #8

bidoubiwa opened this issue Apr 21, 2020 · 4 comments

Comments

@bidoubiwa
Copy link
Contributor

bidoubiwa commented Apr 21, 2020

Foreword

MeiliSearch is an API with which you communicate through HTTP requests.
Because of that, it uses the HTTP error standards to inform you of the state of each request.

  • 2xx
  • 3xx
  • 4xx
  • 5xx

A wrapper, on the other hand, creates a sweet sugar coat around the API to make the communication easier in a given language. Although the wrapper uses HTTP requests, the user does not.

This is why I questioned to which extends do we want to communicate to the user the same way the API communicates with us: through HTTP error codes.

Problem

Imagine a case where you have badly formatted a search parameter the following way:

index.search(null)

If javascript validates data before sending it to MeiliSearch, it will raise an error like this (the text could be different):
MeiliSearchError: invalid query parameter

If javascript DOES NOT validate data before sending it to MeiliSearch, MeiliSearch API will send this back

HTTP/1.1 400 Bad Request
{"message":"invalid query parameter"}%

And could be formatted this way by javascript error handler:

HttpError 400: Bad request: invalid query parameter

This create an inconsistency in the error types.

Examples

How does the other API does it:

  • Stripe does not raise an httpError nor the HttpStatus but created different types of errors:
StripeError
StripeCardError
StripeInvalidRequestError
StripeAPIError
StripeAuthenticationError
StripePermissionError
StripeRateLimitError
StripeConnectionError
StripeSignatureVerificationError
StripeIdempotencyError
StripeInvalidGrantError

example of stripeAPIError
Screenshot 2020-04-21 at 12 03 43

This error will natively output like this:

StripeAPIError: Invalid JSON received from the Stripe API

Used in those cases :
Screenshot 2020-04-21 at 11 44 44
Which means that the error output will natively look like this:

ApiError: User not found

So no mention of Algolia (but you will be able to see that it comes from algolia in the stack that appears below the error), and the status can be fetch this way: e.status

Proposition

MeiliSearch sends back a 4xx or a 5xx

My favorite approach is the one from Stripe. Nevertheless, in a short term I would suggest this one:

  • MeiliSearchApiError
    or
  • MeiliApiError

MeiliSearch sends nothing back, problem with communication

  • HttpError

MeiliSearch SDK raises an error

  • MeiliSearchError
    or
  • MeiliError
@curquiza
Copy link
Member

Hi!

Love your issue and your investigation 🙂

MeiliSearch sends back a 4xx or a 5xx

I like MeiliSearchApiError. Meili is just the name of the company, neither the SDK or the search engine. I like your idea to talk about API instead of HTTP. More friendly at my point of view.

MeiliSearch sends nothing back, problem with communication

I do not like HttpError. An HTTP error is, by definition, a 4XX or a 5XX. Not a communication issue.

I suggest CommunicationError.

MeiliSearch SDK raises an error

I would rather use MeiliSearchError, for the same reason than the first one.

@eskombro
Copy link
Member

I like the Idea. But in the same line as with other issues/features, we might try to define some basic standards. Each SDK will try to handle this in its own way (and it is clearly the case now), but it could be cool to have a generic 'reference' for errors, codes and messages, and implement THAT in each SDK in it's own way and with its own constraints!

@curquiza
Copy link
Member

Yes, we want to define generic rules for error handling, that's the purpose of Charlotte's issue: the first step toward these generic rules ;)

We should definitely take the time to define what are the minimum errors we expect in each SDK and the behavior they should have. The three ones that Charlotte suggests is a good start.

@curquiza
Copy link
Member

Closing this issue in favor of #19

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

No branches or pull requests

3 participants