Long term goals/roadmap of exiting on promises rejections

[misterdjules]

Looking at nodejs/node#20097, it seems that there is an intention to "abort by default" at some point in the future.

Is there a place where a longer term roadmap for those behaviors and which behavior would be the default one for which version of node is documented?

[benjamingr]

There's nodejs/promise-use-cases#27

[misterdjules]

There's nodejs/promise-use-cases#27

But that issue doesn't seem to accurately represent the work done at nodejs/node#20097. For instance, it doesn't mention the "ERROR" mode.

I thought I had also heard from @BridgeAR that he was working on a mode that exits/aborts synchronously on unhandled rejections, which is not mentioned in that issue. @BridgeAR Did I misunderstand that during our private discussion?

[benjamingr]

I thought I had also heard from @BridgeAR that he was working on a mode that exits/aborts synchronously on unhandled rejections

Well, I sort of just asked for it here: nodejs/promise-use-cases#27 (comment) since I wanted to use it for my own code and I suspect a lot of people might too

[misterdjules]

To be more specific, nodejs/promise-use-cases#27 documents two modes: warn and error on GC.

nodejs/node#20097 documents 4 modes: silent, warn, error on GC, error, and its comments mention a "strict" mode that would exit/abort synchronously.

https://github.com/nodejs/node/pull/20097/files#r194422687 mentions that eventually "aborting" (not clear in which case) should be the default.

And finally, as you mentioned, nodejs/promise-use-cases#27 (comment) suggests a strict mode too.

So I'm a bit lost when it comes to understanding:

1. what the overall roadmap is
2. what behaviors have reached consensus (it seems none have reached consensus except for "silent" and "warn")
3. the rationale for each behavior

Would it be possible to document that in a centralized place? Mentioning that information in the description of nodejs/node#20097 would be useful, and updating that as feedback is given/received.

Updating the description of nodejs/promise-use-cases#27 seems like it would be useful too, it seems a bit out of date compared to latest discussions.

[benjamingr]

what the overall roadmap is

The overall roadmap is as outlined in nodejs/promise-use-cases#27 :

• warn on unhandled rejection is the current default
• exit on GC + warn on nextTick will be the default in Node.js 11+ with backporting

In addition, two opt-in modes will be possible:

• NONE - Ignore unhandled rejections (this is already possible today, and will continue being possible - though an environment variable will let it be exposed).
• STRICT - which was renamed to ERROR - Throw on unhandled rejections if not handled synchronously. Note this does not throw synchronously but rather before other microticks.

To get the STRICT behaviour today you can use https://github.com/addaleax/set-promise-reject-callback - I asked for it since I think people will use it (I am) but it won't be the default.

[BridgeAR]

@benjamingr

Note this does not throw synchronously but rather before other microticks.

That is not correct. It'll work like this:

let count = 0;

process.on('uncaughtException', common.mustCall((err, origin) => {
  assert.strictEqual(origin, 'FROM_PROMISE');
  count++;
}, 2));

// Trigger the first uncaughtException synchronously.
Promise.reject('foo');

assert.strictEqual(count, 1);

[benjamingr]

Will it work like this @BridgeAR ?

let count = 0;

process.on('uncaughtException', common.mustCall((err, origin) => {
  assert.strictEqual(origin, 'FROM_PROMISE');
  count++;
}, 2));

// Trigger the first uncaughtException synchronously.
Promise.reject('foo').catch(() => {});

assert.strictEqual(count, 1);

[BridgeAR]

Yes, it does.

[benjamingr]

Huh, I just tested and you're right. This also throws:

(async () => {
  try {
    await Promise.reject(new Error('rejection'));
  } catch (e) {
    console.error('Caught');
  }
})();

I thought it would run after the JS code is done but before other microtasks. I'll investigate it further.

[BridgeAR]

I scratched the suggested STRICT mode due to not being able to implement it in a way that was usable overall (sadly). The strictest mode possible is to trigger errors on the next tick.

[benjamingr]

@BridgeAR what about tagging it in SetPromiseRejectCallback and then exiting if the promise is still rejected before running the microtasks?

[BridgeAR]

I can not think of a way to get #10 (comment) to not trigger an uncaught exception when doing that. I gave it a try but it did not work. I also talked with @hashseed about it but there is AFAIK no way to do this.

[benjamingr]

@BridgeAR just to clarify to make sure I understand: There's no way to get code to run before other microtasks since we might be during a runMicrotasks() call that's inside V8 and internal.

When we are pumping our own nextTick queue we can run code after each callback - but we can't do the same when we're inside V8's queue which we use for promises since _tickCallback just runs the V8 side with isolate->RunMicrotasks.

Is that understanding correct?

[BridgeAR]

Yes, exactly that.

[bmeurer]

@benjamingr @BridgeAR I'm not entirely sure what you need here? Do you want to run the callback after every microtask or just once the microtask queue is empty?