Skip to content

Fix pending exception leak when a lazy property throws during inspect #37292

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
robobun wants to merge 7 commits into from
Closed

Fix pending exception leak when a lazy property throws during inspect #37292

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
01f18bf
Clear pending exception when a property slot lookup throws during ins…
robobun Aug 9, 2026
0d03890
Drain child stderr in the inspect regression test
robobun Aug 9, 2026
6ae6ac0
Install a fallback when the util.inspect lazy property initializer th…
robobun Aug 9, 2026
4349414
Shorten fallback comments
robobun Aug 9, 2026
5c28744
Use the colorless stylizer when util.inspect has no styles table
robobun Aug 9, 2026
a81eb32
Shorten stylize fallback comment
robobun Aug 9, 2026
c4ad31e
Assert exact stdout in the util.inspect fallback tests
robobun Aug 9, 2026
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
66 changes: 44 additions & 22 deletions src/jsc/bindings/ZigGlobalObject.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1929,6 +1929,12 @@ extern "C" napi_env ZigGlobalObject__makeNapiEnvForFFI(Zig::GlobalObject* global
return globalObject->makeNapiEnvForFFI();
}

// Stub installed by m_utilInspectFunction's initializer when node:util fails to load.
JSC_DEFINE_HOST_FUNCTION(jsFunctionUtilInspectFallback, (JSGlobalObject * globalObject, CallFrame*))
{
return JSValue::encode(jsEmptyString(globalObject->vm()));
}

JSC_DEFINE_HOST_FUNCTION(jsFunctionPerformMicrotaskVariadic, (JSGlobalObject * globalObject, CallFrame* callframe))
{
auto& vm = JSC::getVM(globalObject);
Expand Down Expand Up @@ -2307,12 +2313,19 @@ void GlobalObject::finishCreation(VM& vm)
[](const Initializer<JSFunction>& init) {
auto scope = DECLARE_THROW_SCOPE(init.vm);
JSValue nodeUtilValue = uncheckedDowncast<Zig::GlobalObject>(init.owner)->internalModuleRegistry()->requireId(init.owner, init.vm, Bun::InternalModuleRegistry::Field::NodeUtil);
RETURN_IF_EXCEPTION(scope, );
RELEASE_ASSERT(nodeUtilValue.isObject());
auto prop = nodeUtilValue.getObject()->getIfPropertyExists(init.owner, Identifier::fromString(init.vm, "inspect"_s));
RETURN_IF_EXCEPTION(scope, );
ASSERT(prop);
init.set(uncheckedDowncast<JSFunction>(prop));
if (!scope.exception()) [[likely]] {
RELEASE_ASSERT(nodeUtilValue.isObject());
auto prop = nodeUtilValue.getObject()->getIfPropertyExists(init.owner, Identifier::fromString(init.vm, "inspect"_s));
if (!scope.exception()) [[likely]] {
if (auto* inspect = dynamicDowncast<JSFunction>(prop)) [[likely]] {
init.set(inspect);
return;
}
}
}
// Requiring node:util can throw; a LazyProperty initializer must always init.set().
(void)scope.tryClearException();
init.set(JSFunction::create(init.vm, init.owner, 2, "inspect"_s, jsFunctionUtilInspectFallback, ImplementationVisibility::Public));
});

m_utilInspectOptionsStructure.initLater(
Expand All @@ -2332,23 +2345,32 @@ void GlobalObject::finishCreation(VM& vm)
m_utilInspectStylizeColorFunction.initLater(
[](const Initializer<JSFunction>& init) {
auto scope = DECLARE_THROW_SCOPE(init.vm);
JSC::MarkedArgumentBuffer args;
args.append(uncheckedDowncast<Zig::GlobalObject>(init.owner)->utilInspectFunction());
RETURN_IF_EXCEPTION(scope, );

JSC::JSFunction* getStylize = JSC::JSFunction::create(init.vm, init.owner, utilInspectGetStylizeWithColorCodeGenerator(init.vm), init.owner);
RETURN_IF_EXCEPTION(scope, );

JSC::CallData callData = JSC::getCallData(getStylize);
NakedPtr<JSC::Exception> returnedException = nullptr;
auto result = JSC::profiledCall(init.owner, ProfilingReason::API, getStylize, callData, jsNull(), args, returnedException);
RETURN_IF_EXCEPTION(scope, );

if (returnedException) {
throwException(init.owner, scope, returnedException.get());
JSC::JSFunction* inspect = uncheckedDowncast<Zig::GlobalObject>(init.owner)->utilInspectFunction();

if (!scope.exception()) [[likely]] {
// stylizeWithColor reads inspect.styles at call time; the fallback stub has none.
JSValue styles = inspect->getIfPropertyExists(init.owner, Identifier::fromString(init.vm, "styles"_s));
if (!scope.exception() && styles && styles.isObject()) [[likely]] {
JSC::MarkedArgumentBuffer args;
args.append(inspect);
JSC::JSFunction* getStylize = JSC::JSFunction::create(init.vm, init.owner, utilInspectGetStylizeWithColorCodeGenerator(init.vm), init.owner);

JSC::CallData callData = JSC::getCallData(getStylize);
NakedPtr<JSC::Exception> returnedException = nullptr;
auto result = JSC::profiledCall(init.owner, ProfilingReason::API, getStylize, callData, jsNull(), args, returnedException);
if (returnedException) [[unlikely]]
throwException(init.owner, scope, returnedException.get());
if (!scope.exception()) [[likely]] {
if (auto* stylize = dynamicDowncast<JSFunction>(result)) [[likely]] {
init.set(stylize);
return;
}
}
}
}
RETURN_IF_EXCEPTION(scope, );
init.set(uncheckedDowncast<JSFunction>(result));
// The initializer must still init.set() when util.inspect is unavailable.
(void)scope.tryClearException();
init.set(JSC::JSFunction::create(init.vm, init.owner, utilInspectStylizeWithNoColorCodeGenerator(init.vm), init.owner));
});

m_utilInspectStylizeNoColorFunction.initLater(
Expand Down
7 changes: 4 additions & 3 deletions src/jsc/bindings/bindings.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5596,10 +5596,11 @@ 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 hasProperty = object->getPropertySlot(globalObject, property, slot);
// Ignore exceptions from "Get" proxy traps and lazy property builders.
CLEAR_IF_EXCEPTION(scope);
if (!hasProperty)
continue;

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

// The Bun object's lazy properties (like Bun.$) are built on first access. With
// `process` clobbered, building Bun.$ throws, and the property walk used by
// console.log must clear that pending exception instead of leaving it set,
// which aborted debug builds at the next exception-scope assertion.
it("console.log survives a lazy property builder throwing mid-walk", async () => {
const code = `
globalThis.process = undefined;
console.log(Bun);
console.log("SURVIVED");
Comment on lines +936 to +940

@coderabbitai coderabbitai Bot Aug 9, 2026

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.

🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Add coverage for the throwing proxy-trap path.

The C++ change handles both failed lazy property builders and throwing proxy traps. This test covers only the lazy Bun property path. Add a proxy get trap that throws and verify that execution still reaches SURVIVED with exit code 0.

As per coding guidelines, tests must cover the complete relevant variant matrix, including distinct error paths. The PR objective also identifies throwing proxy traps as a handled path.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/js/bun/util/inspect.test.js` around lines 936 - 940, Add coverage in the
test named “console.log survives a lazy property builder throwing mid-walk” for
a separate object whose proxy get trap throws, then log it and assert execution
still prints “SURVIVED” and exits with code 0. Preserve the existing
lazy-property case while exercising the distinct throwing-proxy path.

Source: Coding guidelines

`;
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).toContain("SURVIVED");
expect(exitCode).toBe(0);
});

// util.inspect is loaded lazily the first time a custom inspect function is
// formatted. With `process` clobbered, requiring node:util throws; the lazy
// property initializer must still install a fallback instead of aborting.
// On Windows, console.log(Bun) also reaches this through Bun.env.
it("console.log survives util.inspect failing to load for custom inspect", async () => {
const code = `
globalThis.process = undefined;
console.log({ [Symbol.for("nodejs.util.inspect.custom")]() { return "hi" } });
console.log("SURVIVED");
`;
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("hi\nSURVIVED\n");
expect(exitCode).toBe(0);
});

// With colors on, options.stylize comes from a closure built around util.inspect.
// When util.inspect failed to load, the colorless stylizer must be used instead.
it("stylize degrades to no color when util.inspect failed to load", async () => {
const code = `
globalThis.process = undefined;
console.log({ [Symbol.for("nodejs.util.inspect.custom")](depth, options) { return options.stylize("hi", "string"); } });
console.log("SURVIVED");
`;
await using proc = Bun.spawn({
cmd: [bunExe(), "-e", code],
env: { ...bunEnv, FORCE_COLOR: "1" },
stdout: "pipe",
stderr: "pipe",
});
const [stdout, , exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stdout).toBe("hi\nSURVIVED\n");
expect(exitCode).toBe(0);
});
Loading