--abort-on-uncaught-exception prevents domains from working at all (the process crashes) #836

Activity

1. Fishrock123 commented on Feb 13, 2015

   

   Fishrock123

   on Feb 13, 2015

   Contributor

   More actions

   cc @cjihrig?
2. cjihrig commented on Feb 13, 2015

   

   cjihrig

   on Feb 13, 2015

   Contributor

   More actions

   The relevant PR is nodejs/node-v0.x-archive#8666, where @trevnorris expressed that the solution wouldn't completely work for node 0.12 because of promises. The solution also involved floating the v8 patch nodejs/node-v0.x-archive@fbff705, which we obviously don't want to do.

   I don't know the best way to proceed here. I'd like to hear from others in @iojs/tc

   EDIT: I think I saw something in IRC a while back about explicitly not porting this over.
3. bnoordhuis commented on Feb 14, 2015

   

   bnoordhuis

   on Feb 14, 2015

   Member

   More actions

   I commented on that here. To summarize, it seems to me that --abort_on_uncaught_exception is working as intended.

   Let's take a step back and outline what the desired behavior is. Dump core on uncaught exceptions except when there is an active domain?
4. cjihrig commented on Feb 14, 2015

   

   cjihrig

   on Feb 14, 2015

   Contributor

   More actions

   @geek may want to chime in here, but I would expect the following program to behave the same with, and without, the --abort_on_uncaught_exception flag. With the flag, the process aborts. Without it, the domain is able to catch the error.

   var domain = require('domain');
   var d = domain.create();
   
   d.on('error', function(err) {
     console.log('domain caught ' + err);
   });
   
   d.run(function() {
     throw new Error('foo');
   });
5. geek commented on Feb 14, 2015

   

   geek

   on Feb 14, 2015

   MemberAuthor

   More actions

   My expectation is that a core is created whenever a process would normally crash. The flag should not cause a domain to stop working.

   Without this fixed, how are you expected to do any post mortem debugging and use domains in your application?
6. bnoordhuis commented on Feb 14, 2015

   

   bnoordhuis

   on Feb 14, 2015

   Member

   More actions

   Right, I think this is a flaw in the domains implementation, possibly coupled with a misunderstanding of what --abort_on_uncaught_exception does. That flag means 'abort when there is no JS try/catch block on the stack' and indeed there isn't one in domain.run(). The patch below makes @cjihrig's test case work but it's not a general solution.

   The test case fails again when you wrap the throw in a process.nextTick() and that's because _tickDomainCallback() in src/node.js doesn't have a try/catch block, it only has a try/finally block. If you catch the exception, the process no longer aborts. Ditto for every other place where callbacks are invoked.

   Whoever wants to work on fixing this should probably explore alternatives and write up a change proposal first because just blindly adding try/catch blocks everywhere is not a great idea.

   diff --git a/lib/domain.js b/lib/domain.js
   index c666fb5..5f8e6d4 100644
   --- a/lib/domain.js
   +++ b/lib/domain.js
   @@ -183,16 +183,20 @@ Domain.prototype.run = function(fn) {
      var ret;
   
      this.enter();
   -  if (arguments.length >= 2) {
   -    var len = arguments.length;
   -    var args = new Array(len - 1);
   +  try {
   +    if (arguments.length >= 2) {
   +      var len = arguments.length;
   +      var args = new Array(len - 1);
   
   -    for (var i = 1; i < len; i++)
   -      args[i - 1] = arguments[i];
   +      for (var i = 1; i < len; i++)
   +        args[i - 1] = arguments[i];
   
   -    ret = fn.apply(this, args);
   -  } else {
   -    ret = fn.call(this);
   +      ret = fn.apply(this, args);
   +    } else {
   +      ret = fn.call(this);
   +    }
   +  } catch (er) {
   +    this.emit('error', er);
      }
      this.exit();
   
7. geek

   added

   confirmed-bugIssues and PRs for confirmed bugs.Issues and PRs for confirmed bugs.

   on Feb 16, 2015
8. geek commented on Feb 16, 2015

   

   geek

   on Feb 16, 2015

   MemberAuthor

   More actions

   @bnoordhuis hopefully this bug will get fixed soonish... it's definitely keeping me from wanting to switch to io.js for production.

   What do people use for post mortem debugging in production if they aren't using core files? I've tried heap snapshots, but the /proc tooling in SmartOS is incredibly useful.
9. chrisdickinson

   added a commit that references this issue on Feb 25, 2015

   src: fix domains + --abort-on-uncaught-exception

   ...

   0af4c9e
10. Fishrock123

    closed this as completedon Feb 26, 2015
11. dougwilson commented on Feb 26, 2015

    

    dougwilson

    on Feb 26, 2015

    Member

    More actions

    What do people use for post mortem debugging in production if they aren't using core files?

    Simply write programs without flaws :)