Add SandboxTimeoutError metadata to error message instead of extra pr…

…operties (#9262) * add SandboxTimeoutError metadata to error message instead of extra properties * fix unit tests
pixiebrix · Oct 10, 2024 · 786e594 · 786e594
1 parent 75ca114
commit 786e594
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 80 deletions.
diff --git a/src/sandbox/postMessage.test.ts b/src/sandbox/postMessage.test.ts
@@ -24,7 +24,6 @@ import postMessage, {
   addPostMessageListener,
   pendingMessageMetadataMap,
   type RequestPacket,
-  SandboxTimeoutError,
 } from "./postMessage";
 import { sleep } from "@/utils/timeUtils";
 
@@ -175,18 +174,14 @@ describe("SandboxTimeoutError", () => {
 
     jest.runAllTimers();
 
-    await expect(promise).rejects.toThrow(SandboxTimeoutError);
-    await expect(promise).rejects.toMatchObject({
-      name: "SandboxTimeoutError",
-      message:
-        "Message SANDBOX_PING did not receive a response within 5 seconds",
-      sandboxMessage: {
-        type: "SANDBOX_PING",
-        payloadSize: expect.any(Number),
-        timestamp: expect.any(Number),
-      },
-      pendingSandboxMessages: [],
-    });
+    await expect(promise).rejects.toThrow(
+      expect.objectContaining({
+        name: "SandboxTimeoutError",
+        message: expect.stringMatching(
+          /Sandbox message timed out: type=SANDBOX_PING, sent=\d+, payloadSize=\d+B, pendingMessageCount=0/,
+        ),
+      }),
+    );
   });
 
   test("throws SandboxTimeoutError with correct information for multiple messages", async () => {
@@ -216,62 +211,34 @@ describe("SandboxTimeoutError", () => {
       recipient: unresponsiveChannel as Window,
     });
 
-    await expect(promise1).rejects.toThrow(SandboxTimeoutError);
-    await expect(promise1).rejects.toMatchObject({
-      name: "SandboxTimeoutError",
-      message:
-        "Message SANDBOX_FOO did not receive a response within 5 seconds",
-      sandboxMessage: {
-        type: "SANDBOX_FOO",
-        payloadSize: expect.any(Number),
-        timestamp: expect.any(Number),
-      },
-      pendingSandboxMessages: expect.arrayContaining([
-        {
-          type: "SANDBOX_BAR",
-          payloadSize: expect.any(Number),
-          timestamp: expect.any(Number),
-        },
-        {
-          type: "SANDBOX_BAZ",
-          payloadSize: expect.any(Number),
-          timestamp: expect.any(Number),
-        },
-      ]),
-    });
+    await expect(promise1).rejects.toThrow(
+      expect.objectContaining({
+        name: "SandboxTimeoutError",
+        message: expect.stringMatching(
+          /Sandbox message timed out: type=SANDBOX_FOO, sent=\d+, payloadSize=\d+B, pendingMessageCount=2/,
+        ),
+      }),
+    );
 
-    await expect(promise2).rejects.toThrow(SandboxTimeoutError);
-    await expect(promise2).rejects.toMatchObject({
-      name: "SandboxTimeoutError",
-      message:
-        "Message SANDBOX_BAR did not receive a response within 5 seconds",
-      sandboxMessage: {
-        type: "SANDBOX_BAR",
-        payloadSize: expect.any(Number),
-        timestamp: expect.any(Number),
-      },
-      pendingSandboxMessages: expect.arrayContaining([
-        {
-          type: "SANDBOX_BAZ",
-          payloadSize: expect.any(Number),
-          timestamp: expect.any(Number),
-        },
-      ]),
-    });
+    await expect(promise2).rejects.toThrow(
+      expect.objectContaining({
+        name: "SandboxTimeoutError",
+        message: expect.stringMatching(
+          /Sandbox message timed out: type=SANDBOX_BAR, sent=\d+, payloadSize=\d+B, pendingMessageCount=1/,
+        ),
+      }),
+    );
 
     jest.runAllTimers();
-    await expect(promise3).rejects.toThrow(SandboxTimeoutError);
-    await expect(promise3).rejects.toMatchObject({
-      name: "SandboxTimeoutError",
-      message:
-        "Message SANDBOX_BAZ did not receive a response within 5 seconds",
-      sandboxMessage: {
-        type: "SANDBOX_BAZ",
-        payloadSize: expect.any(Number),
-        timestamp: expect.any(Number),
-      },
-      pendingSandboxMessages: [],
-    });
+
+    await expect(promise3).rejects.toThrow(
+      expect.objectContaining({
+        name: "SandboxTimeoutError",
+        message: expect.stringMatching(
+          /Sandbox message timed out: type=SANDBOX_BAZ, sent=\d+, payloadSize=\d+B, pendingMessageCount=0/,
+        ),
+      }),
+    );
   });
 
   test("clears metadata for resolved messages", async () => {

diff --git a/src/sandbox/postMessage.ts b/src/sandbox/postMessage.ts
@@ -96,17 +96,6 @@ function removePendingMessageMetadata(id: UUID) {
 
 export class SandboxTimeoutError extends Error {
   override name = "SandboxTimeoutError";
-
-  constructor(
-    message: string,
-    public readonly sandboxMessage: PendingMessageMetadata | undefined,
-    public readonly pendingSandboxMessages: PendingMessageMetadata[],
-    options?: ErrorOptions,
-  ) {
-    super(message, options);
-    this.pendingSandboxMessages = pendingSandboxMessages;
-    this.sandboxMessage = sandboxMessage;
-  }
 }
 
 /** Use the postMessage API but expect a response from the target */
@@ -155,9 +144,10 @@ export default async function postMessage<TReturn extends Payload = Payload>({
     const messageMetadata = removePendingMessageMetadata(messageKey);
     if (isSpecificError(error, TimeoutError)) {
       throw new SandboxTimeoutError(
-        error.message,
-        messageMetadata,
-        [...pendingMessageMetadataMap.values()],
+        // Datadog strips any additional properties from the error object, so we include the metadata in the message
+        // itself for now until we have a better solution.
+        // See https://github.com/DataDog/browser-sdk/issues/3067
+        `Sandbox message timed out: type=${type}, sent=${messageMetadata?.timestamp}, payloadSize=${messageMetadata?.payloadSize}B, pendingMessageCount=${pendingMessageMetadataMap.size}`,
         {
           cause: error,
         },