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

Non-error thrown at createRetryError #1301

Closed
holm opened this issue Aug 25, 2021 · 3 comments
Closed

Non-error thrown at createRetryError #1301

holm opened this issue Aug 25, 2021 · 3 comments
Labels
error related to errors

Comments

@holm
Copy link

holm commented Aug 25, 2021

At https://github.com/algolia/algoliasearch-client-javascript/blob/master/packages/transporter/src/concerns/retryableRequest.ts#L65 a plain object is thrown instead of a proper Error instance.

I am not sure why this is the case, but it results in hard-to-handle exceptions, that gives poor stacktraces and error reporting.

@Haroenv
Copy link
Contributor

Haroenv commented Aug 26, 2021

This was done to be able to easily add other properties (like debug information) to the error, and avoiding Error (as this isn't consistent between browsers, babel etc.). I agree that it should be changed for all errors, however I'm not sure what makes this error harder to handle, could you clarify?

@holm
Copy link
Author

holm commented Aug 26, 2021

I'm not sure why Error isn't consistent. It's part of the core language and has always been. It's certainly possible to add additional properties to an error using normal prototypical inheritance from the base Error. Lot's more information is at https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error#custom_error_types.

We use Sentry to handle errors occurring in systems and report them centrally. These will generally verify that a thrown or rejected value is always an Error. Errors are special in that they capture the stacktrace when created, which is essential for tracking down sources of errors.

This is also why a core eslint rule exists for this, which also has some background: https://eslint.org/docs/rules/no-throw-literal.

@Haroenv Haroenv added the error related to errors label Aug 26, 2021
@shortcuts
Copy link
Member

closing in favor of #1444

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

No branches or pull requests

3 participants