inspector: handle BigInt/Symbol in returnByValue instead of asserting

scripts/build/deps/webkit.ts

@@ -3,7 +3,7 @@
  * for local mode. Override via `--webkit-version=<hash>` to test a branch.
  * From https://github.com/oven-sh/WebKit releases.
  */
-export const WEBKIT_VERSION = "34c01d13391e00c06862a3d2c5b7fff350ac87e0";
+export const WEBKIT_VERSION = "autobuild-preview-pr-384-0cd81acb";
 
 /**
  * WebKit (JavaScriptCore) — the JS engine.

test/cli/inspect/inspect.test.ts

@@ -300,49 +300,52 @@ describe("websocket", () => {
   });
 });
 
-describe("http metadata endpoint", () => {
-  let metadataInspectee: Subprocess | undefined;
-
-  async function spawnInspectee(): Promise<URL> {
-    metadataInspectee = spawn({
-      cwd: import.meta.dir,
-      cmd: [bunExe(), "--inspect=127.0.0.1:0", "inspectee.js"],
-      env: bunEnv,
-      stdout: "ignore",
-      stderr: "pipe",
-    });
+async function spawnInspectee(): Promise<{ child: Subprocess; url: URL }> {
+  const child = spawn({
+    cwd: import.meta.dir,
+    cmd: [bunExe(), "--inspect=127.0.0.1:0", "inspectee.js"],
+    env: bunEnv,
+    stdout: "ignore",
+    stderr: "pipe",
+  });
 
-    let url: URL | undefined;
-    let stderr = "";
-    const decoder = new TextDecoder();
-    for await (const chunk of metadataInspectee.stderr as ReadableStream) {
-      stderr += decoder.decode(chunk);
-      for (const line of stderr.split("\n")) {
-        try {
-          url = new URL(line);
-        } catch {}
-        if (url?.protocol.includes("ws")) {
-          break;
-        }
-      }
-      if (stderr.includes("Listening:")) {
+  let url: URL | undefined;
+  let stderr = "";
+  const decoder = new TextDecoder();
+  for await (const chunk of child.stderr as ReadableStream) {
+    stderr += decoder.decode(chunk);
+    for (const line of stderr.split("\n")) {
+      try {
+        url = new URL(line);
+      } catch {}
+      if (url?.protocol.includes("ws")) {
         break;
       }
     }
-
-    if (!url) {
-      process.stderr.write(stderr);
-      throw new Error("Unable to find listening URL");
+    if (stderr.includes("Listening:")) {
+      break;
     }
-    return url;
   }
 
+  if (!url) {
+    process.stderr.write(stderr);
+    child.kill();
+    throw new Error("Unable to find listening URL");
+  }
+  return { child, url };
+}
+
+describe("http metadata endpoint", () => {
+  let metadataInspectee: Subprocess | undefined;
+
   afterEach(() => {
     metadataInspectee?.kill();
   });
 
   test("serves /json/version only for a Host of the bound hostname, localhost, or an IP literal", async () => {
-    const { port } = await spawnInspectee();
+    const { child, url } = await spawnInspectee();
+    metadataInspectee = child;
+    const { port } = url;
     const endpoint = `http://127.0.0.1:${port}/json/version`;
 
     const allowed = await fetch(endpoint);
@@ -365,7 +368,9 @@ describe("http metadata endpoint", () => {
   });
 
   test("serves /json/version only to allowed web origins", async () => {
-    const { port } = await spawnInspectee();
+    const { child, url } = await spawnInspectee();
+    metadataInspectee = child;
+    const { port } = url;
     const endpoint = `http://127.0.0.1:${port}/json/version`;
 
     const loopback = await fetch(endpoint, { headers: { "Origin": "http://127.0.0.1:8080" } });
@@ -377,6 +382,80 @@ describe("http metadata endpoint", () => {
   });
 });
 
+// Runtime.evaluate with returnByValue:true on a value holding a BigInt or
+// Symbol used to hit ASSERT_NOT_REACHED in Inspector::jsToInspectorValue
+// (aborting an assertions build) and return the misleading "Object has too
+// long reference chain" error on release. It should now report the value as
+// unserializable with V8's wording and leave the debuggee running.
+describe("Runtime.evaluate returnByValue with BigInt/Symbol", () => {
+  let child: Subprocess | undefined;
+
+  afterEach(() => {
+    child?.kill();
+    child = undefined;
+  });
+
+  async function evaluateByValue(expression: string) {
+    const spawned = await spawnInspectee();
+    child = spawned.child;
+
+    const ws = new WebSocket(spawned.url);
+    await new Promise<void>((resolve, reject) => {
+      ws.addEventListener("open", () => resolve(), { once: true });
+      ws.addEventListener("error", cause => reject(new Error("WebSocket error", { cause })), { once: true });
+      ws.addEventListener("close", () => reject(new Error("WebSocket closed before open")), { once: true });
+    });
+    const reply = new Promise<any>((resolve, reject) => {
+      ws.addEventListener(
+        "message",
+        ({ data }) => {
+          try {
+            resolve(JSON.parse(data.toString()));
+          } catch (cause) {
+            reject(new Error(`non-JSON inspector reply: ${data}`, { cause }));
+          }
+        },
+        { once: true },
+      );
+      ws.addEventListener("error", cause => reject(new Error("WebSocket error", { cause })), { once: true });
+      ws.addEventListener("close", () => reject(new Error("WebSocket closed before reply")), { once: true });
+    });
+    ws.send(JSON.stringify({ id: 1, method: "Runtime.evaluate", params: { expression, returnByValue: true } }));
+    const result = await reply;
+    ws.close();
+    return result;
+  }
+
+  for (const expression of ["[1n]", "({a: 1n})", 'Symbol("s")', "({b: Symbol()})"]) {
+    test(expression, async () => {
+      const reply = await evaluateByValue(expression);
+      expect(reply).toEqual({
+        id: 1,
+        error: {
+          code: -32000,
+          message: "Object couldn't be returned by value",
+          data: expect.anything(),
+        },
+      });
+      // The debuggee must survive the request (an assertions build used to
+      // SIGABRT here before the reply arrived).
+      expect(child!.exitCode).toBeNull();
+      expect(child!.signalCode).toBeNull();
+    });
+  }
+
+  test("bare 1n", async () => {
+    // A bare top-level BigInt is represented via description without a JSON
+    // value and has always worked; keep it covered so the unserializable path
+    // above does not regress it.
+    const reply = await evaluateByValue("1n");
+    expect(reply).toEqual({
+      id: 1,
+      result: { result: { type: "bigint", description: "1n" }, wasThrown: false },
+    });
+  });
+});
+
 describe("unix domain socket without websocket", () => {
   let tempdir: string;
   let randomSocketPath: () => string;