diff --git a/doc/api/cli.md b/doc/api/cli.md index 2d8cccb8a46382..b8ea4826dec2fe 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -183,6 +183,10 @@ added: v0.10 Aborting instead of exiting causes a core file to be generated for post-mortem analysis using a debugger (such as `lldb`, `gdb`, and `mdb`). +*Note*: If this flag is passed, the behavior can still be set to not abort +through [`process.setUncaughtExceptionCaptureCallback()`][] (and through usage +of the `domain` module that uses it). + ### `--trace-warnings` + +* Returns: {boolean} + +Indicates whether a callback has been set using +[`process.setUncaughtExceptionCaptureCallback()`][]. + ## process.hrtime([time]) + +* `fn` {Function|null} + +The `process.setUncaughtExceptionCapture` function sets a function that will +be invoked when an uncaught exception occurs, which will receive the exception +value itself as its first argument. + +If such a function is set, the [`process.on('uncaughtException')`][] event will +not be emitted. If `--abort-on-uncaught-exception` was passed from the +command line or set through [`v8.setFlagsFromString()`][], the process will +not abort. + +To unset the capture function, `process.setUncaughtExceptionCapture(null)` +may be used. Calling this method with a non-`null` argument while another +capture function is set will throw an error. + +*Note*: Using this function is mutually exclusive with using the +deprecated [`domain`][] built-in module. + ## process.stderr * {Stream} @@ -1921,6 +1954,7 @@ cases: [`JSON.stringify` spec]: https://tc39.github.io/ecma262/#sec-json.stringify [`console.error()`]: console.html#console_console_error_data_args [`console.log()`]: console.html#console_console_log_data_args +[`domain`]: domain.html [`end()`]: stream.html#stream_writable_end_chunk_encoding_callback [`net.Server`]: net.html#net_class_net_server [`net.Socket`]: net.html#net_class_net_socket @@ -1930,11 +1964,14 @@ cases: [`process.exit()`]: #process_process_exit_code [`process.exitCode`]: #process_process_exitcode [`process.kill()`]: #process_process_kill_pid_signal +[`process.on('uncaughtException')`]: process.html#process_event_uncaughtexception +[`process.setUncaughtExceptionCaptureCallback()`]: process.html#process_process_setuncaughtexceptioncapturecallback_fn [`promise.catch()`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/catch [`require()`]: globals.html#globals_require [`require.main`]: modules.html#modules_accessing_the_main_module [`require.resolve()`]: modules.html#modules_require_resolve_request_options [`setTimeout(fn, 0)`]: timers.html#timers_settimeout_callback_delay_args +[`v8.setFlagsFromString()`]: v8.html#v8_v8_setflagsfromstring_flags [Child Process]: child_process.html [Cluster]: cluster.html [Duplex]: stream.html#stream_duplex_and_transform_streams diff --git a/lib/domain.js b/lib/domain.js index dc3c550866c924..6c85ca2b17277b 100644 --- a/lib/domain.js +++ b/lib/domain.js @@ -28,6 +28,7 @@ const util = require('util'); const EventEmitter = require('events'); +const errors = require('internal/errors'); const { createHook } = require('async_hooks'); // communicate with events module, but don't require that @@ -81,19 +82,77 @@ const asyncHook = createHook({ } }); +// When domains are in use, they claim full ownership of the +// uncaught exception capture callback. +if (process.hasUncaughtExceptionCaptureCallback()) { + throw new errors.Error('ERR_DOMAIN_CALLBACK_NOT_AVAILABLE'); +} + +// Get the stack trace at the point where `domain` was required. +const domainRequireStack = new Error('require(`domain`) at this point').stack; + +const { setUncaughtExceptionCaptureCallback } = process; +process.setUncaughtExceptionCaptureCallback = function(fn) { + const err = + new errors.Error('ERR_DOMAIN_CANNOT_SET_UNCAUGHT_EXCEPTION_CAPTURE'); + err.stack = err.stack + '\n' + '-'.repeat(40) + '\n' + domainRequireStack; + throw err; +}; + // It's possible to enter one domain while already inside // another one. The stack is each entered domain. const stack = []; exports._stack = stack; -process._setupDomainUse(stack); +process._setupDomainUse(); -class Domain extends EventEmitter { +function updateExceptionCapture() { + if (stack.every((domain) => domain.listenerCount('error') === 0)) { + setUncaughtExceptionCaptureCallback(null); + } else { + setUncaughtExceptionCaptureCallback(null); + setUncaughtExceptionCaptureCallback((er) => { + return process.domain._errorHandler(er); + }); + } +} + + +process.on('newListener', (name, listener) => { + if (name === 'uncaughtException' && + listener !== domainUncaughtExceptionClear) { + // Make sure the first listener for `uncaughtException` always clears + // the domain stack. + process.removeListener(name, domainUncaughtExceptionClear); + process.prependListener(name, domainUncaughtExceptionClear); + } +}); + +process.on('removeListener', (name, listener) => { + if (name === 'uncaughtException' && + listener !== domainUncaughtExceptionClear) { + // If the domain listener would be the only remaining one, remove it. + const listeners = process.listeners('uncaughtException'); + if (listeners.length === 1 && listeners[0] === domainUncaughtExceptionClear) + process.removeListener(name, domainUncaughtExceptionClear); + } +}); +function domainUncaughtExceptionClear() { + stack.length = 0; + exports.active = process.domain = null; + updateExceptionCapture(); +} + + +class Domain extends EventEmitter { constructor() { super(); this.members = []; asyncHook.enable(); + + this.on('removeListener', updateExceptionCapture); + this.on('newListener', updateExceptionCapture); } } @@ -131,14 +190,14 @@ Domain.prototype._errorHandler = function _errorHandler(er) { // prevent the process 'uncaughtException' event from being emitted // if a listener is set. if (EventEmitter.listenerCount(this, 'error') > 0) { + // Clear the uncaughtExceptionCaptureCallback so that we know that, even + // if technically the top-level domain is still active, it would + // be ok to abort on an uncaught exception at this point + setUncaughtExceptionCaptureCallback(null); try { - // Set the _emittingTopLevelDomainError so that we know that, even - // if technically the top-level domain is still active, it would - // be ok to abort on an uncaught exception at this point - process._emittingTopLevelDomainError = true; caught = this.emit('error', er); } finally { - process._emittingTopLevelDomainError = false; + updateExceptionCapture(); } } } else { @@ -161,11 +220,13 @@ Domain.prototype._errorHandler = function _errorHandler(er) { if (this === exports.active) { stack.pop(); } + updateExceptionCapture(); if (stack.length) { exports.active = process.domain = stack[stack.length - 1]; - caught = process._fatalException(er2); + caught = process.domain._errorHandler(er2); } else { - caught = false; + // Pass on to the next exception handler. + throw er2; } } } @@ -173,8 +234,7 @@ Domain.prototype._errorHandler = function _errorHandler(er) { // Exit all domains on the stack. Uncaught exceptions end the // current tick and no domains should be left on the stack // between ticks. - stack.length = 0; - exports.active = process.domain = null; + domainUncaughtExceptionClear(); return caught; }; @@ -185,6 +245,7 @@ Domain.prototype.enter = function() { // to push it onto the stack so that we can pop it later. exports.active = process.domain = this; stack.push(this); + updateExceptionCapture(); }; @@ -198,6 +259,7 @@ Domain.prototype.exit = function() { exports.active = stack[stack.length - 1]; process.domain = exports.active; + updateExceptionCapture(); }; diff --git a/lib/internal/bootstrap_node.js b/lib/internal/bootstrap_node.js index dfb8b88ee440bb..2bb18741472c1f 100644 --- a/lib/internal/bootstrap_node.js +++ b/lib/internal/bootstrap_node.js @@ -9,6 +9,7 @@ (function(process) { let internalBinding; + const exceptionHandlerState = { captureFn: null }; function startup() { const EventEmitter = NativeModule.require('events'); @@ -34,6 +35,7 @@ const _process = NativeModule.require('internal/process'); _process.setupConfig(NativeModule._source); _process.setupSignalHandlers(); + _process.setupUncaughtExceptionCapture(exceptionHandlerState); NativeModule.require('internal/process/warning').setup(); NativeModule.require('internal/process/next_tick').setup(); NativeModule.require('internal/process/stdio').setup(); @@ -376,8 +378,10 @@ // that threw and was never cleared. So clear it now. async_id_fields[kInitTriggerAsyncId] = 0; - if (process.domain && process.domain._errorHandler) - caught = process.domain._errorHandler(er); + if (exceptionHandlerState.captureFn !== null) { + exceptionHandlerState.captureFn(er); + caught = true; + } if (!caught) caught = process.emit('uncaughtException', er); diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 535cd1937d6c33..aadb0684d0d778 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -171,6 +171,13 @@ E('ERR_CRYPTO_SIGN_KEY_REQUIRED', 'No key provided to sign'); E('ERR_CRYPTO_TIMING_SAFE_EQUAL_LENGTH', 'Input buffers must have the same length'); E('ERR_DNS_SET_SERVERS_FAILED', 'c-ares failed to set servers: "%s" [%s]'); +E('ERR_DOMAIN_CALLBACK_NOT_AVAILABLE', + 'A callback was registered through ' + + 'process.setUncaughtExceptionCaptureCallback(), which is mutually ' + + 'exclusive with using the `domain` module'); +E('ERR_DOMAIN_CANNOT_SET_UNCAUGHT_EXCEPTION_CAPTURE', + 'The `domain` module is in use, which is mutually exclusive with calling ' + + 'process.setUncaughtExceptionCaptureCallback()'); E('ERR_ENCODING_INVALID_ENCODED_DATA', 'The encoded data was not valid for encoding %s'); E('ERR_ENCODING_NOT_SUPPORTED', 'The "%s" encoding is not supported'); @@ -339,6 +346,9 @@ E('ERR_TRANSFORM_ALREADY_TRANSFORMING', 'Calling transform done when still transforming'); E('ERR_TRANSFORM_WITH_LENGTH_0', 'Calling transform done when writableState.length != 0'); +E('ERR_UNCAUGHT_EXCEPTION_CAPTURE_ALREADY_SET', + '`process.setupUncaughtExceptionCapture()` was called while a capture ' + + 'callback was already active'); E('ERR_UNESCAPED_CHARACTERS', '%s contains unescaped characters'); E('ERR_UNHANDLED_ERROR', (err) => { diff --git a/lib/internal/process.js b/lib/internal/process.js index 56ff1f66397628..e58b83d21631ff 100644 --- a/lib/internal/process.js +++ b/lib/internal/process.js @@ -247,6 +247,34 @@ function setupRawDebug() { }; } + +function setupUncaughtExceptionCapture(exceptionHandlerState) { + // This is a typed array for faster communication with JS. + const shouldAbortOnUncaughtToggle = process._shouldAbortOnUncaughtToggle; + delete process._shouldAbortOnUncaughtToggle; + + process.setUncaughtExceptionCaptureCallback = function(fn) { + if (fn === null) { + exceptionHandlerState.captureFn = fn; + shouldAbortOnUncaughtToggle[0] = 1; + return; + } + if (typeof fn !== 'function') { + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'fn', + ['Function', 'null']); + } + if (exceptionHandlerState.captureFn !== null) { + throw new errors.Error('ERR_UNCAUGHT_EXCEPTION_CAPTURE_ALREADY_SET'); + } + exceptionHandlerState.captureFn = fn; + shouldAbortOnUncaughtToggle[0] = 0; + }; + + process.hasUncaughtExceptionCaptureCallback = function() { + return exceptionHandlerState.captureFn !== null; + }; +} + module.exports = { setup_performance, setup_cpuUsage, @@ -256,5 +284,6 @@ module.exports = { setupKillAndExit, setupSignalHandlers, setupChannel, - setupRawDebug + setupRawDebug, + setupUncaughtExceptionCapture }; diff --git a/src/env-inl.h b/src/env-inl.h index b4ec89c0a0d135..bc2d68c1ce41f4 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -268,6 +268,7 @@ inline Environment::Environment(IsolateData* isolate_data, emit_napi_warning_(true), makecallback_cntr_(0), scheduled_immediate_count_(isolate_, 1), + should_abort_on_uncaught_toggle_(isolate_, 1), #if HAVE_INSPECTOR inspector_agent_(new inspector::Agent(this)), #endif @@ -305,6 +306,9 @@ inline Environment::Environment(IsolateData* isolate_data, performance_state_->milestones[ performance::NODE_PERFORMANCE_MILESTONE_V8_START] = performance::performance_v8_start; + + // By default, always abort when --abort-on-uncaught-exception was passed. + should_abort_on_uncaught_toggle_[0] = 1; } inline Environment::~Environment() { @@ -408,6 +412,11 @@ inline void Environment::set_abort_on_uncaught_exception(bool value) { abort_on_uncaught_exception_ = value; } +inline AliasedBuffer& +Environment::should_abort_on_uncaught_toggle() { + return should_abort_on_uncaught_toggle_; +} + inline std::vector* Environment::destroy_async_id_list() { return &destroy_async_id_list_; } diff --git a/src/env.h b/src/env.h index f0444b24d4fa86..c2e87c1dac9415 100644 --- a/src/env.h +++ b/src/env.h @@ -134,7 +134,6 @@ class ModuleWrap; V(dns_txt_string, "TXT") \ V(domain_string, "domain") \ V(emit_string, "emit") \ - V(emitting_top_level_domain_error_string, "_emittingTopLevelDomainError") \ V(exchange_string, "exchange") \ V(enumerable_string, "enumerable") \ V(idle_string, "idle") \ @@ -309,7 +308,6 @@ class ModuleWrap; V(internal_binding_cache_object, v8::Object) \ V(buffer_prototype_object, v8::Object) \ V(context, v8::Context) \ - V(domains_stack_array, v8::Array) \ V(http2ping_constructor_template, v8::ObjectTemplate) \ V(http2stream_constructor_template, v8::ObjectTemplate) \ V(inspector_console_api_object, v8::Object) \ @@ -571,8 +569,15 @@ class Environment { void PrintSyncTrace() const; inline void set_trace_sync_io(bool value); + // This stores whether the --abort-on-uncaught-exception flag was passed + // to Node. inline bool abort_on_uncaught_exception() const; inline void set_abort_on_uncaught_exception(bool value); + // This is a pseudo-boolean that keeps track of whether an uncaught exception + // should abort the process or not if --abort-on-uncaught-exception was + // passed to Node. If the flag was not passed, it is ignored. + inline AliasedBuffer& + should_abort_on_uncaught_toggle(); // The necessary API for async_hooks. inline double new_async_id(); @@ -699,6 +704,7 @@ class Environment { std::vector destroy_async_id_list_; AliasedBuffer scheduled_immediate_count_; + AliasedBuffer should_abort_on_uncaught_toggle_; performance::performance_state* performance_state_ = nullptr; std::map performance_marks_; diff --git a/src/node.cc b/src/node.cc index 5d175df3560032..33101dcd79c6e4 100644 --- a/src/node.cc +++ b/src/node.cc @@ -1107,66 +1107,13 @@ void* ArrayBufferAllocator::Allocate(size_t size) { namespace { -bool DomainHasErrorHandler(const Environment* env, - const Local& domain) { - HandleScope scope(env->isolate()); - - Local domain_event_listeners_v = domain->Get(env->events_string()); - if (!domain_event_listeners_v->IsObject()) - return false; - - Local domain_event_listeners_o = - domain_event_listeners_v.As(); - - Local domain_error_listeners_v = - domain_event_listeners_o->Get(env->error_string()); - - if (domain_error_listeners_v->IsFunction() || - (domain_error_listeners_v->IsArray() && - domain_error_listeners_v.As()->Length() > 0)) - return true; - - return false; -} - -bool DomainsStackHasErrorHandler(const Environment* env) { - HandleScope scope(env->isolate()); - - if (!env->using_domains()) - return false; - - Local domains_stack_array = env->domains_stack_array().As(); - if (domains_stack_array->Length() == 0) - return false; - - uint32_t domains_stack_length = domains_stack_array->Length(); - for (uint32_t i = domains_stack_length; i > 0; --i) { - Local domain_v = domains_stack_array->Get(i - 1); - if (!domain_v->IsObject()) - return false; - - Local domain = domain_v.As(); - if (DomainHasErrorHandler(env, domain)) - return true; - } - - return false; -} - - bool ShouldAbortOnUncaughtException(Isolate* isolate) { HandleScope scope(isolate); - Environment* env = Environment::GetCurrent(isolate); - Local process_object = env->process_object(); - Local emitting_top_level_domain_error_key = - env->emitting_top_level_domain_error_string(); - bool isEmittingTopLevelDomainError = - process_object->Get(emitting_top_level_domain_error_key)->BooleanValue(); - - return isEmittingTopLevelDomainError || !DomainsStackHasErrorHandler(env); + return env->should_abort_on_uncaught_toggle()[0]; } + Local GetDomainProperty(Environment* env, Local object) { Local domain_v = object->GetPrivate(env->context(), env->domain_private_symbol()) @@ -1216,9 +1163,6 @@ void SetupDomainUse(const FunctionCallbackInfo& args) { HandleScope scope(env->isolate()); - CHECK(args[0]->IsArray()); - env->set_domains_stack_array(args[0].As()); - // Do a little housekeeping. env->process_object()->Delete( env->context(), @@ -3521,6 +3465,13 @@ void SetupProcessObject(Environment* env, scheduled_immediate_count, env->scheduled_immediate_count().GetJSArray()).FromJust()); + auto should_abort_on_uncaught_toggle = + FIXED_ONE_BYTE_STRING(env->isolate(), "_shouldAbortOnUncaughtToggle"); + CHECK(process->Set(env->context(), + should_abort_on_uncaught_toggle, + env->should_abort_on_uncaught_toggle().GetJSArray()) + .FromJust()); + // -e, --eval if (eval_string) { READONLY_PROPERTY(process, diff --git a/test/parallel/test-domain-load-after-set-uncaught-exception-capture.js b/test/parallel/test-domain-load-after-set-uncaught-exception-capture.js new file mode 100644 index 00000000000000..9e438368d63207 --- /dev/null +++ b/test/parallel/test-domain-load-after-set-uncaught-exception-capture.js @@ -0,0 +1,18 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); + +process.setUncaughtExceptionCaptureCallback(common.mustNotCall()); + +common.expectsError( + () => require('domain'), + { + code: 'ERR_DOMAIN_CALLBACK_NOT_AVAILABLE', + type: Error, + message: /^A callback was registered.*with using the `domain` module/ + } +); + +process.setUncaughtExceptionCaptureCallback(null); + +assert.doesNotThrow(() => require('domain')); diff --git a/test/parallel/test-domain-set-uncaught-exception-capture-after-load.js b/test/parallel/test-domain-set-uncaught-exception-capture-after-load.js new file mode 100644 index 00000000000000..e7cbffd00758e2 --- /dev/null +++ b/test/parallel/test-domain-set-uncaught-exception-capture-after-load.js @@ -0,0 +1,28 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); + +(function foobar() { + require('domain'); +})(); + +assert.throws( + () => process.setUncaughtExceptionCaptureCallback(common.mustNotCall()), + (err) => { + common.expectsError( + { + code: 'ERR_DOMAIN_CANNOT_SET_UNCAUGHT_EXCEPTION_CAPTURE', + type: Error, + message: /^The `domain` module is in use, which is mutually/ + } + )(err); + + assert(err.stack.includes('-'.repeat(40)), + `expected ${err.stack} to contain dashes`); + + const location = `at foobar (${__filename}:`; + assert(err.stack.includes(location), + `expected ${err.stack} to contain ${location}`); + return true; + } +); diff --git a/test/parallel/test-process-exception-capture-errors.js b/test/parallel/test-process-exception-capture-errors.js new file mode 100644 index 00000000000000..7053497adaf873 --- /dev/null +++ b/test/parallel/test-process-exception-capture-errors.js @@ -0,0 +1,22 @@ +'use strict'; +const common = require('../common'); + +common.expectsError( + () => process.setUncaughtExceptionCaptureCallback(42), + { + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError, + message: 'The "fn" argument must be one of type Function or null' + } +); + +process.setUncaughtExceptionCaptureCallback(common.mustNotCall()); + +common.expectsError( + () => process.setUncaughtExceptionCaptureCallback(common.mustNotCall()), + { + code: 'ERR_UNCAUGHT_EXCEPTION_CAPTURE_ALREADY_SET', + type: Error, + message: /setupUncaughtExceptionCapture.*called while a capture callback/ + } +); diff --git a/test/parallel/test-process-exception-capture-should-abort-on-uncaught-setflagsfromstring.js b/test/parallel/test-process-exception-capture-should-abort-on-uncaught-setflagsfromstring.js new file mode 100644 index 00000000000000..de14177b45a609 --- /dev/null +++ b/test/parallel/test-process-exception-capture-should-abort-on-uncaught-setflagsfromstring.js @@ -0,0 +1,13 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const v8 = require('v8'); + +assert.strictEqual(process.hasUncaughtExceptionCaptureCallback(), false); + +v8.setFlagsFromString('--abort-on-uncaught-exception'); +// This should make the process not crash even though the flag was passed. +process.setUncaughtExceptionCaptureCallback(common.mustCall((err) => { + assert.strictEqual(err.message, 'foo'); +})); +throw new Error('foo'); diff --git a/test/parallel/test-process-exception-capture-should-abort-on-uncaught.js b/test/parallel/test-process-exception-capture-should-abort-on-uncaught.js new file mode 100644 index 00000000000000..f9e685a86ea2e6 --- /dev/null +++ b/test/parallel/test-process-exception-capture-should-abort-on-uncaught.js @@ -0,0 +1,12 @@ +// Flags: --abort-on-uncaught-exception +'use strict'; +const common = require('../common'); +const assert = require('assert'); + +assert.strictEqual(process.hasUncaughtExceptionCaptureCallback(), false); + +// This should make the process not crash even though the flag was passed. +process.setUncaughtExceptionCaptureCallback(common.mustCall((err) => { + assert.strictEqual(err.message, 'foo'); +})); +throw new Error('foo'); diff --git a/test/parallel/test-process-exception-capture.js b/test/parallel/test-process-exception-capture.js new file mode 100644 index 00000000000000..c84d3459e2318f --- /dev/null +++ b/test/parallel/test-process-exception-capture.js @@ -0,0 +1,13 @@ +// Flags: --abort-on-uncaught-exception +'use strict'; +const common = require('../common'); +const assert = require('assert'); + +assert.strictEqual(process.hasUncaughtExceptionCaptureCallback(), false); + +// This should make the process not crash even though the flag was passed. +process.setUncaughtExceptionCaptureCallback(common.mustCall((err) => { + assert.strictEqual(err.message, 'foo'); +})); +process.on('uncaughtException', common.mustNotCall()); +throw new Error('foo');