Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Domains created inside uncaughtException event handler don't allow catching errors thrown inside the domain.run() #22400

Open
m0uneer opened this issue Aug 19, 2018 · 2 comments
Labels
domain Issues and PRs related to the domain subsystem.

Comments

@m0uneer
Copy link

m0uneer commented Aug 19, 2018

  • Version: v10.5.0
  • Platform: Linux pc 4.13.0-41-generic # 46~16.04.1-Ubuntu SMP Thu May 3 10:06:43 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux
  • Subsystem:
const domain = require('domain');

function captureUncaughtException() {
  const createdDomain = domain.create();
  createdDomain.on('error', () => console.log('Caught error within the domain!'));
  createdDomain.run(notExistingFn);
}

process.on('uncaughtException', captureUncaughtException.bind(this));
throw new Error();

// Expected output:
// Caught error within the domain!

// Actual output:
// ReferenceError: notExistingFn is not defined

And on contrary to the above scenario, if we call captureUncaughtException() directly, it behaves normally.

captureUncaughtException();

// Output:
// Caught error within the domain!
@m0uneer m0uneer changed the title Domains created inside uncaughtException event don't allow catching errors thrown inside the domain.run() Domains created inside uncaughtException event handler don't allow catching errors thrown inside the domain.run() Aug 23, 2018
@addaleax addaleax added the domain Issues and PRs related to the domain subsystem. label Aug 24, 2018
@addaleax
Copy link
Member

The reason for this is that when the fatal exception handler throws (and all uncaughtException/domain handlers are invoked from there), Node.js always exits immediately.

It might be possible to fix this, but I’m not sure whether we want to do so, given that domain has been deprecated for a long time and this is a very edge-case-y scenario.

I think patches would be welcome in either case.

@m0uneer
Copy link
Author

m0uneer commented Aug 25, 2018

Thanks, @addaleax for commenting on this.

So, I think that domains internally depend on a special uncaughtException event to catch errors. They don't catch sync errors that are swallowed and not cause this special uncaughtException event to be emitted.

I can also closely explain my thoughts on how a domain works as follows:

function domainRun({runCb, onErrorCb}) {
  process.on('specialUncaughtException', onErrorCb);
  runCb();
}

domainRun({
  runCb: notExistingFn,
  onErrorCb: () => console.log('Caught error within the domain!'),
});

So and to wrap up, the cases (I personally met) that highlight the problem are:

  • Domains created inside uncaughtException event handler that throws a sync error (Explained in the issue description)
  • Domains created inside a try/catch that throws a sync error as the following:
const domain = require('domain');

function foo() {
  const createdDomain = domain.create();
  createdDomain.on('error', () => console.log('Caught error within the `foo()` domain!'));
  createdDomain.run(notExistingFn);
}

try {
  foo();
} catch (err) {
  console.log('Caught error within the `foo()` try/catch!');
}

Workarounds

  1. Convert the logic to be async by using process.nextTick() or a timer method every time we use domain.run(). This guarantees that the error is always saved from being swallowed. Example:
createdDomain.run(() => process.nextTick(notExistingFn));
  1. Using try/catch in conjunction with the domains
function onError(err) { 
  //... 
}

createdDomain.on('error', onError).run(() => {
  try {
    notExistingFn();
  } catch(err) {
    onError(err);
  }
});

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain Issues and PRs related to the domain subsystem.
Projects
None yet
Development

No branches or pull requests

2 participants