From 3ff001f79e667ea93d1ec867e911b2f07297c773 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Mon, 15 Apr 2019 22:19:59 +0200 Subject: [PATCH 1/2] worker: handle exception when creating execArgv errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Handle possible JS exceptions that can occur by returning to JS land immediately. The motivation for this change is that `USE(….FromJust());` is an anti-pattern, and `.FromJust()` with an unused return value is superseded by `.Check()`. However, in this case, checking that the operation succeeded is not necessary. Refs: https://github.com/nodejs/node/pull/27162 --- src/node_worker.cc | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/node_worker.cc b/src/node_worker.cc index bdf324fa047624..2976302bb4b9b7 100644 --- a/src/node_worker.cc +++ b/src/node_worker.cc @@ -459,13 +459,15 @@ void Worker::New(const FunctionCallbackInfo& args) { // The first argument is program name. invalid_args.erase(invalid_args.begin()); if (errors.size() > 0 || invalid_args.size() > 0) { - v8::Local error = - ToV8Value(env->context(), - errors.size() > 0 ? errors : invalid_args) - .ToLocalChecked(); + v8::Local error; + if (!ToV8Value(env->context(), + errors.size() > 0 ? errors : invalid_args) + .ToLocal(&error)) { + return; + } Local key = FIXED_ONE_BYTE_STRING(env->isolate(), "invalidExecArgv"); - USE(args.This()->Set(env->context(), key, error).FromJust()); + USE(args.This()->Set(env->context(), key, error)); return; } } From 75fbf8d5561cfbd1d072104bedd4d4d944a210f2 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Tue, 16 Apr 2019 13:50:58 +0200 Subject: [PATCH 2/2] fixup! worker: handle exception when creating execArgv errors --- src/node_worker.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/node_worker.cc b/src/node_worker.cc index 2976302bb4b9b7..b9143eca7aafdd 100644 --- a/src/node_worker.cc +++ b/src/node_worker.cc @@ -467,6 +467,8 @@ void Worker::New(const FunctionCallbackInfo& args) { } Local key = FIXED_ONE_BYTE_STRING(env->isolate(), "invalidExecArgv"); + // Ignore the return value of Set() because exceptions bubble up to JS + // when we return anyway. USE(args.This()->Set(env->context(), key, error)); return; }