Skip to content

Commit a5ce8ab

Browse files
authored
feat(runtime, react): Explicit error handling and retires for IntelligenceAgent* (CopilotKit#3379)
<!-- Thank you for sending the PR! We appreciate you spending the time to work on these changes. Help us understand your motivation by explaining why you decided to make this change. **Please PLEASE reach out to us first before starting any significant work on new or existing features.** By the time you've gotten here, you're looking at creating a pull request so hopefully we're not too late. We love community contributions! That said, we want to make sure we're all on the same page before you start. Investing a lot of time and effort just to find out it doesn't align with the upstream project feels awful, and we don't want that to happen. It also helps to make sure the work you're planning isn't already in progress. As described in our contributing guide, please file an issue first: https://github.com/ag-ui-protocol/ag-ui/issues Or, reach out to us on Discord: https://discord.com/invite/6dffbvGU3D You can learn more about contributing to copilotkit here: https://github.com/copilotkit/copilotkit/blob/master/CONTRIBUTING.md Happy contributing! --> ## What does this PR do? This PR expands on reconnection and error handling for the IntelligenceAgent and IntelligenceAgentRunner interactions with the Phoenix lib/server, like timeouts and disconnects. It explicitly defines exponential backoff behavior (Phoenix has default settings, but they are silently disabled if you add error callbacks. While that was only done in the AgentRunner to begin with, it felt strange to leave it as a foot gun elsewhere). There is also added logic to ensure cleanup messages are sent (at best effort) before channels are closed. Additionally: * each agent run through the IntelligenceAgentRunner now gets its own web socket connection (before, it only got its own _channel_) * the IntelligenceAgent now stops a running agent by publishing a stop (custom AG-UI) message to Phoenix, which is handled by the IntelligenceAgentRunner. ## Related PRs and Issues - https://linear.app/copilotkit/issue/ENT-54 - https://linear.app/copilotkit/issue/ENT-59 ## Checklist - [ ] I have read the [Contribution Guide](https://github.com/copilotkit/copilotkit/blob/master/CONTRIBUTING.md) - [ ] If the PR changes or adds functionality, I have updated the relevant documentation
2 parents 57eb016 + b78189e commit a5ce8ab

6 files changed

Lines changed: 763 additions & 112 deletions

File tree

packages/v2/core/src/__tests__/intelligence-agent.test.ts

Lines changed: 205 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,113 @@ describe("IntelligenceAgent", () => {
257257
});
258258
});
259259

260+
describe("mid-run disconnect", () => {
261+
it("does not error the observable on a single channel crash (Phoenix retries)", () => {
262+
const agent = createAgent();
263+
let error: Error | null = null;
264+
agent.run(defaultInput).subscribe({
265+
next: () => {},
266+
error: (err) => {
267+
error = err;
268+
},
269+
});
270+
271+
const channel = getChannel(agent);
272+
channel.triggerJoin("ok");
273+
274+
// A single channel error should be a no-op — Phoenix handles rejoin.
275+
channel.triggerError("server crash");
276+
277+
expect(error).toBeNull();
278+
});
279+
280+
it("does not error the observable on a single socket error (Phoenix retries)", () => {
281+
const agent = createAgent();
282+
let error: Error | null = null;
283+
agent.run(defaultInput).subscribe({
284+
next: () => {},
285+
error: (err) => {
286+
error = err;
287+
},
288+
});
289+
290+
const channel = getChannel(agent);
291+
channel.triggerJoin("ok");
292+
293+
const socket = getSocket(agent);
294+
socket.triggerError(new Error("network failure"));
295+
296+
// A single socket error should not kill the connection — Phoenix retries.
297+
expect(error).toBeNull();
298+
});
299+
300+
it("errors the observable after MAX_CONSECUTIVE_ERRORS socket errors", async () => {
301+
const agent = createAgent();
302+
const promise = collectEvents(agent);
303+
304+
const socket = getSocket(agent);
305+
const channel = getChannel(agent);
306+
channel.triggerJoin("ok");
307+
308+
// Fire 5 consecutive errors (the threshold)
309+
for (let i = 0; i < 5; i++) {
310+
socket.triggerError(new Error("network failure"));
311+
}
312+
313+
const result = await promise;
314+
expect(result.completed).toBe(false);
315+
expect(result.error).toBeInstanceOf(Error);
316+
expect(result.error!.message).toContain("5 consecutive errors");
317+
});
318+
319+
it("cleans up socket and channel after reaching the error threshold", async () => {
320+
const agent = createAgent();
321+
const promise = collectEvents(agent);
322+
323+
const socket = getSocket(agent);
324+
const channel = getChannel(agent);
325+
channel.triggerJoin("ok");
326+
327+
for (let i = 0; i < 5; i++) {
328+
socket.triggerError();
329+
}
330+
331+
await promise;
332+
expect(channel.left).toBe(true);
333+
expect(socket.disconnected).toBe(true);
334+
});
335+
336+
it("resets the error counter on successful reconnection", () => {
337+
const agent = createAgent();
338+
let error: Error | null = null;
339+
agent.run(defaultInput).subscribe({
340+
next: () => {},
341+
error: (err) => {
342+
error = err;
343+
},
344+
});
345+
346+
const socket = getSocket(agent);
347+
const channel = getChannel(agent);
348+
channel.triggerJoin("ok");
349+
350+
// 4 errors (just below threshold)
351+
for (let i = 0; i < 4; i++) {
352+
socket.triggerError();
353+
}
354+
expect(error).toBeNull();
355+
356+
// Successful reconnect resets counter
357+
socket.triggerOpen();
358+
359+
// 4 more errors — still below threshold because counter was reset
360+
for (let i = 0; i < 4; i++) {
361+
socket.triggerError();
362+
}
363+
expect(error).toBeNull();
364+
});
365+
});
366+
260367
describe("join failures", () => {
261368
it("errors the observable on join error", async () => {
262369
const agent = createAgent();
@@ -286,66 +393,102 @@ describe("IntelligenceAgent", () => {
286393
});
287394

288395
describe("abortRun", () => {
289-
it("fires a REST POST to /agent/{agentId}/stop/{threadId} and cleans up", async () => {
396+
it("pushes a CUSTOM stop event to the channel", () => {
290397
const agent = createAgent();
291398
agent.run(defaultInput).subscribe({ next: () => {}, error: () => {} });
292399

293400
const channel = getChannel(agent);
294401
channel.triggerJoin("ok");
295402

296-
// Reset fetch mock to only track abort calls
297-
mockFetch.mockClear();
298-
mockFetch.mockResolvedValue({ ok: true });
299-
300403
agent.abortRun();
301404

302-
expect(mockFetch).toHaveBeenCalledTimes(1);
303-
const [url, options] = mockFetch.mock.calls[0];
304-
expect(url).toContain("/agent/my-agent/stop/thread-1");
305-
expect(options.method).toBe("POST");
306-
expect(options.headers).toMatchObject({
307-
"Content-Type": "application/json",
308-
Authorization: "Bearer abc",
405+
const stopPush = channel.pushLog.find(
406+
(c) =>
407+
c.payload?.type === EventType.CUSTOM && c.payload?.name === "stop",
408+
);
409+
expect(stopPush).toBeDefined();
410+
expect(stopPush!.payload).toMatchObject({
411+
type: EventType.CUSTOM,
412+
name: "stop",
413+
value: { threadId: "thread-1" },
309414
});
415+
});
416+
417+
it("defers cleanup until the push is acknowledged (ok)", () => {
418+
const agent = createAgent();
419+
agent.run(defaultInput).subscribe({ next: () => {}, error: () => {} });
420+
421+
const channel = getChannel(agent);
422+
channel.triggerJoin("ok");
423+
424+
agent.abortRun();
425+
426+
// Cleanup has NOT happened yet — waiting for push ACK
427+
expect(channel.left).toBe(false);
428+
429+
// Server acknowledges the stop push
430+
const stopEntry = channel.pushLog.find(
431+
(c) => c.payload?.name === "stop",
432+
)!;
433+
stopEntry.push.trigger("ok");
310434

311-
// Channel should be cleaned up
312435
expect(channel.left).toBe(true);
313436
});
314437

315-
it("is a no-op when fetch is unavailable (SSR)", () => {
438+
it("cleans up on push error reply", () => {
316439
const agent = createAgent();
317440
agent.run(defaultInput).subscribe({ next: () => {}, error: () => {} });
318441

319442
const channel = getChannel(agent);
320443
channel.triggerJoin("ok");
444+
agent.abortRun();
445+
446+
const stopEntry = channel.pushLog.find(
447+
(c) => c.payload?.name === "stop",
448+
)!;
449+
stopEntry.push.trigger("error");
321450

322-
vi.stubGlobal("fetch", undefined);
323-
expect(() => agent.abortRun()).not.toThrow();
324451
expect(channel.left).toBe(true);
325452
});
326453

327-
it("does not push any CUSTOM events to the channel", () => {
454+
it("cleans up on push timeout reply", () => {
328455
const agent = createAgent();
329456
agent.run(defaultInput).subscribe({ next: () => {}, error: () => {} });
330457

331458
const channel = getChannel(agent);
332459
channel.triggerJoin("ok");
460+
agent.abortRun();
461+
462+
const stopEntry = channel.pushLog.find(
463+
(c) => c.payload?.name === "stop",
464+
)!;
465+
stopEntry.push.trigger("timeout");
333466

334-
// Clear the push log (run may have pushed something)
335-
channel.pushLog.length = 0;
467+
expect(channel.left).toBe(true);
468+
});
469+
470+
it("cleans up immediately via fallback timer when socket is down", () => {
471+
vi.useFakeTimers();
472+
const agent = createAgent();
473+
agent.run(defaultInput).subscribe({ next: () => {}, error: () => {} });
336474

475+
const channel = getChannel(agent);
476+
channel.triggerJoin("ok");
337477
agent.abortRun();
338478

339-
// No CUSTOM stop pushes
340-
const stopPush = channel.pushLog.find(
341-
(c) => c.event === EventType.CUSTOM && c.payload?.name === "stop",
342-
);
343-
expect(stopPush).toBeUndefined();
479+
// No ACK will arrive (socket is down) — channel still open
480+
expect(channel.left).toBe(false);
481+
482+
// Fallback fires after 5 seconds
483+
vi.advanceTimersByTime(5_000);
484+
expect(channel.left).toBe(true);
485+
486+
vi.useRealTimers();
344487
});
345488

346-
it("is a no-op when no run is active", () => {
489+
it("cleans up immediately when no run is active", () => {
347490
const agent = createAgent();
348-
// Should not throw.
491+
// Should not throw and should not push anything
349492
expect(() => agent.abortRun()).not.toThrow();
350493
expect(mockFetch).not.toHaveBeenCalled();
351494
});
@@ -387,27 +530,6 @@ describe("IntelligenceAgent", () => {
387530
expect(options.credentials).toBe("include");
388531
});
389532

390-
it("forwards credentials on abortRun fetch when configured", async () => {
391-
const agent = new IntelligenceAgent({
392-
url: "ws://localhost:4000/socket",
393-
runtimeUrl: "http://localhost:4000",
394-
agentId: "my-agent",
395-
credentials: "include",
396-
});
397-
agent.run(defaultInput).subscribe({ next: () => {}, error: () => {} });
398-
399-
const channel = getChannel(agent);
400-
channel.triggerJoin("ok");
401-
402-
mockFetch.mockClear();
403-
mockFetch.mockResolvedValue({ ok: true });
404-
405-
agent.abortRun();
406-
407-
const [, options] = mockFetch.mock.calls[0];
408-
expect(options.credentials).toBe("include");
409-
});
410-
411533
it("omits credentials when not configured", async () => {
412534
const agent = createAgent();
413535
agent.run(defaultInput).subscribe({ next: () => {}, error: () => {} });
@@ -534,6 +656,42 @@ describe("IntelligenceAgent", () => {
534656
expect(result.error).toBeInstanceOf(Error);
535657
expect(result.error!.message).toContain("Failed to join channel");
536658
});
659+
660+
it("does not error the observable on a single channel crash (Phoenix retries)", () => {
661+
const agent = createAgent();
662+
let error: Error | null = null;
663+
(agent as any).connect(defaultInput).subscribe({
664+
next: () => {},
665+
error: (err: Error) => {
666+
error = err;
667+
},
668+
});
669+
670+
const channel = getChannel(agent);
671+
channel.triggerJoin("ok");
672+
channel.triggerError("server crash");
673+
674+
// Channel errors are handled by Phoenix auto-rejoin — no observer error.
675+
expect(error).toBeNull();
676+
});
677+
678+
it("errors the observable after MAX_CONSECUTIVE_ERRORS socket errors", async () => {
679+
const agent = createAgent();
680+
const promise = connectAgent(agent);
681+
682+
const socket = getSocket(agent);
683+
const channel = getChannel(agent);
684+
channel.triggerJoin("ok");
685+
686+
for (let i = 0; i < 5; i++) {
687+
socket.triggerError(new Error("network failure"));
688+
}
689+
690+
const result = await promise;
691+
expect(result.completed).toBe(false);
692+
expect(result.error).toBeInstanceOf(Error);
693+
expect(result.error!.message).toContain("5 consecutive errors");
694+
});
537695
});
538696

539697
describe("clone", () => {

packages/v2/core/src/__tests__/test-utils.ts

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -285,11 +285,12 @@ export class MockPush {
285285
export class MockChannel {
286286
public topic: string;
287287
public params: Record<string, any>;
288-
public pushLog: Array<{ event: string; payload: any }> = [];
288+
public pushLog: Array<{ event: string; payload: any; push: MockPush }> = [];
289289
public left = false;
290290

291291
private handlers = new Map<string, Array<(payload: any) => void>>();
292292
private joinPush = new MockPush();
293+
private errorHandlers: Array<(reason?: any) => void> = [];
293294

294295
constructor(topic: string = "", params: Record<string, any> = {}) {
295296
this.topic = topic;
@@ -304,13 +305,18 @@ export class MockChannel {
304305
return this.handlers.get(event)!.length;
305306
}
306307

308+
onError(callback: (reason?: any) => void): void {
309+
this.errorHandlers.push(callback);
310+
}
311+
307312
join(): MockPush {
308313
return this.joinPush;
309314
}
310315

311316
push(event: string, payload: any): MockPush {
312-
this.pushLog.push({ event, payload });
313-
return new MockPush();
317+
const mockPush = new MockPush();
318+
this.pushLog.push({ event, payload, push: mockPush });
319+
return mockPush;
314320
}
315321

316322
leave(): void {
@@ -328,6 +334,11 @@ export class MockChannel {
328334
handler(payload);
329335
}
330336
}
337+
338+
/** Test helper — simulate the channel crashing server-side. */
339+
triggerError(reason?: string): void {
340+
for (const handler of this.errorHandlers) handler(reason);
341+
}
331342
}
332343

333344
export class MockSocket {
@@ -337,6 +348,9 @@ export class MockSocket {
337348
public disconnected = false;
338349
public channels: MockChannel[] = [];
339350

351+
private errorHandlers: Array<(error?: any) => void> = [];
352+
private openHandlers: Array<() => void> = [];
353+
340354
constructor(url: string = "", opts: Record<string, any> = {}) {
341355
this.url = url;
342356
this.opts = opts;
@@ -350,9 +364,27 @@ export class MockSocket {
350364
this.disconnected = true;
351365
}
352366

367+
onError(callback: (error?: any) => void): void {
368+
this.errorHandlers.push(callback);
369+
}
370+
371+
onOpen(callback: () => void): void {
372+
this.openHandlers.push(callback);
373+
}
374+
353375
channel(topic: string, params: Record<string, any> = {}): MockChannel {
354376
const ch = new MockChannel(topic, params);
355377
this.channels.push(ch);
356378
return ch;
357379
}
380+
381+
/** Test helper — simulate the WebSocket transport erroring. */
382+
triggerError(error?: any): void {
383+
for (const handler of this.errorHandlers) handler(error);
384+
}
385+
386+
/** Test helper — simulate a successful (re)connection. */
387+
triggerOpen(): void {
388+
for (const handler of this.openHandlers) handler();
389+
}
358390
}

0 commit comments

Comments
 (0)