Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions src/jsc/bindings/BunObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -319,9 +319,6 @@ static JSValue defaultBunSQLObject(VM& vm, JSObject* bunObject)
auto scope = DECLARE_THROW_SCOPE(vm);
auto* globalObject = defaultGlobalObject(bunObject->globalObject());
JSValue sqlValue = globalObject->internalModuleRegistry()->requireId(globalObject, vm, InternalModuleRegistry::BunSql);
#if BUN_DEBUG
if (scope.exception()) globalObject->reportUncaughtExceptionAtEventLoop(globalObject, scope.exception());
#endif
RETURN_IF_EXCEPTION(scope, {});
RELEASE_AND_RETURN(scope, sqlValue.getObject()->get(globalObject, vm.propertyNames->defaultKeyword));
}
Expand All @@ -331,9 +328,6 @@ static JSValue constructBunSQLObject(VM& vm, JSObject* bunObject)
auto scope = DECLARE_THROW_SCOPE(vm);
auto* globalObject = defaultGlobalObject(bunObject->globalObject());
JSValue sqlValue = globalObject->internalModuleRegistry()->requireId(globalObject, vm, InternalModuleRegistry::BunSql);
#if BUN_DEBUG
if (scope.exception()) globalObject->reportUncaughtExceptionAtEventLoop(globalObject, scope.exception());
#endif
RETURN_IF_EXCEPTION(scope, {});
auto clientData = WebCore::clientData(vm);
RELEASE_AND_RETURN(scope, sqlValue.getObject()->get(globalObject, clientData->builtinNames().SQLPublicName()));
Expand Down
8 changes: 5 additions & 3 deletions src/jsc/bindings/bindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5596,10 +5596,12 @@ static void JSC__JSValue__forEachPropertyImpl(JSC::EncodedJSValue JSValue0, JSC:
}

JSC::PropertySlot slot(object, PropertySlot::InternalMethodType::Get);
if (!object->getPropertySlot(globalObject, property, slot))
continue;
// Ignore exceptions from "Get" proxy traps.
bool found = object->getPropertySlot(globalObject, property, slot);
// Ignore exceptions from "Get" proxy traps and throwing lazy
// property initializers (which report the slot as not found).
Comment on lines +5600 to +5601

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code

CLEAR_IF_EXCEPTION(scope);
if (!found)
continue;

if ((slot.attributes() & PropertyAttribute::DontEnum) != 0) {
if (property == propertyNames->underscoreProto
Expand Down
51 changes: 51 additions & 0 deletions test/js/bun/util/inspect.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -928,3 +928,54 @@ describe.skipIf(!isASAN)("object mutated while being formatted", () => {
expect(exitCode).toBe(0);
});
});

it("skips properties whose lazy initializer throws instead of leaving the exception pending", async () => {
// Overwriting the global Symbol breaks the lazily-initialized Bun.$ and
// Bun.sql properties (their module-scope code calls Symbol). Inspecting the
// Bun object reifies every property; a throwing initializer has to be
// skipped, not left as a pending exception, which aborts assert-enabled
// builds.
const code = `
Symbol++;
const out = Bun.inspect(Bun);
if (typeof out !== "string" || out.length === 0) throw new Error("empty inspect output");
console.log("ok");
`;
await using proc = Bun.spawn({
cmd: [bunExe(), "-e", code],
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
const [stdout, , exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stdout).toBe("ok\n");
expect(exitCode).toBe(0);
});

it("building an invalid-argument error message survives lazy initializers that throw", async () => {
// Same walk reached through ERR_INVALID_ARG_VALUE rendering the received
// value: new CompressionStream(globalThis) inspects globalThis for its
// error message while the global Symbol is broken. Getters that report
// their failure can set a nonzero exit code, so only assert that execution
// gets past the constructor instead of aborting.
const code = `
Symbol++;
let caught;
try {
new CompressionStream(globalThis);
} catch (e) {
caught = e;
}
if (!caught) throw new Error("expected CompressionStream to throw");
console.log("ok");
`;
await using proc = Bun.spawn({
cmd: [bunExe(), "-e", code],
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
const [stdout] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stdout).toBe("ok\n");
expect(proc.signalCode).toBeNull();
});