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

catch-or-return options to report then(a, b) or then(null, b) #52

Closed
Jessidhia opened this issue Jan 4, 2017 · 4 comments · Fixed by #522
Closed

catch-or-return options to report then(a, b) or then(null, b) #52

Jessidhia opened this issue Jan 4, 2017 · 4 comments · Fixed by #522

Comments

@Jessidhia
Copy link

makePromise().then(thenHandler, catchHandler) is incorrectly considered correct by the always-catch plugin when allowThen: true is specified.

While the catchHandler will handle a rejection of the Promise returned by makePromise(), it will not handle any rejection that happens in the thenHandler; it needs to be handled by its own catch handler, which could be a .then(null /* | undefined */, catchHandler) according to allowThen: true.

@xjamundx
Copy link
Contributor

xjamundx commented Jan 9, 2017

I believe that the current behavior is correct as some people would rather do without catch or a second then altogether.

I see your point that they are going to be missing any crashes in the initial handler. I'd consider an allowThenStrict or something 🤔

What do you think?

@okcoker
Copy link

okcoker commented May 1, 2019

I'd consider an allowThenStrict or something

Would this disallow the second argument being used within .then() altogether? I'm all for having that as I don't think that's currently possible right?

@ota-meshi ota-meshi changed the title always-catch false negative with allowThen: true catch-or-return false negative with allowThen: true Sep 30, 2022
@brettz9
Copy link
Member

brettz9 commented Jul 20, 2024

I think we may need allowThenStrict (to err on then(a, b) but pass for then(null, b)) and preventThenRejectHandler (which errs on both forms) as separate options.

@brettz9 brettz9 changed the title catch-or-return false negative with allowThen: true catch-or-return options to report then(a, b) or then(null, b) Jul 20, 2024
@brettz9
Copy link
Member

brettz9 commented Jul 20, 2024

Maybe it is better for at least the latter option to be its own rule (and as might be tracked by #217).

brettz9 added a commit to brettz9/eslint-plugin-promise that referenced this issue Jul 30, 2024
MichaelDeBoey pushed a commit to brettz9/eslint-plugin-promise that referenced this issue Oct 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants