domain.on('error') should suppress other uncaughtException handlers #4375

Closed
CrabBot opened this Issue Dec 5, 2012 · 4 comments

Comments

Projects
None yet
4 participants

CrabBot commented Dec 5, 2012

Coming from a mocha issue, I realized the following:

  • domains do not "catch" errors in the sense that uncaughtException will still fire, an implementation detail probably best kept out of userland.
  • In a sense, this is inconsistent with the EventEmitter API where EventEmitter.on('error') suppresses errors from being thrown, where as uncaughtException firing is a de facto failure to catch

Example:

process.on('uncaughtException', function(err) {
  console.error(err.stack)
  // Uh oh! Do as the docs recommend & bail
  process.exit()
})

var domain = require('domain').create()
domain.on('error', function(err) {
  // Proceed. We're totally cool
  console.error(err.stack)
  domain.dispose()
})
domain.run(function() {
  process.nextTick(function() {
    throw new Error('fail')
  })
})

A possible workaround would be to wrap your uncaughtException handler in a process.nextTick and then check for err.domain, but that means domains are backwards incompatible with existing uncaughtException handlers for example in 3rd party modules like mocha.

fengmk2 referenced this issue in baryshev/connect-domain Dec 26, 2012

Merged

add unittest and coverage #2

Member

fengmk2 commented Dec 26, 2012

After domain.on('error') , uncaughtException event still fire doubt to what I think.

isaacs commented Dec 26, 2012

Yes, we should move the default error handling function to a separate function that checks for a current domain, and emits error on it, or emits 'uncaughtException' on the process object.

@isaacs isaacs added a commit to isaacs/node-v0.x-archive that referenced this issue Dec 27, 2012

@isaacs isaacs domain: Do not use uncaughtException handler
This adds a process._fatalException method which is called into from
C++ in order to either emit the 'uncaughtException' method, or emit
'error' on the active domain.

The 'uncaughtException' event is an implementation detail that it would
be nice to deprecate one day, so exposing it as part of the domain
machinery is not ideal.

Fix #4375
d3c26b1

Guys, it seems a bit off topic, but what techniqie would you recommend to re-throw errors from nested domains' error handlers to their parents' error handlers?

isaacs commented Dec 27, 2012

@AlexeyKupershtokh You could do something like this:

var domain = require('domain');
var parentDomain = domain.create();
parentDomain.on('error', function(er) {
  console.error('parent got it:', er.message);
});

var myDomain = domain.create();
myDomain.on('error', function(er) {
  if (!myDomainShouldHandleThisError(er))
    parentDomain.emit('error', er);
});

myDomain.run(function() {
  setTimeout(function() {
    undefined_reference;
  });
});

function myDomainShouldHandleThisError(er) {
  // lazy kid...
  return false;
}

Output:

$ node dd.js 
parent got it: undefined_reference is not defined

isaacs closed this in 4401bb4 Dec 29, 2012

@isaacs isaacs added a commit to isaacs/node-v0.x-archive that referenced this issue Apr 2, 2013

@isaacs isaacs domain: Do not use uncaughtException handler
This adds a process._fatalException method which is called into from
C++ in order to either emit the 'uncaughtException' method, or emit
'error' on the active domain.

The 'uncaughtException' event is an implementation detail that it would
be nice to deprecate one day, so exposing it as part of the domain
machinery is not ideal.

Fix #4375
51c52fb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment