Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

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

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

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

CrabBot opened this issue Dec 5, 2012 · 4 comments
Labels

Comments

@CrabBot
Copy link

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
Copy link

fengmk2 commented Dec 26, 2012

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

@isaacs
Copy link

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 added a commit to isaacs/node-v0.x-archive that referenced this issue Dec 27, 2012
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 nodejs#4375
@AlexeyKupershtokh
Copy link

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
Copy link

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 isaacs closed this as completed in 4401bb4 Dec 29, 2012
isaacs added a commit to isaacs/node-v0.x-archive that referenced this issue Apr 2, 2013
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 nodejs#4375
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants