-
Notifications
You must be signed in to change notification settings - Fork 215
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
Throw real Errors instead of objects #1444
Comments
I agree that we should return real errors, but changing this could be a breaking change if anyone relies on the type or the keys of the object now. @shortcuts, in v5 we are throwing real errors right? |
I agree that it can be considered as a breaking change. Looks good to me if it's fixed in the next major release 👍 |
Hi, i would like to work on this issue, as i have good knowledge of throwing proper errors. So i will be able to handle it properly. |
This package has a non standard behavior of throwing objects instead of real JS Errors. As it works approximately in the same way in most cases and these objects are similar to Error (at least, TS is ok with that), there are some edge cases where it messes everything up.
My specific use case is with Sentry, which is not able to handle properly these "errors". Instead of having a proper stack trace, I have the Sentry error Non-Error exception captured and all the informations about the real error are lost.
We do this kind of thing all over the package:
accountCopyIndex.ts
createDestinationIndiceExistsError.ts
I propose instead to either throw a basic Error with the message...
Or even to create custom error objects
I even think that this helper methods are not necessary and that custom errors could be thrown directly. For instance, the custom message is useless in this case.
I can open a PR with a proof of concept if you think it's a good idea. I would also be glad to hear your ideas about the right implementation.
The text was updated successfully, but these errors were encountered: