Since #7008 was added to the 8 milestone for tracking, adding this as well. I think we'll really need to have good docs to make sure folks don't misuse this.

Btw, referencing #7008 & nodejs/node#51292 & reduxjs/redux-toolkit#4102: we should definitely mention strategies for frameworks/libraries to mark Promises as safe (or not expose them).

Note: #8573 has a good suggestion:

I fixed a no-floating-promises error by changing:

- promise.catch(() => {}).then(() => {})
+ promise.catch(() => {}).finally(() => {})

I think this is a good change! Since my .catch() handler couldn't reject, it's probably not even a change.

The .catch().then() form is mentioned on MDN and triggers this rule (playground). So it would be helpful if the docs on this rule included an example of the suggested refactor.

Marking as blocked on PRs implementing #7008 and #8404, as I'd neglected to note this publicly till now. 🙂

Now that #9186 is merged, I believe this is almost unblocked. I'd like to tackle it alongside #8517.

#8404 would be great too. Marking as blocked on that.

When Not To Use It
if you're not worried about crashes from floating or misused Promises

It would be nice for there to be some discussion of browser vs server in the docs. I believe that at least in the past that Node would crash upon there being an unhandled promise rejection though I don't know if that's still the case. Do we need to worry about it in the browser or is this only a Node.js thing? I see on StackOverflow that browsers have not swallowed such errors in a long time.

I also wonder about browser/server with no-misused-promises. A large fraction of the times it triggers is on browser event handlers:

	addEventListener('popstate', async (event) => {
		...
	});

Does this matter? The docs say the rule triggers when "they are not handled properly". But what would be the proper handling here? I wonder if this rule is almost always a false positive in browser code like this or if there's something I'm actually supposed to be doing here.

in the past that Node would crash upon there being an unhandled promise rejection

Throwing on unhandled rejections by default was shipped by Node.js in its v15: nodejs/node#20392 -> nodejs/node@3b10f7f -> https://github.com/nodejs/node/releases/tag/v15.0.0.

browser/server

We should note that whether the runtime (browser, edge worker, full server, ...) handles the Promise isn't the main point of these rules. Roughly all runtimes provide some way of being notified of unhandled rejections (e.g. the unhandledrejection event, but handling them in application logic is a separate matter. The idea that some area of code might start some work and not handle that work's rejection matters.

Does this matter?

Great question, and something we'll definitely want to address in the docs. Yes it does! If that async (event) => { ... } function rejects, nothing will handle that rejection by default. addEventListener doesn't know to.

declare function doSomeWork(): Promise<void>;

addEventListener("popstate", async (event) => {
  await doSomeWork();
  throw new Error("Who should handle me?");
});

But what would be the proper handling here?

Another great question we should handle in the docs! There's no one-size-fits-all solution because how each application might handle generalized or specific errors is different. Two common approaches we've seen have been:

• Having the equivalent of a root-level try catch and/or .finally
• Including a function like the following that registers errors in some app-specific handler:

  function handlePromiseSafely(doWork: () => Promise<unknown>): void {
    doWork().catch((error: unknown) => {
      // (or however you'd handle this)
      console.error("Oh no:", error);
    });
  }

Thanks so much for the detailed and thoughtful response!

Yes it does! If that async (event) => { ... } function rejects, nothing will handle that rejection by default. addEventListener doesn't know to.

Isn't that basically the same with a synchronous handler?

declare function doSomeWork(): void;

addEventListener("popstate", (event) => {
  doSomeWork();
  throw new Error("Who should handle me?");
});

When this rule is used to bubble up exceptions to try/catch blocks it feels useful. But when it's bubbling up exception to event handlers it just feels it's turning all promises into checked exceptions, which can be a bit annoying, and I don't know why async functions should be treated differently than synchronous functions in that case.

Including a function like the following that registers errors in some app-specific handler

Don't we already have this with onunhandledrejection? Why not just use that instead of wrapping every individual promise instance?

Isn't that basically the same with a synchronous handler?

For the case specifically of the DOM APIs, sure, yes. But if the user has their own:

function runAction(action: () => void) {
  console.log("before");
  action();
  console.log("this should only happen if the action didn't crash");
}

declare function doSomeWork: () => Promise<void>;

runAction(async () => {
  await doSomeWork();
  throw new Error("Who should handle me?");
});

Why not just use that instead of wrapping every individual promise instance?

That's one strategy that works for handling floating/miscellaneous exceptions. But see above for control flow issues.

https://jakearchibald.com/2023/unhandled-rejections has more funky situations.

Anyway, @benmccann you raise reasonable things to discuss but this getting off-topic for the specific issue of overhauling the docs. If you want to chat more could I ask you to make a Discussion or thread in the Discord? We can dive into deeper points there.

Per #8517 (comment), moving this to post-v8.