Skip to content

Commit

Permalink
Disable top-level await in require with a compat flag
Browse files Browse the repository at this point in the history
  • Loading branch information
jasnell committed Nov 9, 2024
1 parent df6d46c commit 51d5c9a
Show file tree
Hide file tree
Showing 11 changed files with 112 additions and 49 deletions.
26 changes: 15 additions & 11 deletions src/workerd/api/node/module.c++
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// https://opensource.org/licenses/Apache-2.0
#include "module.h"

#include <workerd/io/features.h>
#include <workerd/jsg/url.h>

namespace workerd::api::node {
Expand Down Expand Up @@ -84,19 +85,22 @@ jsg::JsValue ModuleUtil::createRequire(jsg::Lock& js, kj::String path) {

auto module = info.module.getHandle(js);

jsg::instantiateModule(js, module);
auto handle = jsg::check(module->Evaluate(js.v8Context()));
KJ_ASSERT(handle->IsPromise());
auto prom = handle.As<v8::Promise>();
if (prom->State() == v8::Promise::PromiseState::kPending) {
js.runMicrotasks();
}
JSG_REQUIRE(prom->State() != v8::Promise::PromiseState::kPending, Error,
"Module evaluation did not complete synchronously.");
if (module->GetStatus() == v8::Module::kErrored) {
jsg::throwTunneledException(js.v8Isolate, module->GetException());
auto features = FeatureFlags::get(js);
jsg::InstantiateModuleOptions options = jsg::InstantiateModuleOptions::DEFAULT;
if (features.getNoTopLevelAwaitInRequire()) {
options = jsg::InstantiateModuleOptions::NO_TOP_LEVEL_AWAIT;

// If the module was already evaluated, let's check if it is async.
// If it is, we will throw an error. This case can happen if a previous
// attempt to require the module failed because the module was async.
if (module->GetStatus() == v8::Module::kEvaluated) {
JSG_REQUIRE(!module->IsGraphAsync(), Error,
"Top-level await in module is not permitted at this time.");
}
}

jsg::instantiateModule(js, module, options);

if (isEsm) {
// If the import is an esm module, we will return the namespace object.
jsg::JsObject obj(module->GetModuleNamespace().As<v8::Object>());
Expand Down
9 changes: 8 additions & 1 deletion src/workerd/api/node/tests/module-create-require-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,14 @@ export const doTheTest = {
strictEqual(assert, required);

throws(() => require('invalid'), {
message: 'Module evaluation did not complete synchronously.',
message: 'Top-level await in module is not permitted at this time.',
});
// Trying to require the module again should throw the same error.
throws(() => require('invalid'), {
message: 'Top-level await in module is not permitted at this time.',
});
throws(() => require('invalid2'), {
message: 'Top-level await in module is not permitted at this time.',
});

throws(() => require('does not exist'));
Expand Down
6 changes: 4 additions & 2 deletions src/workerd/api/node/tests/module-create-require-test.wd-test
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,12 @@ const unitTests :Workerd.Config = (
(name = "bar", esModule = "export default 2; export const __cjsUnwrapDefault = true;"),
(name = "baz", commonJsModule = "module.exports = 3;"),
(name = "worker/qux", text = "4"),
(name = "invalid", esModule = "const p = new Promise(() => {}); await p;"),
(name = "invalid", esModule = "await new Promise(() => {});"),
(name = "invalid2", commonJsModule = "require('invalid3');"),
(name = "invalid3", esModule = "await new Promise(() => {});"),
],
compatibilityDate = "2024-08-01",
compatibilityFlags = ["nodejs_compat_v2"]
compatibilityFlags = ["disable_top_level_await_in_require", "nodejs_compat_v2"]
)
),
],
Expand Down
2 changes: 1 addition & 1 deletion src/workerd/api/tests/new-module-registry-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
import { foo, default as def } from 'foo';
import { default as fs } from 'node:fs';
import { Buffer } from 'buffer';
const { foo: foo2, default: def2 } = await import('bar');
import { foo as foo2, default as def2 } from 'bar';

// Verify that import.meta.url is correct here.
strictEqual(import.meta.url, 'file:///worker');
Expand Down
8 changes: 8 additions & 0 deletions src/workerd/io/compatibility-date.capnp
Original file line number Diff line number Diff line change
Expand Up @@ -658,4 +658,12 @@ struct CompatibilityFlags @0x8f8c1b68151b6cef {
#
# This is a compat flag so that we can opt in our test workers into it before rolling out to
# everyone.

noTopLevelAwaitInRequire @67 :Bool
$compatEnableFlag("disable_top_level_await_in_require")
$compatDisableFlag("enable_top_level_await_in_require")
$compatEnableDate("2024-12-02");
# When enabled, use of top-level await syntax in require() calls will be disallowed.
# The ecosystem and runtimes are moving to a state where top level await in modules
# is being strongly discouraged.
}
3 changes: 3 additions & 0 deletions src/workerd/io/worker.c++
Original file line number Diff line number Diff line change
Expand Up @@ -999,6 +999,9 @@ Worker::Isolate::Isolate(kj::Own<Api> apiParam,
if (features.getNodeJsCompatV2()) {
lock->setNodeJsCompatEnabled();
}
if (!features.getNoTopLevelAwaitInRequire()) {
lock->enableTopLevelAwait();
}

if (impl->inspector != kj::none || ::kj::_::Debug::shouldLog(::kj::LogSeverity::INFO)) {
lock->setLoggerCallback([this](jsg::Lock& js, kj::StringPtr message) {
Expand Down
4 changes: 4 additions & 0 deletions src/workerd/jsg/jsg.c++
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,10 @@ void Lock::setNodeJsCompatEnabled() {
IsolateBase::from(v8Isolate).setNodeJsCompatEnabled({}, true);
}

void Lock::enableTopLevelAwait() {
IsolateBase::from(v8Isolate).enableTopLevelAwait();
}

void Lock::setToStringTag() {
IsolateBase::from(v8Isolate).enableSetToStringTag();
}
Expand Down
1 change: 1 addition & 0 deletions src/workerd/jsg/jsg.h
Original file line number Diff line number Diff line change
Expand Up @@ -2516,6 +2516,7 @@ class Lock {

void setNodeJsCompatEnabled();
void setToStringTag();
void enableTopLevelAwait();

using Logger = void(Lock&, kj::StringPtr);
void setLoggerCallback(kj::Function<Logger>&& logger);
Expand Down
84 changes: 51 additions & 33 deletions src/workerd/jsg/modules.c++
Original file line number Diff line number Diff line change
Expand Up @@ -272,24 +272,24 @@ v8::Local<v8::Value> CommonJsModuleContext::require(jsg::Lock& js, kj::String sp
// Adding imported from suffix here not necessary like it is for resolveCallback, since we have a
// js stack that will include the parent module's name and location of the failed require().

JSG_REQUIRE_NONNULL(
info.maybeSynthetic, TypeError, "Cannot use require() to import an ES Module.");

auto module = info.module.getHandle(js);

check(module->InstantiateModule(js.v8Context(), &resolveCallback));
auto handle = check(module->Evaluate(js.v8Context()));
KJ_ASSERT(handle->IsPromise());
auto prom = handle.As<v8::Promise>();

// This assert should always pass since evaluateSyntheticModuleCallback() for CommonJS
// modules (below) always returns an already-resolved promise.
KJ_ASSERT(prom->State() != v8::Promise::PromiseState::kPending);

if (module->GetStatus() == v8::Module::kErrored) {
throwTunneledException(js.v8Isolate, module->GetException());
auto& isolateBase = IsolateBase::from(js.v8Isolate);
jsg::InstantiateModuleOptions options = jsg::InstantiateModuleOptions::DEFAULT;
if (!isolateBase.isTopLevelAwaitEnabled()) {
options = jsg::InstantiateModuleOptions::NO_TOP_LEVEL_AWAIT;

// If the module was already evaluated, let's check if it is async.
// If it is, we will throw an error. This case can happen if a previous
// attempt to require the module failed because the module was async.
if (module->GetStatus() == v8::Module::kEvaluated) {
JSG_REQUIRE(!module->IsGraphAsync(), Error,
"Top-level await in module is not permitted at this time.");
}
}

jsg::instantiateModule(js, module, options);

// Originally, This returned an object like `{default: module.exports}` when we really
// intended to return the module exports raw. We should be extracting `default` here.
// Unfortunately, there is a user depending on the wrong behavior in production, so we
Expand Down Expand Up @@ -322,15 +322,22 @@ NonModuleScript NonModuleScript::compile(kj::StringPtr code, jsg::Lock& js, kj::
return NonModuleScript(js, check(v8::ScriptCompiler::CompileUnboundScript(isolate, &source)));
}

void instantiateModule(jsg::Lock& js, v8::Local<v8::Module>& module) {
void instantiateModule(
jsg::Lock& js, v8::Local<v8::Module>& module, InstantiateModuleOptions options) {
KJ_ASSERT(!module.IsEmpty());
auto isolate = js.v8Isolate;
auto context = js.v8Context();

auto status = module->GetStatus();
// Nothing to do if the module is already instantiated, evaluated, or errored.
if (status == v8::Module::Status::kInstantiated || status == v8::Module::Status::kEvaluated ||
status == v8::Module::Status::kErrored)

// If the previous instantiation failed, throw the exception.
if (status == v8::Module::Status::kErrored) {
isolate->ThrowException(module->GetException());
throw jsg::JsExceptionThrown();
}

// Nothing to do if the module is already instantiated, evaluated.
if (status == v8::Module::Status::kInstantiated || status == v8::Module::Status::kEvaluated)
return;

JSG_REQUIRE(status == v8::Module::Status::kUninstantiated, Error,
Expand All @@ -341,13 +348,22 @@ void instantiateModule(jsg::Lock& js, v8::Local<v8::Module>& module) {

jsg::check(module->InstantiateModule(context, &resolveCallback));
auto prom = jsg::check(module->Evaluate(context)).As<v8::Promise>();
js.runMicrotasks();

if (module->IsGraphAsync() && prom->State() == v8::Promise::kPending) {
// Pump the microtasks if there's an unsettled top-level await in the module.
// Because we do not support i/o in this scope, this *should* resolve in a
// single drain of the microtask queue (tho it's possible that it'll take
// multiple tasks). When the runMicrotasks() is complete, the promise should
// be settled.
JSG_REQUIRE(options != InstantiateModuleOptions::NO_TOP_LEVEL_AWAIT, Error,
"Top-level await in module is not permitted at this time.");
js.runMicrotasks();
}

switch (prom->State()) {
case v8::Promise::kPending:
// Let's make sure nobody is depending on pending modules that do not resolve first.
KJ_LOG(WARNING, "Async module was not immediately resolved.");
break;
JSG_FAIL_REQUIRE(Error, "Top-level await in module is unsettled.");
case v8::Promise::kRejected:
// Since we don't actually support I/O when instantiating a worker, we don't return the
// promise from module->Evaluate, which means we lose any errors that happen during
Expand Down Expand Up @@ -594,20 +610,22 @@ v8::Local<v8::Value> NodeJsModuleContext::require(jsg::Lock& js, kj::String spec

auto module = info.module.getHandle(js);

jsg::instantiateModule(js, module);

auto handle = jsg::check(module->Evaluate(js.v8Context()));
KJ_ASSERT(handle->IsPromise());
auto prom = handle.As<v8::Promise>();

// This assert should always pass since evaluateSyntheticModuleCallback() for CommonJS
// modules (below) always returns an already-resolved promise.
KJ_ASSERT(prom->State() != v8::Promise::PromiseState::kPending);

if (module->GetStatus() == v8::Module::kErrored) {
jsg::throwTunneledException(js.v8Isolate, module->GetException());
auto& isolateBase = IsolateBase::from(js.v8Isolate);
jsg::InstantiateModuleOptions options = jsg::InstantiateModuleOptions::DEFAULT;
if (!isolateBase.isTopLevelAwaitEnabled()) {
options = jsg::InstantiateModuleOptions::NO_TOP_LEVEL_AWAIT;

// If the module was already evaluated, let's check if it is async.
// If it is, we will throw an error. This case can happen if a previous
// attempt to require the module failed because the module was async.
if (module->GetStatus() == v8::Module::kEvaluated) {
JSG_REQUIRE(!module->IsGraphAsync(), Error,
"Top-level await in module is not permitted at this time.");
}
}

jsg::instantiateModule(js, module, options);

return js.v8Get(module->GetModuleNamespace().As<v8::Object>(), "default"_kj);
}

Expand Down
9 changes: 8 additions & 1 deletion src/workerd/jsg/modules.h
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,14 @@ class NonModuleScript {
v8::Global<v8::UnboundScript> unboundScript;
};

void instantiateModule(jsg::Lock& js, v8::Local<v8::Module>& module);
enum class InstantiateModuleOptions {
DEFAULT,
NO_TOP_LEVEL_AWAIT,
};

void instantiateModule(jsg::Lock& js,
v8::Local<v8::Module>& module,
InstantiateModuleOptions options = InstantiateModuleOptions::DEFAULT);

enum class ModuleInfoCompileOption {
// The BUNDLE options tells the compile operation to treat the content as coming
Expand Down
9 changes: 9 additions & 0 deletions src/workerd/jsg/setup.h
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,14 @@ class IsolateBase {
setToStringTag = true;
}

inline void enableTopLevelAwait() {
allowTopLevelAwait = true;
}

inline bool isTopLevelAwaitEnabled() const {
return allowTopLevelAwait;
}

// The logger will be optionally set by the isolate setup logic if there is anywhere
// for the log to go (for instance, if debug logging is enabled or the inspector is
// being used).
Expand Down Expand Up @@ -238,6 +246,7 @@ class IsolateBase {
bool asyncContextTrackingEnabled = false;
bool nodeJsCompatEnabled = false;
bool setToStringTag = false;
bool allowTopLevelAwait = false;

kj::Maybe<kj::Function<Logger>> maybeLogger;
kj::Maybe<kj::Function<ErrorReporter>> maybeErrorReporter;
Expand Down

0 comments on commit 51d5c9a

Please sign in to comment.