Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit bd40a12

Browse filesBrowse files
addaleaxrvagg
authored andcommitted
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
Copy full SHA for bd40a12

File tree

Expand file treeCollapse file tree

5 files changed

+38
-16
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

5 files changed

+38
-16
lines changed
Open diff view settings
Collapse file

‎src/module_wrap.cc‎

Copy file name to clipboardExpand all lines: src/module_wrap.cc
+13-11Lines 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

Collapse file

‎src/node_contextify.cc‎

Copy file name to clipboardExpand all lines: src/node_contextify.cc
+12-5Lines 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();
Collapse file
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
import exit from "./process-exit.mjs";
Collapse file
+2Lines 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;
Collapse file
+10Lines 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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.