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

src: only call .ReThrow() if not terminating · nodejs/node@bd40a12 · GitHub

/ node Public

Commit bd40a12

Browse files
addaleax
authored and
rvagg
committed
src: only call .ReThrow() if not terminating
Otherwise, it looks like a `null` exception is being thrown. PR-URL: #26130 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
1 parent 818b280 commit bd40a12

5 files changed

Lines changed: 38 additions & 16 deletions

File tree

‎src/module_wrap.cc‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -158,12 +158,13 @@ void ModuleWrap::New(const FunctionCallbackInfo<Value>& args) {
158158
Context::Scope context_scope(context);
159159
ScriptCompiler::Source source(source_text, origin);
160160
if (!ScriptCompiler::CompileModule(isolate, &source).ToLocal(&module)) {
161-
CHECK(try_catch.HasCaught());
162-
CHECK(!try_catch.Message().IsEmpty());
163-
CHECK(!try_catch.Exception().IsEmpty());
164-
AppendExceptionLine(env, try_catch.Exception(), try_catch.Message(),
165-
ErrorHandlingMode::MODULE_ERROR);
166-
try_catch.ReThrow();
161+
if (try_catch.HasCaught() && !try_catch.HasTerminated()) {
162+
CHECK(!try_catch.Message().IsEmpty());
163+
CHECK(!try_catch.Exception().IsEmpty());
164+
AppendExceptionLine(env, try_catch.Exception(), try_catch.Message(),
165+
ErrorHandlingMode::MODULE_ERROR);
166+
try_catch.ReThrow();
167+
}
167168
return;
168169
}
169170
}
@@ -245,13 +246,12 @@ void ModuleWrap::Instantiate(const FunctionCallbackInfo<Value>& args) {
245246
Local<Context> context = obj->context_.Get(isolate);
246247
Local<Module> module = obj->module_.Get(isolate);
247248
TryCatchScope try_catch(env);
248-
Maybe<bool> ok = module->InstantiateModule(context, ResolveCallback);
249+
USE(module->InstantiateModule(context, ResolveCallback));
249250

250251
// clear resolve cache on instantiate
251252
obj->resolve_cache_.clear();
252253

253-
if (!ok.FromMaybe(false)) {
254-
CHECK(try_catch.HasCaught());
254+
if (try_catch.HasCaught() && !try_catch.HasTerminated()) {
255255
CHECK(!try_catch.Message().IsEmpty());
256256
CHECK(!try_catch.Exception().IsEmpty());
257257
AppendExceptionLine(env, try_catch.Exception(), try_catch.Message(),
@@ -300,6 +300,8 @@ void ModuleWrap::Evaluate(const FunctionCallbackInfo<Value>& args) {
300300

301301
// Convert the termination exception into a regular exception.
302302
if (timed_out || received_signal) {
303+
if (!env->is_main_thread() && env->is_stopping_worker())
304+
return;
303305
env->isolate()->CancelTerminateExecution();
304306
// It is possible that execution was terminated by another timeout in
305307
// which this timeout is nested, so check whether one of the watchdogs
@@ -312,7 +314,8 @@ void ModuleWrap::Evaluate(const FunctionCallbackInfo<Value>& args) {
312314
}
313315

314316
if (try_catch.HasCaught()) {
315-
try_catch.ReThrow();
317+
if (!try_catch.HasTerminated())
318+
try_catch.ReThrow();
316319
return;
317320
}
318321

@@ -375,7 +378,6 @@ void ModuleWrap::GetError(const FunctionCallbackInfo<Value>& args) {
375378
ASSIGN_OR_RETURN_UNWRAP(&obj, args.This());
376379

377380
Local<Module> module = obj->module_.Get(isolate);
378-
379381
args.GetReturnValue().Set(module->GetException());
380382
}
381383

‎src/node_contextify.cc‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,8 @@ void ContextifyContext::MakeContext(const FunctionCallbackInfo<Value>& args) {
252252
ContextifyContext* context = new ContextifyContext(env, sandbox, options);
253253

254254
if (try_catch.HasCaught()) {
255-
try_catch.ReThrow();
255+
if (!try_catch.HasTerminated())
256+
try_catch.ReThrow();
256257
return;
257258
}
258259

@@ -729,7 +730,8 @@ void ContextifyScript::New(const FunctionCallbackInfo<Value>& args) {
729730
if (v8_script.IsEmpty()) {
730731
DecorateErrorStack(env, try_catch);
731732
no_abort_scope.Close();
732-
try_catch.ReThrow();
733+
if (!try_catch.HasTerminated())
734+
try_catch.ReThrow();
733735
TRACE_EVENT_NESTABLE_ASYNC_END0(
734736
TRACING_CATEGORY_NODE2(vm, script),
735737
"ContextifyScript::New",
@@ -922,6 +924,8 @@ bool ContextifyScript::EvalMachine(Environment* env,
922924

923925
// Convert the termination exception into a regular exception.
924926
if (timed_out || received_signal) {
927+
if (!env->is_main_thread() && env->is_stopping_worker())
928+
return false;
925929
env->isolate()->CancelTerminateExecution();
926930
// It is possible that execution was terminated by another timeout in
927931
// which this timeout is nested, so check whether one of the watchdogs
@@ -944,7 +948,8 @@ bool ContextifyScript::EvalMachine(Environment* env,
944948
// letting try_catch catch it.
945949
// If execution has been terminated, but not by one of the watchdogs from
946950
// this invocation, this will re-throw a `null` value.
947-
try_catch.ReThrow();
951+
if (!try_catch.HasTerminated())
952+
try_catch.ReThrow();
948953

949954
return false;
950955
}
@@ -1098,8 +1103,10 @@ void ContextifyContext::CompileFunction(
10981103
context_extensions.size(), context_extensions.data(), options);
10991104

11001105
if (maybe_fn.IsEmpty()) {
1101-
DecorateErrorStack(env, try_catch);
1102-
try_catch.ReThrow();
1106+
if (try_catch.HasCaught() && !try_catch.HasTerminated()) {
1107+
DecorateErrorStack(env, try_catch);
1108+
try_catch.ReThrow();
1109+
}
11031110
return;
11041111
}
11051112
Local<Function> fn = maybe_fn.ToLocalChecked();

‎test/fixtures/es-modules/import-process-exit.mjs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
import exit from "./process-exit.mjs";

‎test/fixtures/es-modules/process-exit.mjs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
process.exit(42);
2+
export default null;

‎test/parallel/test-worker-esm-exit.js‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
'use strict';
2+
const common = require('../common');
3+
const fixtures = require('../common/fixtures');
4+
const assert = require('assert');
5+
const { Worker } = require('worker_threads');
6+
7+
const w = new Worker(fixtures.path('es-modules/import-process-exit.mjs'),
8+
{ execArgv: ['--experimental-modules'] });
9+
w.on('error', common.mustNotCall());
10+
w.on('exit', (code) => assert.strictEqual(code, 42));

0 commit comments

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