FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

async_wrap: don't abort on callback exception · nodejs/node@747f107 · GitHub

/ node Public

Commit 747f107

Browse files
trevnorris
authored and
Myles Borins
committed
async_wrap: don't abort on callback exception
Rather than abort if the init/pre/post/final/destroy callbacks throw, force the exception to propagate and not be made catchable. This way the application is still not allowed to proceed but also allowed the location of the failure to print before exiting. Though the stack itself may not be of much use since all callbacks except init are called from the bottom of the call stack. /tmp/async-test.js:14 throw new Error('pre'); ^ Error: pre at InternalFieldObject.pre (/tmp/async-test.js:14:9) Ref: #7048 PR-URL: #5756 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
1 parent c06e2b0 commit 747f107

5 files changed

Lines changed: 135 additions & 12 deletions

File tree

‎src/async-wrap-inl.h‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,11 +51,15 @@ inline AsyncWrap::AsyncWrap(Environment* env,
5151
argv[3] = parent->object();
5252
}
5353

54+
v8::TryCatch try_catch(env->isolate());
55+
5456
v8::MaybeLocal<v8::Value> ret =
5557
init_fn->Call(env->context(), object, arraysize(argv), argv);
5658

57-
if (ret.IsEmpty())
58-
FatalError("node::AsyncWrap::AsyncWrap", "init hook threw");
59+
if (ret.IsEmpty()) {
60+
ClearFatalExceptionHandlers(env);
61+
FatalException(env->isolate(), try_catch);
62+
}
5963

6064
bits_ |= 1; // ran_init_callback() is true now.
6165
}
@@ -69,10 +73,13 @@ inline AsyncWrap::~AsyncWrap() {
6973
if (!fn.IsEmpty()) {
7074
v8::HandleScope scope(env()->isolate());
7175
v8::Local<v8::Value> uid = v8::Integer::New(env()->isolate(), get_uid());
76+
v8::TryCatch try_catch(env()->isolate());
7277
v8::MaybeLocal<v8::Value> ret =
7378
fn->Call(env()->context(), v8::Null(env()->isolate()), 1, &uid);
74-
if (ret.IsEmpty())
75-
FatalError("node::AsyncWrap::~AsyncWrap", "destroy hook threw");
79+
if (ret.IsEmpty()) {
80+
ClearFatalExceptionHandlers(env());
81+
FatalException(env()->isolate(), try_catch);
82+
}
7683
}
7784
}
7885

‎src/async-wrap.cc‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ using v8::HeapProfiler;
1818
using v8::Integer;
1919
using v8::Isolate;
2020
using v8::Local;
21+
using v8::MaybeLocal;
2122
using v8::Object;
2223
using v8::RetainedObjectInfo;
2324
using v8::TryCatch;
@@ -225,17 +226,28 @@ Local<Value> AsyncWrap::MakeCallback(const Local<Function> cb,
225226
}
226227

227228
if (ran_init_callback() && !pre_fn.IsEmpty()) {
228-
if (pre_fn->Call(context, 1, &uid).IsEmpty())
229-
FatalError("node::AsyncWrap::MakeCallback", "pre hook threw");
229+
TryCatch try_catch(env()->isolate());
230+
MaybeLocal<Value> ar = pre_fn->Call(env()->context(), context, 1, &uid);
231+
if (ar.IsEmpty()) {
232+
ClearFatalExceptionHandlers(env());
233+
FatalException(env()->isolate(), try_catch);
234+
return Local<Value>();
235+
}
230236
}
231237

232238
Local<Value> ret = cb->Call(context, argc, argv);
233239

234240
if (ran_init_callback() && !post_fn.IsEmpty()) {
235241
Local<Value> did_throw = Boolean::New(env()->isolate(), ret.IsEmpty());
236242
Local<Value> vals[] = { uid, did_throw };
237-
if (post_fn->Call(context, arraysize(vals), vals).IsEmpty())
238-
FatalError("node::AsyncWrap::MakeCallback", "post hook threw");
243+
TryCatch try_catch(env()->isolate());
244+
MaybeLocal<Value> ar =
245+
post_fn->Call(env()->context(), context, arraysize(vals), vals);
246+
if (ar.IsEmpty()) {
247+
ClearFatalExceptionHandlers(env());
248+
FatalException(env()->isolate(), try_catch);
249+
return Local<Value>();
250+
}
239251
}
240252

241253
if (ret.IsEmpty()) {

‎src/node.cc‎

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ using v8::Integer;
115115
using v8::Isolate;
116116
using v8::Local;
117117
using v8::Locker;
118+
using v8::MaybeLocal;
118119
using v8::Message;
119120
using v8::Number;
120121
using v8::Object;
@@ -1165,8 +1166,13 @@ Local<Value> MakeCallback(Environment* env,
11651166
}
11661167

11671168
if (ran_init_callback && !pre_fn.IsEmpty()) {
1168-
if (pre_fn->Call(object, 0, nullptr).IsEmpty())
1169-
FatalError("node::MakeCallback", "pre hook threw");
1169+
TryCatch try_catch(env->isolate());
1170+
MaybeLocal<Value> ar = pre_fn->Call(env->context(), object, 0, nullptr);
1171+
if (ar.IsEmpty()) {
1172+
ClearFatalExceptionHandlers(env);
1173+
FatalException(env->isolate(), try_catch);
1174+
return Local<Value>();
1175+
}
11701176
}
11711177

11721178
Local<Value> ret = callback->Call(recv, argc, argv);
@@ -1177,8 +1183,14 @@ Local<Value> MakeCallback(Environment* env,
11771183
// This needs to be fixed.
11781184
Local<Value> vals[] =
11791185
{ Undefined(env->isolate()).As<Value>(), did_throw };
1180-
if (post_fn->Call(object, arraysize(vals), vals).IsEmpty())
1181-
FatalError("node::MakeCallback", "post hook threw");
1186+
TryCatch try_catch(env->isolate());
1187+
MaybeLocal<Value> ar =
1188+
post_fn->Call(env->context(), object, arraysize(vals), vals);
1189+
if (ar.IsEmpty()) {
1190+
ClearFatalExceptionHandlers(env);
1191+
FatalException(env->isolate(), try_catch);
1192+
return Local<Value>();
1193+
}
11821194
}
11831195

11841196
if (ret.IsEmpty()) {
@@ -2352,6 +2364,25 @@ void OnMessage(Local<Message> message, Local<Value> error) {
23522364
}
23532365

23542366

2367+
void ClearFatalExceptionHandlers(Environment* env) {
2368+
Local<Object> process = env->process_object();
2369+
Local<Value> events =
2370+
process->Get(env->context(), env->events_string()).ToLocalChecked();
2371+
2372+
if (events->IsObject()) {
2373+
events.As<Object>()->Set(
2374+
env->context(),
2375+
OneByteString(env->isolate(), "uncaughtException"),
2376+
Undefined(env->isolate())).FromJust();
2377+
}
2378+
2379+
process->Set(
2380+
env->context(),
2381+
env->domain_string(),
2382+
Undefined(env->isolate())).FromJust();
2383+
}
2384+
2385+
23552386
static void Binding(const FunctionCallbackInfo<Value>& args) {
23562387
Environment* env = Environment::GetCurrent(args);
23572388

‎src/node_internals.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,11 @@ class ArrayBufferAllocator : public v8::ArrayBuffer::Allocator {
237237
Environment* env_;
238238
};
239239

240+
// Clear any domain and/or uncaughtException handlers to force the error's
241+
// propagation and shutdown the process. Use this to force the process to exit
242+
// by clearing all callbacks that could handle the error.
243+
void ClearFatalExceptionHandlers(Environment* env);
244+
240245
enum NodeInstanceType { MAIN, WORKER };
241246

242247
class NodeInstanceData {

‎test/parallel/test-async-wrap-throw-from-callback.js‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
'use strict';
2+
3+
require('../common');
4+
const async_wrap = process.binding('async_wrap');
5+
const assert = require('assert');
6+
const crypto = require('crypto');
7+
const domain = require('domain');
8+
const spawn = require('child_process').spawn;
9+
const callbacks = [ 'init', 'pre', 'post', 'destroy' ];
10+
const toCall = process.argv[2];
11+
var msgCalled = 0;
12+
var msgReceived = 0;
13+
14+
function init() {
15+
if (toCall === 'init')
16+
throw new Error('init');
17+
}
18+
function pre() {
19+
if (toCall === 'pre')
20+
throw new Error('pre');
21+
}
22+
function post() {
23+
if (toCall === 'post')
24+
throw new Error('post');
25+
}
26+
function destroy() {
27+
if (toCall === 'destroy')
28+
throw new Error('destroy');
29+
}
30+
31+
if (typeof process.argv[2] === 'string') {
32+
async_wrap.setupHooks({ init, pre, post, destroy });
33+
async_wrap.enable();
34+
35+
process.on('uncaughtException', () => assert.ok(0, 'UNREACHABLE'));
36+
37+
const d = domain.create();
38+
d.on('error', () => assert.ok(0, 'UNREACHABLE'));
39+
d.run(() => {
40+
// Using randomBytes because timers are not yet supported.
41+
crypto.randomBytes(0, () => { });
42+
});
43+
44+
} else {
45+
46+
process.on('exit', (code) => {
47+
assert.equal(msgCalled, callbacks.length);
48+
assert.equal(msgCalled, msgReceived);
49+
});
50+
51+
callbacks.forEach((item) => {
52+
msgCalled++;
53+
54+
const child = spawn(process.execPath, [__filename, item]);
55+
var errstring = '';
56+
57+
child.stderr.on('data', (data) => {
58+
errstring += data.toString();
59+
});
60+
61+
child.on('close', (code) => {
62+
if (errstring.includes('Error: ' + item))
63+
msgReceived++;
64+
65+
assert.equal(code, 1, `${item} closed with code ${code}`);
66+
});
67+
});
68+
}

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL