Add a new process.unhandledRejections runtime flag

[nylen]

Is your feature request related to a problem? Please describe.

Unwieldy or incorrect behavior related to stack traces from unhandled promise rejections, depending on command-line options used.
Follow-up to #32312 (comment) and #17871.

Describe the solution you'd like

Add a new runtime flag:
process.unhandledRejections = 'strict';
This should behave the same way as setting --unhandled-rejections=strict when launching Node.js.

Relevant documentation for already existing feature: https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode
This new flag should also be documented near here: https://nodejs.org/api/process.html#process_process_throwdeprecation

After landing #32312, this situation has gotten better. However the stack trace from such an error is still overly long and unwieldy (lots of extraneous information):

Stack trace with `process.throwDeprecation = true;` after #32312

$ echo 'process.throwDeprecation = true; async function x() { throw new Error( "aaaa" ) }; x()' > test.js; ~/code/node/out/Release/node test.js; rm test.js(node:7885) UnhandledPromiseRejectionWarning: Error: aaaa
    at x (/home/james/test.js:1:61)
    at Object.<anonymous> (/home/james/test.js:1:84)
    at Module._compile (internal/modules/cjs/loader.js:1147:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1167:10)
    at Module.load (internal/modules/cjs/loader.js:996:32)
    at Function.Module._load (internal/modules/cjs/loader.js:896:14)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:71:12)
    at internal/main/run_main_module.js:17:47
(node:7885) 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)
internal/process/warning.js:135
        throw warning;
        ^

DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
    at emitDeprecationWarning (internal/process/promises.js:161:11)
    at processPromiseRejections (internal/process/promises.js:213:13)
    at processTicksAndRejections (internal/process/task_queues.js:98:32) {
  name: 'DeprecationWarning',
  code: 'DEP0018'
}

With --unhandled-rejections=strict the output is somewhat cleaner:

Stack trace with `--unhandled-rejections=strict`

$ echo 'async function x() { throw new Error( "aaaa" ) }; x()' > test.js; ~/code/node/out/Release/node --unhandled-rejections=strict test.js; rm test.js
/home/james/test.js:1
async function x() { throw new Error( "aaaa" ) }; x()
                           ^

Error: aaaa
    at x (/home/james/test.js:1:28)
    at Object.<anonymous> (/home/james/test.js:1:51)
    at Module._compile (internal/modules/cjs/loader.js:1147:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1167:10)
    at Module.load (internal/modules/cjs/loader.js:996:32)
    at Function.Module._load (internal/modules/cjs/loader.js:896:14)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:71:12)
    at internal/main/run_main_module.js:17:47

Why a further change is needed

Currently there is not an easy and foolproof way to get guaranteed error exits from unhandled rejections, with clean stack traces. The best current solution is to change how all Node.js processes are launched and add --unhandled-rejections=strict to their command line. In a complex system, there are multiple ways that Node.js processes can be launched, so this is difficult to achieve fully.

This solution of adding a new runtime flag should allow us to just add a single line to our code instead, which should guarantee the desired behavior for all scripts that contain this line.

[mmarchini]

Would setting the flag via NODE_OPTIONS help the "complex systems" use case? Or maybe setting the unhandledRejection handler?

If we implement this, it shouldn't be possible to override --unhandled-rejection=strict with process.unhandledRejections = 'warn', as the goal of strict as a flag is to be absolute (which is why it supersedes the unhandledRejection handler).

[benjamingr]

I don't understand why this needs another programmatic interface and setting a process.on('unhandledRejection', handler isn't sufficient.

[nylen]

The problem with setting an unhandledRejection handler is that it's at least 4 lines of code in every script, and it's not trivial for most developers to decide what that handler should be. You guys probably know the error-handling behavior of Node.js inside out, because you know more about the internals. Developers like me who write Node.js code don't have easy access to that knowledge. I have more understanding of these issues now, but that's only because I've been looking at this issue since late 2017 (#17871).

I don't understand why this needs another programmatic interface and setting a process.on('unhandledRejection', handler isn't sufficient.

Because Node's default error handling behavior is unexpected, even arguably broken. Should we really have to set an unhandledRejection handler for every single script in order to be sure that errors are going to behave reasonably? How many people are going to get this handler subtly wrong due to this lack of a standard, easy way to enforce the desired behavior?

--unhandled-rejections=strict is the closest we can come to "correct behavior" today, but this needs to be easier to use and easily enforceable from within application code.

When strict is the default then this issue will mostly go away. That would be for a separate issue, but I haven't seen any clear info about the plan for that either.

Until then, I want to be able to add a single line of code to my scripts/applications, and know that it's going to work correctly as long as I have a recent version of Node. "Correctly" in this case means that unhandled errors should abort the process with a clean, clear stack trace.

Would setting the flag via NODE_OPTIONS help the "complex systems" use case?

Not really, because (1) this can't be set from within application code, and (2) even if it could, doing so would override anything else that had previously set NODE_OPTIONS.

[mmarchini]

@nylen thanks for sharing more context. I'm not objectively against it as I understand different environments and infrastructures have different constraints, but as I mentioned above, if we do implement it I'd like to ensure that the flag cannot be overriden (it might even make more sense to leave strict out of the options, and users can set throw instead, assuming #33475 lands).

I think if we can better understand your constraints, it'll be easier to come up with solutions to this problem and to move any changes forward. Is there anything else you can share about the infrastructure you're dealing with (is it serverless, containers, regular cloud instances, mainframes, local machines running Electron, are you starting child processes or clusters, etc.).

The problem with setting an unhandledRejection handler is that it's at least 4 lines of code in every script, and it's not trivial for most developers to decide what that handler should be.

It can be achieved with a single line of code (process.on("unhandledRejection", (e) => { throw e });), but I understand your point on "getting it right". An alternative is to use something like https://www.npmjs.com/package/make-promises-safe which ensures the appropriate handler is attached.

Because Node's default error handling behavior is unexpected, even arguably broken. Should we really have to set an unhandledRejection handler for every single script in order to be sure that errors are going to behave reasonably?

We are currently discussing the default behavior for unhandled rejections and the default might become (or not) throwing on unhandled rejections, so I'd wait until that decision is reached before moving forward with this.

Would setting the flag via NODE_OPTIONS help the "complex systems" use case?
Not really, because (1) this can't be set from within application code, and (2) even if it could, doing so would override anything else that had previously set NODE_OPTIONS.

Regarding (2): environment variables can be extended (well, concatenated, export NODE_OPTIONS="$NODE_OPTIONS --unhandled-rejections=strict), that's what we usually do with PATH, for example (export PATH="$PATH:/new/path). I understand that depending on the environment that might be more challenging though.

(1) makes me even more curious about your use case: it sounds like this is targeted at environments where you don't have control on how the Node.js process is launched (so likely a serverless infrastructure)? Please correct me if my assumption is wrong.

[benjamingr]

Because Node's default error handling behavior is unexpected, even arguably broken. Should we really have to set an unhandledRejection handler for every single script in order to be sure that errors are going to behave reasonably?

FWIW this is one of my biggest regrets and failures in Node and this is being fixed. @mmarchini wrote a survey nodejs/TSC#857 that is being sent to users now. Given the results the project TSC will make a decision and possibly land #33021 or a similar PR (hopefully).

You can work around the issue by setting --unhandled-rejections=strict as Matheus mentioned above.

FWIW:

. "Correctly" in this case means that unhandled errors should abort the process with a clean, clear stack trace.

process.on('unhandledRejection', e => { throw e; });

Should meet those constraints IIUC.

[nylen]

this [Node's default error handling behavior] is being fixed

Glad to hear it. I think we (programmers in general) learned a long time ago that unhandled errors should abort, any other behavior is asking for trouble in complex systems.

You can work around the issue by setting --unhandled-rejections=strict as Matheus mentioned above.

As I explained above, application code needs to be able to control this behavior.

Is there anything else you can share about the infrastructure you're dealing with (is it serverless, containers, regular cloud instances, mainframes, local machines running Electron, are you starting child processes or clusters, etc.). [...] (1) makes me even more curious about your use case: it sounds like this is targeted at environments where you don't have control on how the Node.js process is launched (so likely a serverless infrastructure)? Please correct me if my assumption is wrong.

I have dozens of Node.js projects running in different environments, being launched different ways. Cron jobs (system and cPanel), init scripts, things launched in bash running in tmux sessions, with nvm or another version manager sometimes loaded and sometimes not. Sometimes these processes are launched by running node app.js, or sometimes just ./app.js relying on the #!/usr/bin/env node line at the top of app.js.

So, in theory I have full control over all of this code and how it's launched, but in practice I don't, because I can't realistically look back through every place where a Node.js process is launched (including child processes) and confirm that it has the right command-line options.

Then, if I were to move my code to such a serverless environment as you suggested, it would suddenly break, unless the application code itself has full control over error handling behavior.

Many people are likely in similar situations, which is why I created this issue to further push towards a simple solution and further push towards "ideal" error handling behavior.

process.on('unhandledRejection', e => { throw e; });

I agree something like this is probably still the best solution today. However, this is still 3 lines of code under most formatting rulesets, and setting a runtime flag as suggested in this issue would still be an improvement, because I wouldn't have to test and understand as many subtleties of Node.js error handling in order to use it. Tools that don't make programmers think about such issues are a good thing.

Given the results [of nodejs/TSC#857] the project TSC will make a decision and possibly land #33021 or a similar PR

Thanks for the links. That would be a full fix which would eliminate the need for this issue as far as I'm concerned. I'll be following this with interest.

[nylen]

as I mentioned above, if we do implement it I'd like to ensure that the flag cannot be overriden (it might even make more sense to leave strict out of the options

Sounds fine, but I probably wouldn't leave strict out of the options, because then it wouldn't be possible to really enforce this behavior in all cases (if a library adds an unhandledRejection handler for example. Not sure why a library would do that but I bet some do...)

Maybe the runtime behavior should be whichever of --unhandled-rejections and process.unhandledRejections is "more strict" (in order, none, warn, warn-with-error, throw, strict).

[ljharb]

This seems very dangerous to me. I don't want to have to protect my node application against any arbitrary third-party code being able to change the unhandled rejections mode of my entire application, at any time.

[joaolucasl]

@ljharb's point of this giving third-party access to changing is very concerning to me too. I feel like working around this on user land would turn out being more complex than the event handler itself.

[benjamingr]

This seems very dangerous to me. I don't want to have to protect my node application against any arbitrary third-party code being able to change the unhandled rejections mode of my entire application, at any time.

To be fair, that's entirely possible today (and with sync exceptions) through the unhandledRejection global handler and we had debates about this. For example in bluebird there are "local" handlers where a different library copy can catch all rejections that originated in it (but not globally).

I think this problem is not solved well currently (boundaries) and adding this hook doesn't change that.

I am still not sure what's the issue with process.on('unhandledRejection', e => { throw e }) though it seems plenty easy.