Skip to content

Commit c167bc3

Browse files
committed
TS: emit JSON-RPC error on pending-buffer overflow + guard drop
Carries forward the Rust SDK PR #1394 follow-up review feedback into the TS port: 1. Cap the per-session inbound-request parked-waiter list at 128. When exceeded, reject the oldest waiter with 'pending session buffer overflow'. vscode-jsonrpc translates the rejection into a JSON-RPC error response (-32603) back to the runtime so the request id isn't left hanging — silently dropping it would leave the runtime waiting on the response until its own timeout. Mirrors Rust commit 491b442. 2. Use a distinct message ('pending session routing ended before session was registered') when the pending guard drops without registration. Lets debugging tell the overflow path from the create-failed path. Mirrors Rust commit e0ff254. Adds two tests: - overflow path resolves to overflow error for the oldest waiter, remaining 128 resolve normally after registration - guard-drop path rejects parked waiters with the distinct message
1 parent b64b85d commit c167bc3

2 files changed

Lines changed: 106 additions & 1 deletion

File tree

nodejs/src/client.ts

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1129,9 +1129,12 @@ export class CopilotClient {
11291129
this.pendingSessionEvents.clear();
11301130
for (const waiters of this.pendingSessionWaiters.values()) {
11311131
for (const w of waiters) {
1132+
// Distinct phrasing from the overflow-eviction path so the
1133+
// runtime / future debugging can tell the two cases apart.
1134+
// Matches the Rust SDK message in PR #1394 (commit e0ff254f).
11321135
w.reject(
11331136
new Error(
1134-
"Cloud session.create completed without registering this sessionId; request dropped"
1137+
"pending session routing ended before session was registered"
11351138
)
11361139
);
11371140
}
@@ -2271,6 +2274,18 @@ export class CopilotClient {
22712274
waiters = [];
22722275
this.pendingSessionWaiters.set(sessionId, waiters);
22732276
}
2277+
// Cap parked waiters per session id. When exceeded, reject the
2278+
// oldest with a distinct message; vscode-jsonrpc surfaces the
2279+
// rejection as a JSON-RPC error response to the runtime (rather
2280+
// than silently dropping, which would hang the runtime on the
2281+
// request id). Matches the Rust SDK fix in PR #1394 (commit
2282+
// 491b4427).
2283+
if (waiters.length >= CopilotClient.PENDING_SESSION_BUFFER_LIMIT) {
2284+
const oldest = waiters.shift();
2285+
if (oldest) {
2286+
oldest.reject(new Error("pending session buffer overflow"));
2287+
}
2288+
}
22742289
waiters.push({ resolve, reject });
22752290
});
22762291
}

nodejs/test/client.test.ts

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,96 @@ describe("CopilotClient", () => {
213213
await expect(parked).resolves.toEqual({ answer: "yes", wasFreeform: false });
214214
});
215215

216+
it("rejects parked requests with overflow error when buffer cap exceeded", async () => {
217+
const client = new CopilotClient();
218+
await client.start();
219+
onTestFinished(() => client.forceStop());
220+
221+
let resolveCreate: ((value: unknown) => void) | undefined;
222+
vi.spyOn((client as any).connection!, "sendRequest").mockImplementation(
223+
(method: unknown) => {
224+
if (method !== "session.create") return Promise.resolve({});
225+
return new Promise((resolve) => {
226+
resolveCreate = resolve;
227+
});
228+
}
229+
);
230+
231+
const sessionPromise = client.createCloudSession({
232+
onPermissionRequest: approveAll,
233+
cloud: { repository: { owner: "github", name: "copilot-sdk" } },
234+
onUserInputRequest: async () => ({ answer: "ok", wasFreeform: false }),
235+
});
236+
await new Promise<void>((r) => setImmediate(r));
237+
238+
const handler = (
239+
client as unknown as {
240+
handleUserInputRequest: (p: {
241+
sessionId: string;
242+
question: string;
243+
}) => Promise<{ answer: string; wasFreeform: boolean }>;
244+
}
245+
).handleUserInputRequest.bind(client);
246+
247+
// Park 128 + 1 requests; the oldest must reject with overflow message.
248+
// vscode-jsonrpc translates the rejection into a JSON-RPC error response
249+
// back to the runtime so the request id isn't left hanging.
250+
const parked: Promise<unknown>[] = [];
251+
for (let i = 0; i < 129; i++) {
252+
parked.push(handler({ sessionId: "cloud-session", question: `q${i}` }));
253+
}
254+
255+
await expect(parked[0]).rejects.toThrow(/pending session buffer overflow/);
256+
257+
resolveCreate!({ sessionId: "cloud-session" });
258+
await sessionPromise;
259+
260+
// The remaining 128 parked requests resolve after registration.
261+
for (let i = 1; i < 129; i++) {
262+
await expect(parked[i]).resolves.toEqual({ answer: "ok", wasFreeform: false });
263+
}
264+
});
265+
266+
it("rejects parked requests when pending routing ends without registration", async () => {
267+
const client = new CopilotClient();
268+
await client.start();
269+
onTestFinished(() => client.forceStop());
270+
271+
let rejectCreate: ((err: Error) => void) | undefined;
272+
vi.spyOn((client as any).connection!, "sendRequest").mockImplementation(
273+
(method: unknown) => {
274+
if (method !== "session.create") return Promise.resolve({});
275+
return new Promise((_resolve, reject) => {
276+
rejectCreate = reject;
277+
});
278+
}
279+
);
280+
281+
const sessionPromise = client.createCloudSession({
282+
onPermissionRequest: approveAll,
283+
cloud: { repository: { owner: "github", name: "copilot-sdk" } },
284+
});
285+
await new Promise<void>((r) => setImmediate(r));
286+
287+
const parked = (
288+
client as unknown as {
289+
handleUserInputRequest: (p: {
290+
sessionId: string;
291+
question: string;
292+
}) => Promise<{ answer: string; wasFreeform: boolean }>;
293+
}
294+
).handleUserInputRequest({ sessionId: "cloud-session", question: "ok?" });
295+
296+
// session.create fails before registration; guard drop must reject all
297+
// parked waiters with a distinct message (separate from overflow).
298+
rejectCreate!(new Error("create failed"));
299+
await expect(sessionPromise).rejects.toThrow();
300+
301+
await expect(parked).rejects.toThrow(
302+
/pending session routing ended before session was registered/
303+
);
304+
});
305+
216306
it("forwards clientName in session.resume request", async () => {
217307
const client = new CopilotClient();
218308
await client.start();

0 commit comments

Comments
 (0)