Promise.reject() crashes repl when using --unhandled-rejections=strict

[dfabulich]

• Version: 14.11.0
• Platform: macOS Catalina 10.15.6

I have also reproduced this in Node 12.18.4.

What steps will reproduce the bug?

Launch the repl with this command:

node --unhandled-rejections=strict

(--unhandled-rejections=throw has the same issue.)

On the repl command line, type Promise.reject().

What is the expected behavior?

When an uncaught error is thrown in the repl, the repl should print "Uncaught error" without terminating the process. For example:

> throw new Error()
Uncaught Error
> void setTimeout(_=>{throw new Error()}, 0)
undefined
> Uncaught Error

What do you see instead?

The process crashes.

> Promise.reject()
Promise { <rejected> undefined }
> internal/process/promises.js:213
        triggerUncaughtException(err, true /* fromPromise */);
        ^

[UnhandledPromiseRejection: This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). The promise rejected with the reason "undefined".] {
  code: 'ERR_UNHANDLED_REJECTION'
}

Additional information

This bug will become more important in Node 15 when --unhandled-rejections=throw becomes the default, per PR #33021. At that point, the bug will repro when launching node using the default settings with no flags.

[dfabulich]

FWIW, this bug is not a regression. It happens in Node 12 as well.

I'm curious to take a crack at this, but I have no idea where to begin. Can anyone suggest a starting point? (How do repl tests work…?)

[mmarchini]

Should repl ignore the flag as use warn /none instead? That's more similar to what happens with sync errors

[BridgeAR]

Setting it to none would silence such issues in the REPL and the main issue is the programmatic REPL instances. If these are started and we just set the flag to none, rejections in the "regular code" are also silenced. Otherwise it's a good solution (if set to warn).

[dfabulich]

That would fix the crash, but it would be a little noisy.

$ node --unhandled-rejections=warn
Welcome to Node.js v14.11.0.
Type ".help" for more information.
> Promise.reject()
Promise { <rejected> undefined }
> (node:66834) UnhandledPromiseRejectionWarning: undefined
(Use `node --trace-warnings ...` to show where the warning was created)
(node:66834) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)

I feel like I'd want to make the repl set an unhandledRejections hook to log a warning that wasn't so noisy.

Is there a good place I could add code like that?

[devsnek]

with throw mode, it should bubble up as an uncaught exception, the same way throwing does.

[mmarchini]

@devsnek yeah I'm a bit surprised with the behavior here honestly. I won't have time to look into it during the week but might have some time during the weekend.

[BridgeAR]

The reason why rejections behave differently is that we use domains to track uncaught exceptions in the REPL.

[mmarchini]

Didn't had time to look into it over the weekend, and I'm not sure if I'll have time to look into it during the week, so if anyone else wants to take a look go ahead. As a workaround we should probably let it to warn, and then keep this issue open to investigate making it work without changing the default.

[Aplet123]

With the new defaults added in v15, this issue is especially pertinent.

[benjamingr]

This is quite the blast from the past, I haven't touched domains in several years ^^ I will take a look.