Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
14 changes: 10 additions & 4 deletions src/jsc/bindings/bindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5596,10 +5596,11 @@
}

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 initializers.
CLEAR_IF_EXCEPTION(scope);
if (!hasProperty)
continue;

if ((slot.attributes() & PropertyAttribute::DontEnum) != 0) {
if (property == propertyNames->underscoreProto
Expand Down Expand Up @@ -5671,7 +5672,12 @@
break;
if (iterating == globalObject)
break;
iterating = iterating->getPrototype(globalObject).getObject();
JSValue prototype = iterating->getPrototype(globalObject);
// Ignore exceptions from "getPrototypeOf" proxy traps.
CLEAR_IF_EXCEPTION(scope);
if (!prototype) [[unlikely]]
break;
iterating = prototype.getObject();

Check warning on line 5680 in src/jsc/bindings/bindings.cpp

View check run for this annotation

Claude / Claude Code Review

Same-class getPrototype().getObject() crash left unfixed in napi.cpp

The identical `getPrototype(globalObject).getObject()` chain — same SIGSEGV-at-0x5 when a Proxy trap throws — has exactly one other occurrence in `src/`, at `src/jsc/bindings/napi.cpp:2072` inside `napi_get_all_property_names`. Per REVIEW.md's "fix the whole class" rule, consider applying the same split-and-check there (or noting in the PR why it's left for a follow-up).
Comment thread
claude[bot] marked this conversation as resolved.
}
}

Expand Down
74 changes: 74 additions & 0 deletions test/js/bun/util/inspect.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,80 @@ describe("crash testing", () => {
}
});

describe("exceptions thrown while walking properties", () => {
// Each case crashed the process before the walk cleared the exception, so
// they run in a subprocess.
async function run(fixture) {
await using proc = Bun.spawn({
cmd: [bunExe(), "-e", fixture],
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
return { stdout, stderr, exitCode };
}

it.concurrent("a throwing Proxy trap in the prototype chain skips only that property", async () => {
const { stdout, stderr, exitCode } = await run(`
{
const proto = new Proxy(
{ a: 1, b: 2, c: 3 },
{
get(target, key, receiver) {
if (key === "a") throw new Error("boom");
return Reflect.get(target, key, receiver);
},
},
);
console.log(Bun.inspect(Object.create(proto)));
}
{
const proto = new Proxy(
{ a: 1 },
{
getPrototypeOf() {
throw new Error("boom");
},
},
);
console.log(Bun.inspect(Object.create(proto)));
}
`);
expect(stdout).toMatchInlineSnapshot(`
"{
b: 2,
c: 3,
}
{
a: 1,
}
"
`);
expect(stderr).toBe("");
expect(exitCode).toBe(0);
});

it.concurrent("a throwing lazy property initializer skips only that property", async () => {
// The builtin behind Bun.$ calls the global Symbol, so clobbering it makes
// that lazy property throw when it is first reified.
const { stdout, stderr, exitCode } = await run(`
globalThis.Symbol = NaN;
let threw = false;
try {
Bun.$;
} catch {
threw = true;
}
const inspected = Bun.inspect(Bun);
console.log(threw, inspected.includes("Archive:"), inspected.includes("version:"));
`);
expect(stdout).toBe("true true true\n");
expect(stderr).toBe("");
expect(exitCode).toBe(0);
});
});

it("possibly formatted emojis log", () => {
expect(Bun.inspect("✔")).toBe('"✔"');
});
Expand Down