Skip to content

Commit 575ced8

Browse files
committed
module: print location of unsettled top-level await in entry points
When the entry point is a module and the graph it imports still contains unsettled top-level await when the Node.js instance finishes the event loop, search from the entry point module for unsettled top-level await and print their location. To avoid unnecessary overhead, we register a promise that only gets settled when the entry point graph evaluation returns from await, and only search the module graph if it's still unsettled by the time the instance is exiting. This patch only handles this for entry point modules. Other kinds of modules are more complicated so will be left for the future. Drive-by: update the terminology "unfinished promise" to the more correct one "unsettled promise" in the codebase. PR-URL: #51999 Fixes: #42868 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
1 parent fb356b3 commit 575ced8

16 files changed

+192
-36
lines changed

doc/api/process.md

+2-2
Original file line numberDiff line numberDiff line change
@@ -3953,8 +3953,8 @@ cases:
39533953
and generally can only happen during development of Node.js itself.
39543954
* `12` **Invalid Debug Argument**: The `--inspect` and/or `--inspect-brk`
39553955
options were set, but the port number chosen was invalid or unavailable.
3956-
* `13` **Unfinished Top-Level Await**: `await` was used outside of a function
3957-
in the top-level code, but the passed `Promise` never resolved.
3956+
* `13` **Unsettled Top-Level Await**: `await` was used outside of a function
3957+
in the top-level code, but the passed `Promise` never settled.
39583958
* `14` **Snapshot Failure**: Node.js was started to build a V8 startup
39593959
snapshot and it failed because certain requirements of the state of
39603960
the application were not met.

lib/internal/modules/esm/hooks.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ const {
3232
ERR_METHOD_NOT_IMPLEMENTED,
3333
ERR_WORKER_UNSERIALIZABLE_ERROR,
3434
} = require('internal/errors').codes;
35-
const { exitCodes: { kUnfinishedTopLevelAwait } } = internalBinding('errors');
35+
const { exitCodes: { kUnsettledTopLevelAwait } } = internalBinding('errors');
3636
const { URL } = require('internal/url');
3737
const { canParse: URLCanParse } = internalBinding('url');
3838
const { receiveMessageOnPort } = require('worker_threads');
@@ -615,7 +615,7 @@ class HooksProxy {
615615
} while (response == null);
616616
debug('got sync response from worker', { method, args });
617617
if (response.message.status === 'never-settle') {
618-
process.exit(kUnfinishedTopLevelAwait);
618+
process.exit(kUnsettledTopLevelAwait);
619619
} else if (response.message.status === 'exit') {
620620
process.exit(response.message.body);
621621
}

lib/internal/modules/esm/loader.js

+4-4
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ class ModuleLoader {
182182
}
183183
}
184184

185-
async eval(source, url) {
185+
async eval(source, url, isEntryPoint = false) {
186186
const evalInstance = (url) => {
187187
const { ModuleWrap } = internalBinding('module_wrap');
188188
const { registerModule } = require('internal/modules/esm/utils');
@@ -201,7 +201,7 @@ class ModuleLoader {
201201
const job = new ModuleJob(
202202
this, url, undefined, evalInstance, false, false);
203203
this.loadCache.set(url, undefined, job);
204-
const { module } = await job.run();
204+
const { module } = await job.run(isEntryPoint);
205205

206206
return {
207207
__proto__: null,
@@ -311,9 +311,9 @@ class ModuleLoader {
311311
* module import.
312312
* @returns {Promise<ModuleExports>}
313313
*/
314-
async import(specifier, parentURL, importAttributes) {
314+
async import(specifier, parentURL, importAttributes, isEntryPoint = false) {
315315
const moduleJob = await this.getModuleJob(specifier, parentURL, importAttributes);
316-
const { module } = await moduleJob.run();
316+
const { module } = await moduleJob.run(isEntryPoint);
317317
return module.getNamespace();
318318
}
319319

lib/internal/modules/esm/module_job.js

+10-2
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,15 @@ const {
1717
StringPrototypeIncludes,
1818
StringPrototypeSplit,
1919
StringPrototypeStartsWith,
20+
globalThis,
2021
} = primordials;
2122

2223
const { ModuleWrap } = internalBinding('module_wrap');
23-
24+
const {
25+
privateSymbols: {
26+
entry_point_module_private_symbol,
27+
},
28+
} = internalBinding('util');
2429
const { decorateErrorStack, kEmptyObject } = require('internal/util');
2530
const {
2631
getSourceMapsEnabled,
@@ -213,8 +218,11 @@ class ModuleJob {
213218
return { __proto__: null, module: this.module };
214219
}
215220

216-
async run() {
221+
async run(isEntryPoint = false) {
217222
await this.instantiate();
223+
if (isEntryPoint) {
224+
globalThis[entry_point_module_private_symbol] = this.module;
225+
}
218226
const timeout = -1;
219227
const breakOnSigint = false;
220228
setHasStartedUserESMExecution();

lib/internal/modules/run_main.js

+11-16
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
const {
44
StringPrototypeEndsWith,
5+
globalThis,
56
} = primordials;
67

78
const { containsModuleSyntax } = internalBinding('contextify');
@@ -16,9 +17,12 @@ const {
1617
} = require('internal/process/execution');
1718
const {
1819
triggerUncaughtException,
19-
exitCodes: { kUnfinishedTopLevelAwait },
2020
} = internalBinding('errors');
21-
21+
const {
22+
privateSymbols: {
23+
entry_point_promise_private_symbol,
24+
},
25+
} = internalBinding('util');
2226
/**
2327
* Get the absolute path to the main entry point.
2428
* @param {string} main - Entry point path
@@ -102,20 +106,10 @@ function shouldUseESMLoader(mainPath) {
102106
return type === 'module';
103107
}
104108

105-
/**
106-
* Handle a Promise from running code that potentially does Top-Level Await.
107-
* In that case, it makes sense to set the exit code to a specific non-zero value
108-
* if the main code never finishes running.
109-
*/
110-
function handleProcessExit() {
111-
process.exitCode ??= kUnfinishedTopLevelAwait;
112-
}
113-
114109
/**
115110
* @param {function(ModuleLoader):ModuleWrap|undefined} callback
116111
*/
117112
async function asyncRunEntryPointWithESMLoader(callback) {
118-
process.on('exit', handleProcessExit);
119113
const cascadedLoader = require('internal/modules/esm/loader').getOrInitializeCascadedLoader();
120114
try {
121115
const userImports = getOptionValue('--import');
@@ -137,8 +131,6 @@ async function asyncRunEntryPointWithESMLoader(callback) {
137131
err,
138132
true, /* fromPromise */
139133
);
140-
} finally {
141-
process.off('exit', handleProcessExit);
142134
}
143135
}
144136

@@ -152,6 +144,10 @@ async function asyncRunEntryPointWithESMLoader(callback) {
152144
*/
153145
function runEntryPointWithESMLoader(callback) {
154146
const promise = asyncRunEntryPointWithESMLoader(callback);
147+
// Register the promise - if by the time the event loop finishes running, this is
148+
// still unsettled, we'll search the graph from the entry point module and print
149+
// the location of any unsettled top-level await found.
150+
globalThis[entry_point_promise_private_symbol] = promise;
155151
return promise;
156152
}
157153

@@ -171,7 +167,7 @@ function executeUserEntryPoint(main = process.argv[1]) {
171167
const mainURL = pathToFileURL(mainPath).href;
172168

173169
runEntryPointWithESMLoader((cascadedLoader) => {
174-
// Note that if the graph contains unfinished TLA, this may never resolve
170+
// Note that if the graph contains unsettled TLA, this may never resolve
175171
// even after the event loop stops running.
176172
return cascadedLoader.import(mainURL, undefined, { __proto__: null }, true);
177173
});
@@ -185,5 +181,4 @@ function executeUserEntryPoint(main = process.argv[1]) {
185181
module.exports = {
186182
executeUserEntryPoint,
187183
runEntryPointWithESMLoader,
188-
handleProcessExit,
189184
};

lib/internal/process/per_thread.js

-3
Original file line numberDiff line numberDiff line change
@@ -173,9 +173,6 @@ function wrapProcessMethods(binding) {
173173
memoryUsage.rss = rss;
174174

175175
function exit(code) {
176-
const { handleProcessExit } = require('internal/modules/run_main');
177-
process.off('exit', handleProcessExit);
178-
179176
if (arguments.length !== 0) {
180177
process.exitCode = code;
181178
}

src/api/embed_helpers.cc

+14-1
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,20 @@ Maybe<ExitCode> SpinEventLoopInternal(Environment* env) {
7373

7474
env->PrintInfoForSnapshotIfDebug();
7575
env->ForEachRealm([](Realm* realm) { realm->VerifyNoStrongBaseObjects(); });
76-
return EmitProcessExitInternal(env);
76+
Maybe<ExitCode> exit_code = EmitProcessExitInternal(env);
77+
if (exit_code.FromMaybe(ExitCode::kGenericUserError) !=
78+
ExitCode::kNoFailure) {
79+
return exit_code;
80+
}
81+
82+
auto unsettled_tla = env->CheckUnsettledTopLevelAwait();
83+
if (unsettled_tla.IsNothing()) {
84+
return Nothing<ExitCode>();
85+
}
86+
if (!unsettled_tla.FromJust()) {
87+
return Just(ExitCode::kUnsettledTopLevelAwait);
88+
}
89+
return Just(ExitCode::kNoFailure);
7790
}
7891

7992
struct CommonEnvironmentSetup::Impl {

src/env-inl.h

+4
Original file line numberDiff line numberDiff line change
@@ -371,6 +371,10 @@ inline void Environment::set_exiting(bool value) {
371371
exit_info_[kExiting] = value ? 1 : 0;
372372
}
373373

374+
inline bool Environment::exiting() const {
375+
return exit_info_[kExiting] == 1;
376+
}
377+
374378
inline ExitCode Environment::exit_code(const ExitCode default_code) const {
375379
return exit_info_[kHasExitCode] == 0
376380
? default_code

src/env.cc

+37
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#include "debug_utils-inl.h"
55
#include "diagnosticfilename-inl.h"
66
#include "memory_tracker-inl.h"
7+
#include "module_wrap.h"
78
#include "node_buffer.h"
89
#include "node_context_data.h"
910
#include "node_contextify.h"
@@ -50,6 +51,7 @@ using v8::HeapSpaceStatistics;
5051
using v8::Integer;
5152
using v8::Isolate;
5253
using v8::Local;
54+
using v8::Maybe;
5355
using v8::MaybeLocal;
5456
using v8::NewStringType;
5557
using v8::Number;
@@ -1228,6 +1230,41 @@ void Environment::AtExit(void (*cb)(void* arg), void* arg) {
12281230
at_exit_functions_.push_front(ExitCallback{cb, arg});
12291231
}
12301232

1233+
Maybe<bool> Environment::CheckUnsettledTopLevelAwait() {
1234+
HandleScope scope(isolate_);
1235+
Local<Context> ctx = context();
1236+
Local<Value> value;
1237+
1238+
Local<Value> entry_point_promise;
1239+
if (!ctx->Global()
1240+
->GetPrivate(ctx, entry_point_promise_private_symbol())
1241+
.ToLocal(&entry_point_promise)) {
1242+
return v8::Nothing<bool>();
1243+
}
1244+
if (!entry_point_promise->IsPromise()) {
1245+
return v8::Just(true);
1246+
}
1247+
if (entry_point_promise.As<Promise>()->State() !=
1248+
Promise::PromiseState::kPending) {
1249+
return v8::Just(true);
1250+
}
1251+
1252+
if (!ctx->Global()
1253+
->GetPrivate(ctx, entry_point_module_private_symbol())
1254+
.ToLocal(&value)) {
1255+
return v8::Nothing<bool>();
1256+
}
1257+
if (!value->IsObject()) {
1258+
return v8::Just(true);
1259+
}
1260+
Local<Object> object = value.As<Object>();
1261+
CHECK(BaseObject::IsBaseObject(isolate_data_, object));
1262+
CHECK_EQ(object->InternalFieldCount(),
1263+
loader::ModuleWrap::kInternalFieldCount);
1264+
auto* wrap = BaseObject::FromJSObject<loader::ModuleWrap>(object);
1265+
return wrap->CheckUnsettledTopLevelAwait();
1266+
}
1267+
12311268
void Environment::RunAndClearInterrupts() {
12321269
while (native_immediates_interrupts_.size() > 0) {
12331270
NativeImmediateQueue queue;

src/env.h

+2
Original file line numberDiff line numberDiff line change
@@ -735,6 +735,7 @@ class Environment : public MemoryRetainer {
735735
// a pseudo-boolean to indicate whether the exit code is undefined.
736736
inline AliasedInt32Array& exit_info();
737737
inline void set_exiting(bool value);
738+
bool exiting() const;
738739
inline ExitCode exit_code(const ExitCode default_code) const;
739740

740741
// This stores whether the --abort-on-uncaught-exception flag was passed
@@ -840,6 +841,7 @@ class Environment : public MemoryRetainer {
840841
void AtExit(void (*cb)(void* arg), void* arg);
841842
void RunAtExitCallbacks();
842843

844+
v8::Maybe<bool> CheckUnsettledTopLevelAwait();
843845
void RunWeakRefCleanup();
844846

845847
v8::MaybeLocal<v8::Value> RunSnapshotSerializeCallback() const;

src/env_properties.h

+2
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424
V(transfer_mode_private_symbol, "node:transfer_mode") \
2525
V(host_defined_option_symbol, "node:host_defined_option_symbol") \
2626
V(js_transferable_wrapper_private_symbol, "node:js_transferable_wrapper") \
27+
V(entry_point_module_private_symbol, "node:entry_point_module") \
28+
V(entry_point_promise_private_symbol, "node:entry_point_promise") \
2729
V(napi_type_tag, "node:napi:type_tag") \
2830
V(napi_wrapper, "node:napi:wrapper") \
2931
V(untransferable_object_private_symbol, "node:untransferableObject") \

src/module_wrap.cc

+38
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,44 @@ ModuleWrap* ModuleWrap::GetFromModule(Environment* env,
104104
return nullptr;
105105
}
106106

107+
v8::Maybe<bool> ModuleWrap::CheckUnsettledTopLevelAwait() {
108+
Isolate* isolate = env()->isolate();
109+
Local<Context> context = env()->context();
110+
111+
// This must be invoked when the environment is shutting down, and the module
112+
// is kept alive by the module wrap via an internal field.
113+
CHECK(env()->exiting());
114+
CHECK(!module_.IsEmpty());
115+
116+
Local<Module> module = module_.Get(isolate);
117+
// It's a synthetic module, likely a facade wrapping CJS.
118+
if (!module->IsSourceTextModule()) {
119+
return v8::Just(true);
120+
}
121+
122+
if (!module->IsGraphAsync()) { // There is no TLA, no need to check.
123+
return v8::Just(true);
124+
}
125+
auto stalled = module->GetStalledTopLevelAwaitMessage(isolate);
126+
if (stalled.size() == 0) {
127+
return v8::Just(true);
128+
}
129+
130+
if (env()->options()->warnings) {
131+
for (auto pair : stalled) {
132+
Local<v8::Message> message = std::get<1>(pair);
133+
134+
std::string reason = "Warning: Detected unsettled top-level await at ";
135+
std::string info =
136+
FormatErrorMessage(isolate, context, "", message, true);
137+
reason += info;
138+
FPrintF(stderr, "%s\n", reason);
139+
}
140+
}
141+
142+
return v8::Just(false);
143+
}
144+
107145
// new ModuleWrap(url, context, source, lineOffset, columnOffset)
108146
// new ModuleWrap(url, context, exportNames, syntheticExecutionFunction)
109147
void ModuleWrap::New(const FunctionCallbackInfo<Value>& args) {

src/module_wrap.h

+1
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ class ModuleWrap : public BaseObject {
5858
}
5959

6060
v8::Local<v8::Context> context() const;
61+
v8::Maybe<bool> CheckUnsettledTopLevelAwait();
6162

6263
SET_MEMORY_INFO_NAME(ModuleWrap)
6364
SET_SELF_SIZE(ModuleWrap)

src/node_exit_code.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ namespace node {
2727
/* This was intended for invalid inspector arguments but is actually now */ \
2828
/* just a duplicate of InvalidCommandLineArgument */ \
2929
V(InvalidCommandLineArgument2, 12) \
30-
V(UnfinishedTopLevelAwait, 13) \
30+
V(UnsettledTopLevelAwait, 13) \
3131
V(StartupSnapshotFailure, 14) \
3232
/* If the process exits from unhandled signals e.g. SIGABRT, SIGTRAP, */ \
3333
/* typically the exit codes are 128 + signal number. We also exit with */ \

0 commit comments

Comments
 (0)