Improve errors

[flevi29]

Pull Request

Related issues

Fixes #1612, #1655

What does this PR do?

This PR aims to improve errors, so that they can contain all the necessary information, and more.

• Prevent browser test jsdom from replacing fetch and AbortController for consistency with node tests, replacing previous solution where we removed the builtin fetch from node tests
• Remove "abort-controller" package, it was only used in tests and now AbortController is always available
• Rename MeiliSearchCommunicationError to MeiliSearchRequestError, as this error might not be entirely related to communication, but rather the request itself
• Make errors use cause, preserving the original error, simplifying things, taking advantage of modern browsers and runtimes actually printing this property
• Remove the use of Object.setPrototypeOf in errors, this is not needed in modern browsers, and bundlers take care of it if we need to support older browsers (so in UMD bundle it's done twice currently). (https://stackoverflow.com/a/76851585)
• Remove the use of Error.captureStackTrace, this is done by the base Error constructor, and it's only available in V8 engine based browsers/runtimes
  • https://v8.dev/docs/stack-trace-api
  • https://nodejs.org/api/errors.html#new-errormessage-options
  • https://stackoverflow.com/a/64063868
  • https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error/stack
• Only catch the error from fetch to re-throw it as MeiliSearchRequestError, other potential errors should propagate as they're either truly unexpected or are thrown by us, simplifying error handling and not putting unexpected errors under the MeiliSearchError umbrella
• Rename MeiliSearchErrorInfo type to MeiliSearchErrorResponse
• Other minor changes/improvements

NOTE: Tests are horrifying, I didn't change all that much in src, but I had to change almost every test and by quite a bit. Testing is what I should aim to improve ASAP.

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 96.94%. Comparing base (01f51b4) to head (4b3dc78).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1656      +/-   ##
==========================================
- Coverage   97.44%   96.94%   -0.51%     
==========================================
  Files          22       21       -1     
  Lines         861      818      -43     
  Branches       93       78      -15     
==========================================
- Hits          839      793      -46     
- Misses         21       25       +4     
+ Partials        1        0       -1     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[flevi29]

Breaking changes:

• Rename MeiliSearchCommunicationError to MeiliSearchRequestError
• Rename MeiliSearchErrorInfo type to MeiliSearchErrorResponse
• MeiliSearchApiError now has a different property structure
  • MeiliSearchErrorResponse is now in its cause property
  • Instead of saving the HTTP status on the error, we save the whole Response on its response property for more available information (one of which is status, so error.response.status)
• MeiliSearchRequestError now contains the whole caught error from fetch on its cause property instead of plucking some properties off of this error (this old way potentially losing information as we can not know the exact shape of this error)

[flevi29]

I've looked into meilisearch-js-plugins, there is no handling of these special errors there, this should not break it in any way.

[flevi29]

These changes might appear daunting at first, but it's mostly tests, and because tests are repeated ad nauseam with very little generalization, you've seen 3 types of changes and you've seen them all.

[flevi29]

Well, actually some more details about the cause property. Runtimes seem to print it and play nicely.

However, browsers don't give a damn. Only Firefox prints it at this moment, and unless cause is another Error object with a message property, it's not going to be of any service to you, absolutely beautiful work here:

But there's still hope, chromium is working on it: https://issues.chromium.org/issues/40182832 Maybe they will have the right ideas, unlike Firefox, even though they are massively late to the party, and let's not even talk about Safari, I'm sure they haven't yet started working on it. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error/cause#browser_compatibility

Still, it's not like the old errors we have had any of their bespoke properties printed, so it's still an improvement, and it's future-proof.

[curquiza]

bors merge

[meili-bors[bot]]

Build succeeded:

• integration-tests (Node.js 18)
• integration-tests (Node.js 20)
• style-check
• types-check

[flevi29]

Hi @brunoocasali sorry for the late response.

Errors now make use of the standardized cause property.

• MeiliSearchApiError
  • is thrown when response.ok is false
  • message -> cause.message
  • code -> cause.code
  • type -> cause.type
  • link -> cause.link
  • in case the request doesn't return a response body (for instance in case of an internal server error 500), cause remains undefined
  • httpStatus -> response.status
  • message, in case the response body is not empty, is its message property, otherwise HTTP status + status text
  • response property contains the whole fetch Response
  • MeiliSearchErrorInfo type renamed to MeiliSearchErrorResponse
• MeiliSearchCommunicationError is removed

New:

• MeiliSearchRequestError
  • Is thrown when fetch throws for all of the reasons listed here
  • cause property contains the error thrown by fetch