Closed alallema closed 11 months ago
Will Meilisearch keep the restriction of requiring a token to get the version (more context on this question here)? I think things in this issue somewhat depend on if the version becomes available to use or not.
Triggering the hint specifically for MeilisearchAPIError Disadvantage: Depending on how errors are raised in the SDKs, it is possible that the hint may not be triggered. This is especially true for errors that are not classified as Meilisearch errors. For example, status code > 404.
If the request reaches Meilisearch, and Meilisearch returns a 400 >, I think it is considered a MeilisearchAPIErrors. If the request did not reach Meilisearch, in this case, it is a MeilisearchCommunicationError. At least this is how it works in most SDK's.
So just to understand correctly your disadvantage, if the request is not able to reach the server, the hint wouldn't be added?
I have an additional corner-case. Should the hint be added only on the methods that changed and could trigger an error, or on all methods?
Solutions:
❌ Triggering the hint for all errors: No reason to add a hint in unrelated Errors.
✅ Triggering the hint for all Meilisearch Errors. Not all of them though. MeilisearchTimeoutErrors or others that may have been created should not add the hint.
✅ Triggering the hint specifically for MeilisearchAPIError. Since some of the SDK's are not in line with when to trigger a communication error vs a meilisearch API error, I prefer the previous solution. But in a world where they are correctly implemented, this one is more relevant.
❌ Triggering the hint specifically for MeilisearchAPIError without a message. Depending on the HTTP library there may be a message. For example, the fetch library in javascript adds a body message based on the HTTP error. For example Not found
. While isahc
in rust does not. Also, the error might return a well formated Meilisearch error (error_code, error_type, etc...) and be relevant for the hint. For example if I use filter
on a route that previously did not have that parameter, Meilisearch would return a error_type invalidArgument
(something like that) with an error message in the lines of "filter" is not a known parameter, these are the available ones: limit, offset, etc..
. In this case, the hint is relevant.
❌ Triggering the hint specifically for MeilisearchCommunicationError. For the same reasons you mentioned.
So just to understand correctly your disadvantage, if the request is not able to reach the server, the hint wouldn't be added?
Correct, if the request does not reach the server, the problem is not related to compatibility, so the hint should not be added.
✅ Triggering the hint specifically for MeilisearchAPIError.
Agreed!
Depending on the HTTP library there may be a message. For example, the fetch library in javascript adds a body message based on the HTTP error.
I should explicitly say without Meilisearch message, code, type link ...
Also, the error might return a well formated Meilisearch error (error_code, error_type, etc...) and be relevant for the hint.
Yes, that's my main question. The message sent back by Meilisearch should be quite clear and we always choose to give access to Meilisearch Error as a priority.
I believe we can send the compatibility hint in potential breaking situations, especially when we are going to change the behavior of a method based on a new Meilisearch implementation, like the one that happened during v1.2.0 on deleteDocuments
.
I'm closing this topic now, if someone else thinks this is a mistake let me know and we can reopen this issue! 😉
Foreword
Following the release of Meilisearch v1, the SDKs should strive to be stable and not break when using an older version of Meilisearch. This is done to facilitate the use of the SDKs and Meilisearch itself.
However, as new features are introduced with each release, we have decided to address this situation by adding hints in the error messages returned by the API to help users identify the source of their problem.
For example, this feature is available if a user tries to retrieve their documents using filters with the latest version of the SDK. However, if they have not updated their Meilisearch instance, which unfortunately remains on v1.1, this feature is unavailable. In such a case, they would receive a status code
405
with the associated errorMethod not allowed.
Problem:
We only have one or two scenarios for now, but we would like to implement this system in all the methods that could have this type of behavior. So far, we have created a method that adds an error message to an existing error.
The question is, when should this hint be triggered? Should it be triggered every time an error is returned or only when a specific Meilisearch error is returned? This way, we can integrate it directly into our error-handling protocol in all our SDKs.
Solution:
Triggering the hint for all errors
Triggering the hint for all Meilisearch Errors
The error is triggered in general whenever an error related to Meilisearch is returned, which includes
MeiliSearchApiError
,MeiliSearchCommunicationError
, andMeiliSearchError
Disadvantage:
Advantage:
Triggering the hint specifically for MeilisearchAPIError
Triggering the hint specifically for MeilisearchAPIError without a message
Triggering the hint specifically for MeilisearchCommunicationError
Conclusion:
I haven't detailed all possible solutions for the sake of readability and because these solutions covered most of the questions.
This issue is created as a continuation of this one and it may be subject to change based on the decisions made.
If there are any important points that I may have overlooked, please feel free to comment and share your thoughts.
At the end of this discussion, it would be beneficial to create a general issue for implementing the
versionErrorHintMessage
method and to handle the generated issues with all the details for each SDK.