Domains on rejected promises in unhandledRejection handler

[ORESoftware]

I am on Node.js version 9-12. This code was working on 9,10,11, but changed with version 12:

  process.on('unhandledRejection', (e, p) => {
    
    let d = null;
    
    if (p.domain && p.domain.havenUuid) {
      d = p.domain;   
    } else if (process.domain && process.domain.havenUuid) {
      d = process.domain;
    }

     // more code here can be ignored

});

What I do is mark a domain with a property "havenUuid", if it has that property, it's a domain that I created/control.

What's happening starting with version 12 is that rejected promises no longer have an associated domain property attached to them when they reach the unhandledRejection handler. Is this an expected change in version 12? Because relevant domains were attached to rejected promises in versions 9, 10, 11, etc.

I am guessing that domains are no longer attached to rejected promises because doing so introduced bugs or something? I assume it's not a bug that domains are no longer attached to rejected promises in unhandledRejection handlers, but who knows.

[ORESoftware]

Here is a repro:

'use strict';

const Domain = require('domain');
const d = Domain.create();

process.once('unhandledRejection', (r, p) => {
  console.log(p.domain);  // on versions 9, 10, 11, p.domain is defined, on version 12, it is *undefined*
});

d.once('error', () => {
  console.log('domain caught');
});


d.run(() => {
  Promise.resolve(null).then(() => {
    throw 'foo';
  });
});

[ORESoftware]

I did some more research, p.domain is not defined starting with 12.0.0, so I assume a deliberate decision to omit it, any docs on this? In the release notes, I don't see anything in 12.0.0 which mentions this change: https://github.com/nodejs/node/blob/master/doc/changelogs/CHANGELOG_V12.md#2019-04-23-version-1200-current-bethgriggs

here was my original issue: nodejs/help#2110

[ORESoftware]

@benjamingr you know anything about this?

[devsnek]

probably related to the missing async context in #28317

[ORESoftware]

@devsnek is there a way to demonstrate that the two problems are related?

[devsnek]

@ORESoftware domain uses async_hooks, so if the context is wrong for async hooks, it would be wrong for domain too.

[ORESoftware]

do you think this could get fixed before version 13? I could try to help

[benjamingr]

Sure, probably look at Anna's PR for adding it in. Honestly I don't use domains and don't know the code well enough. CC @addaleax

[ORESoftware]

@addaleax any idea how to fix?

[ORESoftware]

just making a bump to ensure it gets looked at ty

[devsnek]

#26794

[ORESoftware]

thanks all, just tried this on Node.js 13.11, and still broken:

'use strict';

const Domain = require('domain');
const d = Domain.create();

process.once('unhandledRejection', (r, p) => {
  console.log(p.domain);  // on versions 9, 10, 11, p.domain is defined, on version 12, it is *undefined*
});

d.once('error', () => {
  console.log('domain caught');
});


d.run(() => {
  Promise.resolve(null).then(() => {
    throw 'foo';
  });
});

I got "undefined" logged, but I am looking for a domain instance to get logged, as it used to be in version 9,10,11. Starting with version 12.0, it stopped working as before.

[benjamingr]

I think I fixed this pretty recently for the REPL PR can someone check?

[ORESoftware]

@benjamingr this is looking good in Node v15.3.0, according to my testing, glad to see it fixed