Skip to content

Commit

Permalink
Improve JS async error handling for batch runs (#466)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
jacoblee93 authored Feb 22, 2024
1 parent dce7251 commit 79a763e
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 19 deletions.
37 changes: 18 additions & 19 deletions js/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -597,7 +599,7 @@ export class Client {
void this.processRunOperation({
action: "create",
item: runCreate,
});
}).catch(console.error);
return;
}
const mergedRunCreateParams = await mergeRuntimeEnvIntoRunCreates([
Expand Down Expand Up @@ -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> {
Expand Down Expand Up @@ -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;
}
Expand Down
32 changes: 32 additions & 0 deletions js/src/tests/batch_client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down

0 comments on commit 79a763e

Please sign in to comment.