Conversation
There was a problem hiding this comment.
If process.domain is set, don't fire an uncaughtException.
|
r+ on the first two commits, if you want to break those out to a new PR. Is there a reason to merge this before Domain support is implemented enough to actually work? Without support for hooking async events, this is basically a broken try-catch. In addition to Node-defined things, like Because of how it works, Domain support has to invade every other part of the system. I'm not entirely convinced we want to go down this rabbit hole just yet. Is there something blocking on this, or even a compelling use case? |
|
This PR would unblock code which relies on but not extensively uses the This provides a starting point for other contributors to submit patches against if they want to build out And a weak technical argument: since the domain error case presumes you're going to do graceful system shutdown, even implementing domain support improperly will, in the worst case, abort the VM on error, which on Tessel frees all allocated resources anyway. |
On master,
require('domain')throws an error. This patch