From 79a763e10d85a9b8beb688ccbf47d9d2dd8ceabd Mon Sep 17 00:00:00 2001
From: Jacob Lee <jacoblee93@gmail.com>
Date: Thu, 22 Feb 2024 14:32:25 -0800
Subject: [PATCH] Improve JS async error handling for batch runs (#466)

This way, if an error occurs in a background autobatch processing run,
it will be logged instead of bubbling up an error that is uncatchable to
the user.

@hinthornw @dqbd @barberscott
---
 js/src/client.ts                  | 37 +++++++++++++++----------------
 js/src/tests/batch_client.test.ts | 32 ++++++++++++++++++++++++++
 2 files changed, 50 insertions(+), 19 deletions(-)

diff --git a/js/src/client.ts b/js/src/client.ts
index 8be8a3eac..5643530d6 100644
--- a/js/src/client.ts
+++ b/js/src/client.ts
@@ -551,7 +551,9 @@ export class Client {
       this.autoBatchTimeout = setTimeout(
         () => {
           this.autoBatchTimeout = undefined;
-          void this.drainAutoBatchQueue();
+          // This error would happen in the background and is uncatchable
+          // from the outside. So just log instead.
+          void this.drainAutoBatchQueue().catch(console.error);
         },
         oldTimeout
           ? this.autoBatchAggregationDelayMs
@@ -597,7 +599,7 @@ export class Client {
       void this.processRunOperation({
         action: "create",
         item: runCreate,
-      });
+      }).catch(console.error);
       return;
     }
     const mergedRunCreateParams = await mergeRuntimeEnvIntoRunCreates([
@@ -694,22 +696,17 @@ export class Client {
       "Content-Type": "application/json",
       Accept: "application/json",
     };
-
-    try {
-      const response = await this.caller.call(
-        fetch,
-        `${this.apiUrl}/runs/batch`,
-        {
-          method: "POST",
-          headers,
-          body: JSON.stringify(body),
-          signal: AbortSignal.timeout(this.timeout_ms),
-        }
-      );
-      await raiseForStatus(response, "batch create run");
-    } catch (e) {
-      console.error(`Failed to batch create runs: ${e}`);
-    }
+    const response = await this.caller.call(
+      fetch,
+      `${this.apiUrl}/runs/batch`,
+      {
+        method: "POST",
+        headers,
+        body: JSON.stringify(body),
+        signal: AbortSignal.timeout(this.timeout_ms),
+      }
+    );
+    await raiseForStatus(response, "batch create run");
   }
 
   public async updateRun(runId: string, run: RunUpdate): Promise<void> {
@@ -737,7 +734,9 @@ export class Client {
         await this.processRunOperation({ action: "update", item: data }, true);
         return;
       } else {
-        void this.processRunOperation({ action: "update", item: data });
+        void this.processRunOperation({ action: "update", item: data }).catch(
+          console.error
+        );
       }
       return;
     }
diff --git a/js/src/tests/batch_client.test.ts b/js/src/tests/batch_client.test.ts
index 7f69b8e49..7a0e1797c 100644
--- a/js/src/tests/batch_client.test.ts
+++ b/js/src/tests/batch_client.test.ts
@@ -60,6 +60,38 @@ describe("Batch client tracing", () => {
     );
   });
 
+  it("should not throw an error if fetch fails for batch requests", async () => {
+    const client = new Client({
+      apiKey: "test-api-key",
+      autoBatchTracing: true,
+    });
+    jest.spyOn((client as any).caller, "call").mockImplementation(() => {
+      throw new Error("Mock error!");
+    });
+    jest
+      .spyOn(client as any, "batchEndpointIsSupported")
+      .mockResolvedValue(true);
+    const projectName = "__test_batch";
+
+    const runId = uuidv4();
+    const dottedOrder = convertToDottedOrderFormat(
+      new Date().getTime() / 1000,
+      runId
+    );
+
+    await client.createRun({
+      id: runId,
+      project_name: projectName,
+      name: "test_run",
+      run_type: "llm",
+      inputs: { text: "hello world" },
+      trace_id: runId,
+      dotted_order: dottedOrder,
+    });
+
+    await new Promise((resolve) => setTimeout(resolve, 300));
+  });
+
   it("Create + update batching should merge into a single call", async () => {
     const client = new Client({
       apiKey: "test-api-key",