fix(js): better error messages when makeSafe fails

.vscode/launch.json

@@ -505,6 +505,18 @@
       // Don't pause when the GC runs while the debugger is open.
       "postRunCommands": ["command source '${workspaceFolder}/misctools/lldb/lldb_commands'"],
     },
+    {
+      "type": "bun",
+      "request": "launch",
+      "name": "JS: bun test [file]",
+      "runtime": "${workspaceFolder}/build/debug/bun-debug",
+      "runtimeArgs": ["test"],
+      "program": "${file}",
+      "env": {
+        "BUN_DEBUG_QUIET_LOGS": "1",
+        "BUN_GARBAGE_COLLECTOR_LEVEL": "2",
+      },
+    },
     // Windows: bun test [file]
     {
       "type": "cppvsdbg",

src/js/internal-for-testing.ts

@@ -152,3 +152,4 @@ export const bindgen = $zig("bindgen_test.zig", "getBindgenTestFunctions") as {
 
 export const noOpForTesting = $cpp("NoOpForTesting.cpp", "createNoOpForTesting");
 export const Dequeue = require("internal/fifo");
+export const primordials = require("internal/primordials");

src/js/internal/primordials.js

@@ -41,27 +41,37 @@ const copyProps = (src, dest) => {
   });
 };
 
+/**
+ * @type {<T extends Function, U extends T>(unsafe: T, safe: U) => U}
+ */
 const makeSafe = (unsafe, safe) => {
   if (Symbol.iterator in unsafe.prototype) {
     const dummy = new unsafe();
     let next; // We can reuse the same `next` method.
 
-    ArrayPrototypeForEach(Reflect.ownKeys(unsafe.prototype), key => {
-      if (!Reflect.getOwnPropertyDescriptor(safe.prototype, key)) {
-        const desc = Reflect.getOwnPropertyDescriptor(unsafe.prototype, key);
-        if (typeof desc.value === "function" && desc.value.length === 0) {
-          const called = desc.value.$call(dummy) || {};
-          if (Symbol.iterator in (typeof called === "object" ? called : {})) {
-            const createIterator = uncurryThis(desc.value);
-            next ??= uncurryThis(createIterator(dummy).next);
-            const SafeIterator = createSafeIterator(createIterator, next);
-            desc.value = function () {
-              return new SafeIterator(this);
-            };
-          }
+    ArrayPrototypeForEach(Reflect.ownKeys(unsafe.prototype), function makeIterableMethodsSafe(key) {
+      // if (Reflect.hasOwnProperty(safe.prototype, key)) return;
+      if (Reflect.getOwnPropertyDescriptor(safe.prototype, key)) return;
+
+      const desc = Reflect.getOwnPropertyDescriptor(unsafe.prototype, key);
+      if (typeof desc.value === "function" && desc.value.length === 0) {
+        try {
+          var called = desc.value.$call(dummy) || {};
+        } catch (e) {
+          const err = new Error(`${unsafe.name}.prototype.${key} thew an error while creating a safe version. This is likely due to prototype pollution.`);
+          Object.assign(err, { unsafe: unsafe.name, safe: safe.name, key, cause: e });
+          throw err;
+        }
+        if (Symbol.iterator in (typeof called === "object" ? called : {})) {
+          const createIterator = uncurryThis(desc.value);
+          next ??= uncurryThis(createIterator(dummy).next);
+          const SafeIterator = createSafeIterator(createIterator, next);
+          desc.value = function () {
+            return new SafeIterator(this);
+          };
         }
-        Reflect.defineProperty(safe.prototype, key, desc);
       }
+      Reflect.defineProperty(safe.prototype, key, desc);
     });
   } else copyProps(unsafe.prototype, safe.prototype);
   copyProps(unsafe, safe);

test/internal/primordials.test.ts

@@ -0,0 +1,80 @@
+import { describe, it, expect, beforeAll, afterEach, jest } from "bun:test";
+import { primordials } from "bun:internal-for-testing";
+
+describe("makeSafe(unsafe, safe)", () => {
+  const { makeSafe } = primordials;
+
+  describe("when making a SafeMap", () => {
+    let SafeMap: typeof Map;
+
+    beforeAll(() => {
+      SafeMap = makeSafe(
+        Map,
+        class SafeMap extends Map {
+          constructor(x) {
+            super(x);
+          }
+        },
+      );
+    });
+
+    it("has a prototype with the same properties as the original", () => {
+      expect(SafeMap.prototype).toEqual(expect.objectContaining(Map.prototype));
+    });
+
+    it("has a frozen prototype", () => {
+      const desc = Object.getOwnPropertyDescriptor(SafeMap, "prototype");
+      expect(desc).toBeDefined();
+      expect(desc!.writable).toBeFalse();
+    });
+  }); // </when making a SafeMap>
+
+  describe("given a custom unsafe iterable class", () => {
+    class Unsafe implements Iterable<number> {
+      *[Symbol.iterator]() {
+        yield 1;
+        yield 2;
+        yield 3;
+      }
+      public foo() {
+        throw new Error("foo");
+      }
+    }
+
+    it("when a method throws, a prototype pollution message is thrown", () => {
+      expect(() => makeSafe(Unsafe, class Safe extends Unsafe {})).toThrow(
+        "Unsafe.prototype.foo thew an error while creating a safe version. This is likely due to prototype pollution.",
+      );
+    });
+  }); // </given a custom unsafe iterable class>
+
+  describe("given a custom unsafe non-iterable class", () => {
+    let foo = jest.fn(function foo() {
+      throw new Error("foo");
+    });
+
+    class Unsafe implements Iterable<number> {
+      *[Symbol.iterator]() {
+        yield 1;
+        yield 2;
+        yield 3;
+      }
+      public foo = foo;
+    }
+
+    afterEach(() => {
+      foo.mockClear();
+    });
+
+    it("makeSafe() does not throw", () => {
+      expect(() => makeSafe(Unsafe, class Safe extends Unsafe {})).not.toThrow(
+        "Unsafe.prototype.foo thew an error while creating a safe version. This is likely due to prototype pollution.",
+      );
+    });
+
+    it("Unsafe.foo() is never called()", () => {
+      makeSafe(Unsafe, class Safe extends Unsafe {});
+      expect(foo).not.toHaveBeenCalled();
+    });
+  });
+}); // </makeSafe(unsafe, safe)>