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

Why prefer-promise-reject-errors?? #1273

Closed
Jakobud opened this issue Apr 14, 2019 · 1 comment

Comments

@Jakobud
Copy link

commented Apr 14, 2019

What version of standard? Latest

What operating system, Node.js, and npm version? Ubuntu 16, Node.js 9.11.1, npm 6.4.1

Why is Standard using prefer-promise-reject-errors? If you are assuming that every Promise rejection is the result of an error that should be a new Error() then you are assuming too much. A perfect example that led me here: I'm developing some extensions for VSCode and many of the UI interactions that are initiated from the extension return a Promise. For example, asking the user to pick a file for some action to happen on that file. If the user changes their mind and does not pick a file, then the only way to handle this is is a Promise reject() in order to keep the "action" from happening. But returning an actual Error doesn't make sense here. The extension is behaving perfectly normal. No error has occurred. But if you are using StandardJS it's forcing me to unnecessarily throw an Error object.

Doesn't really make sense to enforce such a thing for all situations.

@timoxley

This comment has been minimized.

Copy link
Contributor

commented Apr 14, 2019

This is an edge case, almost all of the time you want an error with the rejection, anything else e.g. a string, is most likely bad practice as it loses the stack trace and makes it a nightmare to track down where the error came from. Rejecting or throwing a string or an object without a stack trace is frustratingly common. Since the rule promotes a good practice for the majority of cases it should probably remain enabled for standard and you can safely eslint-disable this rule around this piece of code.

Additionally you could consider returning false instead of rejecting, i.e. don't use errors for flow control. Consider if a user forgets to add a rejection handler, then their unhandled rejection hander will fire with a "reason" that doesn't have a stack trace, which could make it very difficult to discover where the rejection was coming from.

Alternatively you could reject a custom UserCancelledError Error subclass instance which gets around the issue without any surprises in the code or for a consumer. Just make sure you expose something that allows consumers to reliably check for that specific error. I.e. you should never have an exception/rejection handler that assumes the type of error/rejection without checking it, especially since syntax/reference errors are also caught by these. Nothing more frustrating than discovering the program was trying to tell you why it was failing but you ignored and suppressed the error, assuming the error was something else.

@Jakobud Jakobud closed this Apr 14, 2019

@lock lock bot locked as resolved and limited conversation to collaborators Jul 13, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
2 participants
You can’t perform that action at this time.