Fix exception handling in the console.log / Bun.inspect property walk, napi_get_all_property_names and util.isError

src/jsc/bindings/bindings.cpp

@@ -5643,10 +5643,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 initializers.
                 CLEAR_IF_EXCEPTION(scope);
+                if (!hasProperty)
+                    continue;
 
                 if ((slot.attributes() & PropertyAttribute::DontEnum) != 0) {
                     if (property == propertyNames->underscoreProto
@@ -5718,7 +5719,11 @@ static void JSC__JSValue__forEachPropertyImpl(JSC::EncodedJSValue JSValue0, JSC:
                 break;
             if (iterating == globalObject)
                 break;
-            iterating = iterating->getPrototype(globalObject).getObject();
+            JSValue prototype = iterating->getPrototype(globalObject);
+            // Empty when a Proxy "getPrototypeOf" trap threw; the exception is cleared below.
+            if (!prototype) [[unlikely]]
+                break;
+            iterating = prototype.getObject();
         }
     }
 

src/jsc/bindings/napi.cpp

@@ -2077,14 +2077,18 @@ extern "C" napi_status napi_get_all_property_names(
             if (key_mode == napi_key_include_prototypes) {
                 // Climb up the prototype chain to find inherited properties
                 while (!owner->getOwnPropertyDescriptor(globalObject, propKey, desc)) {
-                    JSObject* proto = owner->getPrototype(globalObject).getObject();
+                    NAPI_RETURN_IF_EXCEPTION(env);
+                    JSValue prototype = owner->getPrototype(globalObject);
+                    NAPI_RETURN_IF_EXCEPTION(env);
+                    JSObject* proto = prototype.getObject();
                     if (!proto) {
                         break;
                     }
                     owner = proto;
                 }
             } else {
                 owner->getOwnPropertyDescriptor(globalObject, propKey, desc);
+                NAPI_RETURN_IF_EXCEPTION(env);
             }
 
             // V8 never applies ONLY_WRITABLE/ONLY_CONFIGURABLE to Proxy keys

src/jsc/modules/NodeUtilTypesModule.cpp

@@ -901,6 +901,8 @@ JSC_DEFINE_HOST_FUNCTION(jsFunctionIsError,
         PropertySlot slot(object, PropertySlot::InternalMethodType::VMInquiry, &vm);
         bool has = object->getPropertySlot(globalObject, vm.propertyNames->toStringTagSymbol, slot);
         scope.assertNoException();
+        // A VMInquiry slot forbids entering the VM while it is alive, and getPrototype() below may run a Proxy trap.
+        slot.disallowVMEntry.reset();
         if (has) {
             if (slot.isValue()) {
                 JSValue value = slot.getValue(globalObject, vm.propertyNames->toStringTagSymbol);
@@ -914,6 +916,7 @@ JSC_DEFINE_HOST_FUNCTION(jsFunctionIsError,
         }
 
         JSValue proto = object->getPrototype(globalObject);
+        RETURN_IF_EXCEPTION(scope, {});
         if (proto.isCell() && (proto.inherits<JSC::ErrorInstance>() || proto.asCell()->type() == ErrorInstanceType || proto.inherits<JSC::ErrorPrototype>()))
             return JSValue::encode(jsBoolean(true));
     }

test/js/bun/util/inspect.test.js

@@ -928,3 +928,62 @@ describe.skipIf(!isASAN)("object mutated while being formatted", () => {
     expect(exitCode).toBe(0);
   });
 });
+
+describe.concurrent("exceptions thrown while walking properties", () => {
+  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("a lazy property initializer that throws only hides that property", async () => {
+    // Bun.$ is initialized on first access and reads process.env while doing so.
+    // The exception it throws here used to stay pending while the following
+    // properties were visited, so they were dropped from the output (and debug
+    // builds asserted when the next initializer ran).
+    const { stdout, stderr, exitCode } = await run(`
+      // Formatting a value with a custom inspect function (process.env on Windows)
+      // loads node:util's inspect helpers, which read process.env. Load them
+      // before breaking process.env so only the Bun.$ initializer throws.
+      Bun.inspect({ [Bun.inspect.custom]() { return ""; } });
+      Object.defineProperty(process, "env", { get() { throw new Error("boom"); } });
+      const s = Bun.inspect(Bun);
+      console.log(/^  \\$:/m.test(s), /^  Archive:/m.test(s), /^  version:/m.test(s));
+    `);
+    expect({ stdout, stderr, exitCode }).toEqual({ stdout: "false true true\n", stderr: "", exitCode: 0 });
+  });
+
+  it("a get trap that throws for one inherited property only hides that property", async () => {
+    // Same leak through a Proxy: the pending exception made the remaining
+    // properties look missing and then the prototype step dereferenced the
+    // empty value returned by the proxy's getPrototype.
+    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("trap");
+          return Reflect.get(target, key, receiver);
+        },
+      });
+      console.log(Bun.inspect(Object.create(proto)));
+    `);
+    expect({ stdout, stderr, exitCode }).toEqual({ stdout: "{\n  b: 2,\n  c: 3,\n}\n", stderr: "", exitCode: 0 });
+  });
+
+  it("a throwing getPrototypeOf trap in the prototype chain stops the walk", async () => {
+    const { stdout, stderr, exitCode } = await run(`
+      const proto = new Proxy({ inherited: 1 }, { getPrototypeOf() { throw new Error("trap"); } });
+      console.log(Bun.inspect(Object.create(proto)));
+      console.log(Object.create(proto));
+    `);
+    expect({ stdout, stderr, exitCode }).toEqual({
+      stdout: "{\n  inherited: 1,\n}\n{\n  inherited: 1,\n}\n",
+      stderr: "",
+      exitCode: 0,
+    });
+  });
+});

test/js/node/util/util.test.js

@@ -23,7 +23,7 @@
 
 import assert from "assert";
 import { describe, expect, it } from "bun:test";
-import "harness";
+import { bunEnv, bunExe } from "harness";
 import util from "util";
 // const context = require('vm').runInNewContext; // TODO: Use a vm polyfill
 
@@ -152,6 +152,27 @@ describe("util", () => {
       let err8 = new Error3();
       strictEqual(util.isError(err8), true);
     });
+
+    it("propagates an exception from a getPrototypeOf trap", async () => {
+      // Used to crash the process, so the call runs in a child.
+      await using proc = Bun.spawn({
+        cmd: [
+          bunExe(),
+          "-e",
+          `const proxy = new Proxy({}, { getPrototypeOf() { throw new Error("getPrototypeOf trap"); } });
+           try {
+             console.log("returned", require("util").isError(proxy));
+           } catch (e) {
+             console.log("threw", e.message);
+           }`,
+        ],
+        env: bunEnv,
+        stdout: "pipe",
+        stderr: "pipe",
+      });
+      const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+      expect({ stdout, stderr, exitCode }).toEqual({ stdout: "threw getPrototypeOf trap\n", stderr: "", exitCode: 0 });
+    });
   });
 
   describe("isObject", () => {

test/napi/napi-app/module.js

@@ -321,6 +321,40 @@ nativeTests.test_get_all_property_names_proxy_and_string_wrapper = () => {
   show("frozen writable:", apn(Object.freeze({ a: 1, b: 2 }), napi_key_writable));
 };
 
+nativeTests.test_get_all_property_names_throwing_trap = () => {
+  const napi_key_include_prototypes = 0;
+  const napi_key_own_only = 1;
+  const napi_key_enumerable = 1 << 1;
+  const napi_key_keep_numbers = 0;
+
+  // The enumerable filter makes both engines ask the proxy for descriptors.
+  const makeProxy = () =>
+    new Proxy(
+      { k: 1 },
+      {
+        getOwnPropertyDescriptor() {
+          throw new Error("descriptor trap");
+        },
+      },
+    );
+  for (const [label, obj, mode] of [
+    ["own_only", makeProxy(), napi_key_own_only],
+    ["include_prototypes", Object.create(makeProxy()), napi_key_include_prototypes],
+  ]) {
+    try {
+      const { status, keys } = nativeTests.get_all_property_names(
+        obj,
+        mode,
+        napi_key_enumerable,
+        napi_key_keep_numbers,
+      );
+      console.log(`${label}: returned status=${status} keys=${JSON.stringify(keys)}`);
+    } catch (e) {
+      console.log(`${label}: threw ${e.message}`);
+    }
+  }
+};
+
 nativeTests.test_set_property = () => {
   const objects = [
     {},

test/napi/napi.test.ts

@@ -880,6 +880,11 @@ describe.concurrent.skipIf(!canBuildNodeAddons())("napi", () => {
       expect(output).toContain(`plain writable: status=0 keys=["w","nc"]`);
       expect(output).toContain(`frozen writable: status=0 keys=[]`);
     });
+    it("propagates an exception thrown by a Proxy trap while filtering", async () => {
+      const output = await checkSameOutput("test_get_all_property_names_throwing_trap", []);
+      expect(output).toContain("own_only: threw descriptor trap");
+      expect(output).toContain("include_prototypes: threw descriptor trap");
+    });
   });
 
   describe("napi_value <=> integer conversion", () => {