Reviewing of error handling

[alallema]

Warm up:

The aim of this issue is to detail the basic requirements of the SDK's error handlers, provide a brief refresher, and to question its implementation with the possibility of modifying or improving it.
This issue is created keeping in mind this one that defined the implementation of error handlers.

Reminder:

Meilisearch is an API with which the SDKs communicate via the HTTP protocol. Because of this, responses and therefore errors are returned through various HTTP statuses. The purpose of the error handler in the SDKs is to specify to the user that this error is returned by the package, therefore by Meilisearch that you are using. This is to increase visibility for the user amid other errors.

As specified in the central issue the SDKs should raise at minimum 3 errors:

• MeiliSearchApiError: MeiliSearch (HTTP) sends back a 4xx or a 5xx.
• MeiliSearchCommunicationError: problem with communication, MeiliSearch instance sends nothing back.
• MeiliSearchError: for any other errors raised by the SDK.

Today, the API returns a certain number of errors for this status code:

200 | ✅ Ok
201 | ✅ Created
202 | ✅ Accepted 
204 | ✅ No Content
205 | ✅ Reset Content
400 | ❌ Bad Request
401 | ❌ Unauthorized
403 | ❌ Forbidden
404 | ❌ Not Found

These specific errors returned by the API have a certain format with a type, a code, a message and a link.

Errors above 404 as well as certain errors between 400 and 404, do not have a specific error message. For example, if I query Meilisearch with an incorrect route, I will receive the error: 404 Not found. This is normal because the route does not exist, and Meilisearch cannot return a specific error. However, if I try to access an index that does not exist on the route localhost:7700/index/index_name, I will also receive a 404 Not found error, but with this request body:

{
	"message": "Index `movies` not found.",
	"code": "index_not_found",
	"type": "invalid_request",
	"link": "https://docs.meilisearch.com/errors#index_not_found"
}

Question:

The question at hand is whether all our errors are properly categorized according to the different codes chosen at the beginning or if this has changed.

The second issue is that as Meilisearch has more and more features and error messages based on different situations, we should separate the errors returned by the API when they do or do not contain a message.

Different possibilities

• Keep as is

• Transfer errors > 400 and without message to CommunicationError (I am not in favor of this solution)

• Create a new error that is not a MeilisearchAPIError or that is a subclass of MeilisearchAPIError for errors with the API when a message is missing, for example, in these cases:

  • For all errors above 404.
  • For all errors between 400 and 404 without a message.

• Create 2 new sub-errors within the MeilisearchAPIError category to handle errors with or without a message.

TODO:

• Create a base error called MeilisearchError which will extend the standard error if it does not exist (when the language supports)
  • Make all the other errors extend this error.
• Move all errors without message to MeilisearchCommunicationError since it is not a Meilisearch error anyway.

[sanders41]

I have a general question for some additional context. Have people reported issues with the current errors? I haven't seen any. I'm trying to think of a time when this would be an issue. Taking the 404 as an example, in this case the error won't be a specific Meilisearch error, but it will be clear what the issue is since the underlying library handling the http requests will show the 404.

I'm just wondering if making a change will add a lot of extra maintenance for not much benefit.

[alallema]

Hi @sanders41,
That's an excellent question, indeed. When we first implemented the versionErrorHintMessage method, we realized that you weren't all aligned on the definition of errors. Moreover, some names like wait_for_task return a MeilisearchTimeoutError are not necessarily appropriate.
In addition, we try to remain vigilant and up to date regarding the errors returned in SDKs, as it can save a lot of situations to have clear and concise errors.
But this is more of a reminder and reflection on the subject for now.

[bidoubiwa]

I think this option is the best:

Transfer errors > 400 and without message to CommunicationError (I am not in favor of this solution)

About MeilisearchError I know @brunoocasali would like that all other errors inherit from this error (at least in the languages where it is possible).

For the MeilisearchTimeoutError, I would like to rename it MeilisearchWaitForTaskTimeOut or something that does not imply the Meilisearch server timed out.

[brunoocasali]

I've opened issues in each repository, and I will close this issue now.

I believe this change could be beneficial for the consumers and for the maintainers, since it is going to ensure a concise public API regarding the errors and will make things easier when you try to catch errors in the test suite.