Repository navigation
--abort-on-uncaught-exception prevents domains from working at all (the process crashes) #836
Description
Activity
cc @cjihrig?
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.
I commented on that here. To summarize, it seems to me that
--abort_on_uncaught_exceptionis 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?
@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_exceptionflag. 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'); });
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?
Right, I think this is a flaw in the domains implementation, possibly coupled with a misunderstanding of what
--abort_on_uncaught_exceptiondoes. 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();
@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.
- added a commit that references this issue
on Feb 25, 2015 What do people use for post mortem debugging in production if they aren't using core files?
Simply write programs without flaws :)
It was fixed in node: nodejs/node-v0.x-archive#8631
This is a major issue that is preventing anyone from using domains and the very powerful
--abort-on-uncaught-exceptionflag.