Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

process: add exitWithException() #25715

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions doc/api/process.md
Original file line number Diff line number Diff line change
Expand Up @@ -1177,6 +1177,44 @@ a code.
Specifying a code to [`process.exit(code)`][`process.exit()`] will override any
previous setting of `process.exitCode`.

## process.exitWithException(error[, fromPromise])
<!-- YAML
added: REPLACEME
-->

* `error` {any}
* `fromPromise` {boolean}

Exit with the provided error value as if it was thrown from its original
context. This allows an error to re-enter Node.js's error logic unmodified,
and will use the default formatted error printing.

If an error stack is available on the value, this method of exiting does not add
additional context, unlike re-throwing using [`throw`][].

This method is particularly useful when listening to the
[`'unhandledRejection'`][] process event.

As an example:
```js
process.on('unhandledRejection', (err) => {
Copy link
Member

@joyeecheung joyeecheung May 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if this is uncaughtException? I think that results in a dead loop? I think we should at least document about this footgun, but maybe a better solution is to just implement this separately and exclude part of process._fatalException() - or otherwise this does not exit unconditionally any more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, throw seems to be short-cut somehow, but this blows the stack size. :/

Copy link
Member

@joyeecheung joyeecheung May 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we even supposed to emit 'uncaughtException' when this is called? If so is there any point of using this instead of...well, just throwing an uncaught exception? I can see it makes sense to add this method if it does not trigger 'uncaughtException', but not really if it does..

process.exitWithException(err);
});

Promise.reject(new Error('uh oh'));
```

Outputs something similar to the following, as is often desirable:
```console
throw.js:5
Promise.reject(new Error('uh oh'));
^

Error: uh oh
at Object.<anonymous> (throw.js:5:16)
<more internal stack lines>
```

## process.getegid()
<!-- YAML
added: v2.0.0
Expand Down Expand Up @@ -2291,6 +2329,7 @@ cases:
[`'exit'`]: #process_event_exit
[`'message'`]: child_process.html#child_process_event_message
[`'uncaughtException'`]: #process_event_uncaughtexception
[`'unhandledrejection'`]: #process_event_unhandledrejection
[`ChildProcess.disconnect()`]: child_process.html#child_process_subprocess_disconnect
[`ChildProcess.send()`]: child_process.html#child_process_subprocess_send_message_sendhandle_options_callback
[`ChildProcess`]: child_process.html#child_process_class_childprocess
Expand Down Expand Up @@ -2318,6 +2357,7 @@ cases:
[`require.main`]: modules.html#modules_accessing_the_main_module
[`require.resolve()`]: modules.html#modules_require_resolve_request_options
[`subprocess.kill()`]: child_process.html#child_process_subprocess_kill_signal
[`throw`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/throw
[`v8.setFlagsFromString()`]: v8.html#v8_v8_setflagsfromstring_flags
[Android building]: https://github.com/nodejs/node/blob/master/BUILDING.md#androidandroid-based-devices-eg-firefox-os
[Child Process]: child_process.html
Expand Down
5 changes: 5 additions & 0 deletions lib/internal/bootstrap/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,11 @@ if (isMainThread) {
process.dlopen = rawMethods.dlopen;
process.uptime = rawMethods.uptime;

Object.defineProperty(process, 'exitWithException', {
enumerable: true,
value: rawMethods.exitWithException
joyeecheung marked this conversation as resolved.
Show resolved Hide resolved
});

// TODO(joyeecheung): either remove them or make them public
process._getActiveRequests = rawMethods._getActiveRequests;
process._getActiveHandles = rawMethods._getActiveHandles;
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -819,7 +819,7 @@ Module.runMain = function() {
return loader.import(pathToFileURL(process.argv[1]).pathname);
})
.catch((e) => {
internalBinding('task_queue').triggerFatalException(
internalBinding('process_methods').exitWithException(
e,
true /* fromPromise */
);
Expand Down
5 changes: 2 additions & 3 deletions lib/internal/process/promises.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@ const {
kPromiseResolveAfterResolved,
kPromiseRejectAfterResolved
},
setPromiseRejectCallback,
triggerFatalException
setPromiseRejectCallback
} = internalBinding('task_queue');

// *Must* match Environment::TickInfo::Fields in src/env.h.
Expand Down Expand Up @@ -202,7 +201,7 @@ function fatalException(reason) {
);
err.code = 'ERR_UNHANDLED_REJECTION';
}
triggerFatalException(err, true /* fromPromise */);
process.exitWithException(err, true /* fromPromise */);
}

function listenForRejections() {
Expand Down
5 changes: 2 additions & 3 deletions lib/internal/process/task_queues.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@ const {
// Used to run V8's micro task queue.
runMicrotasks,
setTickCallback,
enqueueMicrotask,
triggerFatalException
enqueueMicrotask
} = internalBinding('task_queue');

const {
Expand Down Expand Up @@ -157,7 +156,7 @@ function runMicrotask() {
// https://bugs.chromium.org/p/v8/issues/detail?id=8326
// is resolved such that V8 triggers the fatal exception
// handler for microtasks.
triggerFatalException(error, false /* fromPromise */);
process.exitWithException(error, false /* fromPromise */);
} finally {
this.emitDestroy();
}
Expand Down
15 changes: 15 additions & 0 deletions src/node_process_methods.cc
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ using v8::Array;
using v8::ArrayBuffer;
using v8::BigUint64Array;
using v8::Context;
using v8::Exception;
using v8::Float64Array;
using v8::Function;
using v8::FunctionCallbackInfo;
Expand Down Expand Up @@ -401,6 +402,19 @@ static void ReallyExit(const FunctionCallbackInfo<Value>& args) {
env->Exit(code);
}

static void ExitWithException(const FunctionCallbackInfo<Value>& args) {
Isolate* isolate = args.GetIsolate();
Environment* env = Environment::GetCurrent(isolate);
Local<Value> exception = args[0];
Local<Message> message = Exception::CreateMessage(isolate, exception);
if (env != nullptr && env->abort_on_uncaught_exception()) {
ReportException(env, exception, message);
Abort();
}
bool from_promise = args[1]->IsTrue();
FatalException(isolate, exception, message, from_promise);
}

static void InitializeProcessMethods(Local<Object> target,
Local<Value> unused,
Local<Context> context,
Expand Down Expand Up @@ -435,6 +449,7 @@ static void InitializeProcessMethods(Local<Object> target,
env->SetMethod(target, "reallyExit", ReallyExit);
env->SetMethodNoSideEffect(target, "uptime", Uptime);
env->SetMethod(target, "patchProcessObject", PatchProcessObject);
env->SetMethod(target, "exitWithException", ExitWithException);
}

} // namespace node
Expand Down
15 changes: 0 additions & 15 deletions src/node_task_queue.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ namespace node {

using v8::Array;
using v8::Context;
using v8::Exception;
using v8::Function;
using v8::FunctionCallbackInfo;
using v8::Isolate;
Expand Down Expand Up @@ -122,27 +121,13 @@ static void SetPromiseRejectCallback(
env->set_promise_reject_callback(args[0].As<Function>());
}

static void TriggerFatalException(const FunctionCallbackInfo<Value>& args) {
Isolate* isolate = args.GetIsolate();
Environment* env = Environment::GetCurrent(isolate);
Local<Value> exception = args[0];
Local<Message> message = Exception::CreateMessage(isolate, exception);
if (env != nullptr && env->abort_on_uncaught_exception()) {
ReportException(env, exception, message);
Abort();
}
bool from_promise = args[1]->IsTrue();
FatalException(isolate, exception, message, from_promise);
}

static void Initialize(Local<Object> target,
Local<Value> unused,
Local<Context> context,
void* priv) {
Environment* env = Environment::GetCurrent(context);
Isolate* isolate = env->isolate();

env->SetMethod(target, "triggerFatalException", TriggerFatalException);
env->SetMethod(target, "enqueueMicrotask", EnqueueMicrotask);
env->SetMethod(target, "setTickCallback", SetTickCallback);
env->SetMethod(target, "runMicrotasks", RunMicrotasks);
Expand Down
12 changes: 12 additions & 0 deletions test/message/process-exit-with-exception.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
'use strict';

const common = require('../common');
common.disableCrashOnUnhandledRejection();

process.on('unhandledRejection', (err) => {
process.exitWithException(err);
});

Promise.reject(new Error('uh oh'));

setImmediate(common.mustNotCall);
12 changes: 12 additions & 0 deletions test/message/process-exit-with-exception.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
*test*message*process-exit-with-exception.js:*
Promise.reject(new Error('uh oh'));
^

Error: uh oh
at Object.<anonymous> (*test*message*process-exit-with-exception.js:*:*)
at *
at *
at *
at *
at *
at *