From f62bb8f1567d5772b4f03987adaaad1365cf7f39 Mon Sep 17 00:00:00 2001 From: thejesh23 Date: Sat, 1 Aug 2026 02:31:27 -0700 Subject: [PATCH 1/2] Stop sendAndWait from emitting an unhandled rejection `sendAndWait` creates `idlePromise` and registers the event listener that can reject it, but the first consumer is only attached by the `Promise.race` further down -- after `await this.send(options)`, a full JSON-RPC round trip to the CLI. A `session.error` arriving in that window therefore rejects a promise that has no handler yet. Node's rejection tracker runs at the following checkpoint, well before the `session.send` response lands, and classifies it as unhandled, which terminates the process under the default `--unhandled-rejections=throw`. The window is reachable from ordinary, non-fatal traffic. `session.log` with `{ level: "error" }` emits a `session.error` carrying `errorType: "notification"` (asserted in test/e2e/session.e2e.test.ts), so a joined client or extension writing an error log line while another caller is mid-`sendAndWait` is enough. MCP servers failing to start and sub-agent errors do the same. A caller cannot defend against this: the rejection is on the internal promise, not on the one `sendAndWait` returns, so even correct `.catch`/`try` handling around the call does not prevent the crash. Attaching a no-op `catch` marks the promise handled without consuming the rejection, so the `Promise.race` still observes it and `sendAndWait` rejects with the original error exactly as before. The added test drives a session whose `session.send` RPC is held open, dispatches a `session.error` into the window, and asserts both that no `unhandledRejection` fires and that `sendAndWait` still rejects. It fails on the unfixed code with the error captured by the process-level listener. --- nodejs/src/session.ts | 7 +++ nodejs/test/session-send-and-wait.test.ts | 61 +++++++++++++++++++++++ 2 files changed, 68 insertions(+) create mode 100644 nodejs/test/session-send-and-wait.test.ts diff --git a/nodejs/src/session.ts b/nodejs/src/session.ts index 0d1d90fbbb..79fe52f304 100644 --- a/nodejs/src/session.ts +++ b/nodejs/src/session.ts @@ -726,6 +726,13 @@ export class CopilotSession { resolveIdle = resolve; rejectWithError = reject; }); + // A `session.error` can arrive while `send()`'s RPC is still in flight — + // that is, before the `Promise.race` below attaches the first consumer to + // `idlePromise`. Mark the promise handled now so such a rejection can never + // surface as an unhandled rejection, which terminates the process under + // Node's default `--unhandled-rejections=throw`. This extra `catch` does + // not consume the rejection: the `race` below still sees and rethrows it. + void idlePromise.catch(() => {}); let lastAssistantMessage: AssistantMessageEvent | undefined; diff --git a/nodejs/test/session-send-and-wait.test.ts b/nodejs/test/session-send-and-wait.test.ts new file mode 100644 index 0000000000..9e9b1eba4b --- /dev/null +++ b/nodejs/test/session-send-and-wait.test.ts @@ -0,0 +1,61 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + *--------------------------------------------------------------------------------------------*/ + +import { describe, expect, it, onTestFinished } from "vitest"; +import type { MessageConnection } from "vscode-jsonrpc/node.js"; +import { CopilotSession } from "../src/session.js"; +import type { SessionEvent } from "../src/generated/session-events.js"; + +/** Builds a `session.error` event, the shape `session.log(…, { level: "error" })` produces. */ +function errorEvent(message: string): SessionEvent { + return { + type: "session.error", + id: "00000000-0000-4000-8000-000000000001", + parentId: null, + timestamp: new Date().toISOString(), + data: { errorType: "notification", message }, + } as SessionEvent; +} + +describe("sendAndWait", () => { + it("does not emit an unhandled rejection when session.error arrives before the idle race is armed", async () => { + // Hold the `session.send` RPC open so the test can dispatch an event in the + // window between the event listener being registered and Promise.race + // attaching the first consumer to the internal idle promise. + let resolveSend: ((value: unknown) => void) | undefined; + const connection = { + sendRequest: () => + new Promise((resolve) => { + resolveSend = resolve; + }), + } as unknown as MessageConnection; + + const session = new CopilotSession("session-1", connection); + + const unhandled: unknown[] = []; + const onUnhandled = (reason: unknown): void => { + unhandled.push(reason); + }; + process.on("unhandledRejection", onUnhandled); + onTestFinished(() => { + process.off("unhandledRejection", onUnhandled); + }); + + const pending = session.sendAndWait({ prompt: "hi" }); + + // A session.error lands while send()'s RPC is still in flight. This is + // ordinary traffic: a joined client calling session.log(…, { level: "error" }) + // or an MCP server failing to start both produce one. + session._dispatchEvent(errorEvent("MCP server failed to start")); + + // Yield past a macrotask boundary so Node has run the checkpoint at which + // it classifies a rejection as unhandled. + await new Promise((resolve) => setTimeout(resolve, 0)); + + expect(unhandled).toEqual([]); + + resolveSend?.({ messageId: "msg-1" }); + await expect(pending).rejects.toThrow("MCP server failed to start"); + }); +}); From 90be27b14bbf95a27d29d6941124524f9da27e8f Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Tue, 4 Aug 2026 11:43:29 +0000 Subject: [PATCH 2/2] Model sendAndWait completion without rejected promise Represent idle and error events as a resolved outcome so an error received while session.send is pending cannot become an unhandled rejection. Preserve send failure precedence and first-event settlement, with focused ordering tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nodejs/src/session.ts | 25 +++--- nodejs/test/session-send-and-wait.test.ts | 102 +++++++++++++++++++--- 2 files changed, 99 insertions(+), 28 deletions(-) diff --git a/nodejs/src/session.ts b/nodejs/src/session.ts index 79fe52f304..d3b8195f20 100644 --- a/nodejs/src/session.ts +++ b/nodejs/src/session.ts @@ -720,19 +720,11 @@ export class CopilotSession { typeof optionsOrPrompt === "string" ? { prompt: optionsOrPrompt } : optionsOrPrompt; const effectiveTimeout = timeout ?? 60_000; - let resolveIdle: () => void; - let rejectWithError: (error: Error) => void; - const idlePromise = new Promise((resolve, reject) => { - resolveIdle = resolve; - rejectWithError = reject; + type SessionOutcome = { kind: "idle" } | { kind: "error"; error: Error }; + let resolveOutcome: (outcome: SessionOutcome) => void; + const outcomePromise = new Promise((resolve) => { + resolveOutcome = resolve; }); - // A `session.error` can arrive while `send()`'s RPC is still in flight — - // that is, before the `Promise.race` below attaches the first consumer to - // `idlePromise`. Mark the promise handled now so such a rejection can never - // surface as an unhandled rejection, which terminates the process under - // Node's default `--unhandled-rejections=throw`. This extra `catch` does - // not consume the rejection: the `race` below still sees and rethrows it. - void idlePromise.catch(() => {}); let lastAssistantMessage: AssistantMessageEvent | undefined; @@ -742,11 +734,11 @@ export class CopilotSession { if (event.type === "assistant.message") { lastAssistantMessage = event; } else if (event.type === "session.idle") { - resolveIdle(); + resolveOutcome({ kind: "idle" }); } else if (event.type === "session.error") { const error = new Error(event.data.message); error.stack = event.data.stack; - rejectWithError(error); + resolveOutcome({ kind: "error", error }); } }); @@ -765,7 +757,10 @@ export class CopilotSession { effectiveTimeout ); }); - await Promise.race([idlePromise, timeoutPromise]); + const outcome = await Promise.race([outcomePromise, timeoutPromise]); + if (outcome.kind === "error") { + throw outcome.error; + } return lastAssistantMessage; } finally { diff --git a/nodejs/test/session-send-and-wait.test.ts b/nodejs/test/session-send-and-wait.test.ts index 9e9b1eba4b..8b6e390c4a 100644 --- a/nodejs/test/session-send-and-wait.test.ts +++ b/nodejs/test/session-send-and-wait.test.ts @@ -7,6 +7,17 @@ import type { MessageConnection } from "vscode-jsonrpc/node.js"; import { CopilotSession } from "../src/session.js"; import type { SessionEvent } from "../src/generated/session-events.js"; +function sessionEvent(type: "session.idle", data: Record = {}): SessionEvent { + return { + type, + id: "00000000-0000-4000-8000-000000000001", + parentId: null, + timestamp: new Date().toISOString(), + ephemeral: true, + data, + } as SessionEvent; +} + /** Builds a `session.error` event, the shape `session.log(…, { level: "error" })` produces. */ function errorEvent(message: string): SessionEvent { return { @@ -18,20 +29,38 @@ function errorEvent(message: string): SessionEvent { } as SessionEvent; } +function controlledSession(): { + session: CopilotSession; + sendStarted: Promise; + resolveSend: () => void; + rejectSend: (error: Error) => void; +} { + let resolveSendRequest: ((value: unknown) => void) | undefined; + let rejectSendRequest: ((error: Error) => void) | undefined; + let markSendStarted: () => void; + const sendStarted = new Promise((resolve) => { + markSendStarted = resolve; + }); + const connection = { + sendRequest: () => + new Promise((resolve, reject) => { + resolveSendRequest = resolve; + rejectSendRequest = reject; + markSendStarted(); + }), + } as unknown as MessageConnection; + + return { + session: new CopilotSession("session-1", connection), + sendStarted, + resolveSend: () => resolveSendRequest?.({ messageId: "msg-1" }), + rejectSend: (error) => rejectSendRequest?.(error), + }; +} + describe("sendAndWait", () => { it("does not emit an unhandled rejection when session.error arrives before the idle race is armed", async () => { - // Hold the `session.send` RPC open so the test can dispatch an event in the - // window between the event listener being registered and Promise.race - // attaching the first consumer to the internal idle promise. - let resolveSend: ((value: unknown) => void) | undefined; - const connection = { - sendRequest: () => - new Promise((resolve) => { - resolveSend = resolve; - }), - } as unknown as MessageConnection; - - const session = new CopilotSession("session-1", connection); + const { session, sendStarted, resolveSend } = controlledSession(); const unhandled: unknown[] = []; const onUnhandled = (reason: unknown): void => { @@ -43,6 +72,7 @@ describe("sendAndWait", () => { }); const pending = session.sendAndWait({ prompt: "hi" }); + await sendStarted; // A session.error lands while send()'s RPC is still in flight. This is // ordinary traffic: a joined client calling session.log(…, { level: "error" }) @@ -55,7 +85,53 @@ describe("sendAndWait", () => { expect(unhandled).toEqual([]); - resolveSend?.({ messageId: "msg-1" }); + resolveSend(); await expect(pending).rejects.toThrow("MCP server failed to start"); }); + + it("preserves an early idle event until send completes", async () => { + const { session, sendStarted, resolveSend } = controlledSession(); + const pending = session.sendAndWait({ prompt: "hi" }); + await sendStarted; + + session._dispatchEvent(sessionEvent("session.idle")); + + const stateBeforeSend = await Promise.race([ + pending.then(() => "settled"), + new Promise<"pending">((resolve) => setTimeout(() => resolve("pending"), 0)), + ]); + expect(stateBeforeSend).toBe("pending"); + + resolveSend(); + await expect(pending).resolves.toBeUndefined(); + }); + + it("preserves the send rejection when a session error arrives first", async () => { + const { session, sendStarted, rejectSend } = controlledSession(); + const pending = session.sendAndWait({ prompt: "hi" }); + await sendStarted; + + session._dispatchEvent(errorEvent("session error")); + rejectSend(new Error("send failed")); + + await expect(pending).rejects.toThrow("send failed"); + }); + + it("uses the first session outcome observed while send is in flight", async () => { + const idleFirst = controlledSession(); + const idleFirstPending = idleFirst.session.sendAndWait({ prompt: "hi" }); + await idleFirst.sendStarted; + idleFirst.session._dispatchEvent(sessionEvent("session.idle")); + idleFirst.session._dispatchEvent(errorEvent("later error")); + idleFirst.resolveSend(); + await expect(idleFirstPending).resolves.toBeUndefined(); + + const errorFirst = controlledSession(); + const errorFirstPending = errorFirst.session.sendAndWait({ prompt: "hi" }); + await errorFirst.sendStarted; + errorFirst.session._dispatchEvent(errorEvent("first error")); + errorFirst.session._dispatchEvent(sessionEvent("session.idle")); + errorFirst.resolveSend(); + await expect(errorFirstPending).rejects.toThrow("first error"); + }); });