diff --git a/.changeset/distinct-abort-error-code.md b/.changeset/distinct-abort-error-code.md new file mode 100644 index 0000000000..ae959da9bf --- /dev/null +++ b/.changeset/distinct-abort-error-code.md @@ -0,0 +1,6 @@ +--- +'@modelcontextprotocol/core-internal': patch +'@modelcontextprotocol/client': patch +--- + +Cancelling a request via `AbortSignal` (`controller.abort()`) now rejects with the new `SdkErrorCode.RequestAborted`, distinct from `SdkErrorCode.RequestTimeout`. Previously both a deliberate abort and the request's own timeout elapsing surfaced as `RequestTimeout`, so callers could not tell them apart (`Protocol.request()`, the response-cache warm-hit path, and `Client.listen()` all shared the same fallback). The request's own timeout still reports `RequestTimeout` unchanged. diff --git a/packages/client/src/client/client.ts b/packages/client/src/client/client.ts index 096a8660e2..a1be6bde29 100644 --- a/packages/client/src/client/client.ts +++ b/packages/client/src/client/client.ts @@ -1841,12 +1841,19 @@ export class Client extends Protocol { if (hit !== undefined) { // A pre-aborted caller signal must reject the same way it would on // the wire path (`Protocol.request()` wraps an already-aborted - // signal as `SdkError(RequestTimeout, reason)`); without this guard + // signal as `SdkError(RequestAborted, reason)`); without this guard // a cache hit would resolve successfully and silently swallow the // abort. if (options?.signal?.aborted) { const reason = options.signal.reason; - throw reason instanceof SdkError ? reason : new SdkError(SdkErrorCode.RequestTimeout, String(reason)); + throw reason instanceof SdkError + ? reason + : new SdkError( + SdkErrorCode.RequestAborted, + reason === undefined ? 'Request was aborted' : String(reason), + undefined, + reason === undefined ? undefined : { cause: reason } + ); } return hit.value as R; } @@ -2010,12 +2017,19 @@ export class Client extends Protocol { // Honor RequestOptions.signal exactly as request() does: an // already-aborted signal rejects synchronously before any setup, and - // the rejection is the same `SdkError(RequestTimeout, reason)` wrap + // the rejection is the same `SdkError(RequestAborted, reason)` wrap // request() / `_serveFromCache` apply (unless `reason` is already an // SdkError — preserved verbatim). if (options?.signal?.aborted) { const reason = options.signal.reason; - throw reason instanceof SdkError ? reason : new SdkError(SdkErrorCode.RequestTimeout, String(reason)); + throw reason instanceof SdkError + ? reason + : new SdkError( + SdkErrorCode.RequestAborted, + reason === undefined ? 'Request was aborted' : String(reason), + undefined, + reason === undefined ? undefined : { cause: reason } + ); } const requestAbort = new AbortController(); diff --git a/packages/client/test/client/listen.test.ts b/packages/client/test/client/listen.test.ts index acc8e33d72..ada5fdcb0e 100644 --- a/packages/client/test/client/listen.test.ts +++ b/packages/client/test/client/listen.test.ts @@ -379,7 +379,7 @@ describe('Client.listen()', () => { await client.close(); }); - it('options.signal already aborted: listen() rejects with SdkError(RequestTimeout) before any setup (parity with request())', async () => { + it('options.signal already aborted: listen() rejects with SdkError(RequestAborted) before any setup (parity with request())', async () => { const { clientTx, written } = await scriptedModern(); const client = new Client({ name: 'c', version: '1' }, { versionNegotiation: { mode: 'auto' } }); await client.connect(clientTx); @@ -388,9 +388,9 @@ describe('Client.listen()', () => { ac.abort('user cancelled'); const error = await client.listen({ toolsListChanged: true }, { signal: ac.signal }).catch(e => e as SdkError); // Same wrap as `Protocol.request()` / `_serveFromCache`: a non-SdkError - // reason is wrapped as RequestTimeout; the reason text is preserved. + // reason is wrapped as RequestAborted; the reason text is preserved. expect(error).toBeInstanceOf(SdkError); - expect((error as SdkError).code).toBe(SdkErrorCode.RequestTimeout); + expect((error as SdkError).code).toBe(SdkErrorCode.RequestAborted); expect((error as SdkError).message).toContain('user cancelled'); // No subscriptions/listen reached the wire; no listen state registered. await flush(); diff --git a/packages/client/test/client/responseCache.test.ts b/packages/client/test/client/responseCache.test.ts index e2e2326705..0941641dca 100644 --- a/packages/client/test/client/responseCache.test.ts +++ b/packages/client/test/client/responseCache.test.ts @@ -1143,7 +1143,7 @@ describe('Client honours cacheHints (SEP-2549)', () => { expect(wireCount('resources/read')).toBe(3); }); - it('a pre-aborted signal on a warm-cache hit rejects with SdkError(RequestTimeout) — the abort is not swallowed by the cache serve', async () => { + it('a pre-aborted signal on a warm-cache hit rejects with SdkError(RequestAborted) — the abort is not swallowed by the cache serve', async () => { const store = new InMemoryResponseCacheStore(); const { clientTx, listCount } = await scriptedModernServer([[TOOL_A]], { listHint: { ttlMs: 60_000, cacheScope: 'public' } }); const client = modernClient(store); @@ -1158,7 +1158,7 @@ describe('Client honours cacheHints (SEP-2549)', () => { ac.abort('user cancelled'); const error = await client.listTools(undefined, { signal: ac.signal }).catch(e => e as SdkError); expect(error).toBeInstanceOf(SdkError); - expect((error as SdkError).code).toBe(SdkErrorCode.RequestTimeout); + expect((error as SdkError).code).toBe(SdkErrorCode.RequestAborted); expect((error as SdkError).message).toContain('user cancelled'); // The aborted call did not reach the wire. expect(listCount()).toBe(1); diff --git a/packages/core-internal/src/errors/sdkErrors.ts b/packages/core-internal/src/errors/sdkErrors.ts index 3ad48cc7f6..00c5877004 100644 --- a/packages/core-internal/src/errors/sdkErrors.ts +++ b/packages/core-internal/src/errors/sdkErrors.ts @@ -24,6 +24,13 @@ export enum SdkErrorCode { // Transport errors /** Request timed out waiting for response */ RequestTimeout = 'REQUEST_TIMEOUT', + /** + * The request's `AbortSignal` fired before a response arrived. Distinct + * from {@linkcode RequestTimeout} so callers can tell a deliberate + * `controller.abort()` apart from the request's own timeout elapsing — + * both used to surface as `RequestTimeout`. + */ + RequestAborted = 'REQUEST_ABORTED', /** Connection was closed */ ConnectionClosed = 'CONNECTION_CLOSED', /** Failed to send message */ diff --git a/packages/core-internal/src/shared/inputRequiredDriver.ts b/packages/core-internal/src/shared/inputRequiredDriver.ts index 673461cbc9..0b3d9434f9 100644 --- a/packages/core-internal/src/shared/inputRequiredDriver.ts +++ b/packages/core-internal/src/shared/inputRequiredDriver.ts @@ -160,7 +160,16 @@ export function inputRequiredRoundsExceededMessage(method: string, maxRounds: nu export function sleep(ms: number, signal: AbortSignal | undefined): Promise { return new Promise((resolve, reject) => { if (signal?.aborted) { - reject(signal.reason instanceof SdkError ? signal.reason : new SdkError(SdkErrorCode.RequestTimeout, String(signal.reason))); + reject( + signal.reason instanceof SdkError + ? signal.reason + : new SdkError( + SdkErrorCode.RequestAborted, + signal.reason === undefined ? 'Request was aborted' : String(signal.reason), + undefined, + signal.reason === undefined ? undefined : { cause: signal.reason } + ) + ); return; } const timer = setTimeout(() => { @@ -169,7 +178,16 @@ export function sleep(ms: number, signal: AbortSignal | undefined): Promise { clearTimeout(timer); - reject(signal?.reason instanceof SdkError ? signal.reason : new SdkError(SdkErrorCode.RequestTimeout, String(signal?.reason))); + reject( + signal?.reason instanceof SdkError + ? signal.reason + : new SdkError( + SdkErrorCode.RequestAborted, + signal?.reason === undefined ? 'Request was aborted' : String(signal?.reason), + undefined, + signal?.reason === undefined ? undefined : { cause: signal?.reason } + ) + ); }; signal?.addEventListener('abort', onAbort, { once: true }); }); diff --git a/packages/core-internal/src/shared/protocol.ts b/packages/core-internal/src/shared/protocol.ts index 9d60be8efe..54648a6e27 100644 --- a/packages/core-internal/src/shared/protocol.ts +++ b/packages/core-internal/src/shared/protocol.ts @@ -1412,14 +1412,21 @@ export abstract class Protocol { } // An already-aborted caller signal must surface the same way an - // in-flight abort does (`SdkError(RequestTimeout, reason)` via + // in-flight abort does (`SdkError(RequestAborted, reason)` via // `cancel()` below). Bare `throwIfAborted()` would propagate the // raw `signal.reason` instead, so callers that introduce an async // hop before `request()` (e.g. a cache freshness check) would see // a different rejection type depending on where the abort lands. if (options?.signal?.aborted) { const reason = options.signal.reason; - throw reason instanceof SdkError ? reason : new SdkError(SdkErrorCode.RequestTimeout, String(reason)); + throw reason instanceof SdkError + ? reason + : new SdkError( + SdkErrorCode.RequestAborted, + reason === undefined ? 'Request was aborted' : String(reason), + undefined, + reason === undefined ? undefined : { cause: reason } + ); } // Spec basic/patterns/cancellation §Transport-Specific (2026-07-28): @@ -1501,8 +1508,21 @@ export abstract class Protocol { requestAbort.abort(); } - // Wrap the reason in an SdkError if it isn't already - const error = reason instanceof SdkError ? reason : new SdkError(SdkErrorCode.RequestTimeout, String(reason)); + // Wrap the reason in an SdkError if it isn't already. The only + // caller that reaches this fallback is `onAbort` below — + // `timeoutHandler` always passes a pre-wrapped + // `SdkError(RequestTimeout, ...)`, so `reason instanceof + // SdkError` is already true on that path and this branch is + // abort-only in practice. + const error = + reason instanceof SdkError + ? reason + : new SdkError( + SdkErrorCode.RequestAborted, + reason === undefined ? 'Request was aborted' : String(reason), + undefined, + reason === undefined ? undefined : { cause: reason } + ); reject(error); }; diff --git a/packages/core-internal/test/shared/abortReasonPassthrough.test.ts b/packages/core-internal/test/shared/abortReasonPassthrough.test.ts index 3277edef27..76432008cc 100644 --- a/packages/core-internal/test/shared/abortReasonPassthrough.test.ts +++ b/packages/core-internal/test/shared/abortReasonPassthrough.test.ts @@ -1,8 +1,8 @@ import { describe, expect, it, vi } from 'vitest'; +import { SdkError, SdkErrorCode } from '../../src/errors/sdkErrors'; import type { BaseContext } from '../../src/shared/protocol'; import { Protocol } from '../../src/shared/protocol'; -import { SdkError, SdkErrorCode } from '../../src/errors/sdkErrors'; import { InMemoryTransport } from '../../src/util/inMemory'; class TestProtocolImpl extends Protocol { @@ -50,16 +50,68 @@ describe('request() abort-reason passthrough', () => { await expect(protocol.request({ method: 'ping' }, { signal: controller.signal })).rejects.toBe(reason); }); - it('wraps a non-SdkError abort reason in SdkError(RequestTimeout)', async () => { + it('wraps a non-SdkError abort reason in SdkError(RequestAborted) and preserves cause', async () => { const protocol = await connectedProtocol(); const controller = new AbortController(); - controller.abort(new Error('plain')); + const root = new Error('plain'); + controller.abort(root); const rejection = await protocol.request({ method: 'ping' }, { signal: controller.signal }).then( () => { throw new Error('request unexpectedly resolved'); }, - (e: unknown) => e + (error: unknown) => error + ); + expect(rejection).toBeInstanceOf(SdkError); + expect((rejection as SdkError).code).toBe(SdkErrorCode.RequestAborted); + expect((rejection as SdkError).cause).toBe(root); + }); + + it('handles default controller.abort() DOMException reason', async () => { + const protocol = await connectedProtocol(); + const controller = new AbortController(); + controller.abort(); + + const rejection = await protocol.request({ method: 'ping' }, { signal: controller.signal }).then( + () => { + throw new Error('request unexpectedly resolved'); + }, + (error: unknown) => error + ); + expect(rejection).toBeInstanceOf(SdkError); + expect((rejection as SdkError).code).toBe(SdkErrorCode.RequestAborted); + expect((rejection as SdkError).message).toContain('This operation was aborted'); + expect((rejection as SdkError).cause).toBe(controller.signal.reason); + }); + + it('distinguishes an in-flight abort (RequestAborted) from a real timeout (RequestTimeout)', async () => { + const protocol = await connectedProtocol(); + const controller = new AbortController(); + + // Abort well before the timeout would fire, so a REQUEST_TIMEOUT + // rejection here could only come from the abort path misreporting + // itself as a timeout. + setTimeout(() => controller.abort(new DOMException('User cancelled', 'AbortError')), 10); + + const rejection = await protocol.request({ method: 'ping' }, { signal: controller.signal, timeout: 60_000 }).then( + () => { + throw new Error('request unexpectedly resolved'); + }, + (error: unknown) => error + ); + expect(rejection).toBeInstanceOf(SdkError); + expect((rejection as SdkError).code).toBe(SdkErrorCode.RequestAborted); + expect((rejection as SdkError).code).not.toBe(SdkErrorCode.RequestTimeout); + }); + + it('still reports an actual timeout as SdkError(RequestTimeout), not RequestAborted', async () => { + const protocol = await connectedProtocol(); + + const rejection = await protocol.request({ method: 'ping' }, { timeout: 10 }).then( + () => { + throw new Error('request unexpectedly resolved'); + }, + (error: unknown) => error ); expect(rejection).toBeInstanceOf(SdkError); expect((rejection as SdkError).code).toBe(SdkErrorCode.RequestTimeout); diff --git a/packages/core-internal/test/shared/inputRequiredDriver.test.ts b/packages/core-internal/test/shared/inputRequiredDriver.test.ts index 2d4ec5d138..4f585e1a0d 100644 --- a/packages/core-internal/test/shared/inputRequiredDriver.test.ts +++ b/packages/core-internal/test/shared/inputRequiredDriver.test.ts @@ -297,10 +297,31 @@ describe('driver loop', () => { }); // Abort while the loop is in the 250 ms pacing sleep — the call must // settle without waiting it out. - const aborted = new SdkError(SdkErrorCode.RequestTimeout, 'aborted'); + const aborted = new SdkError(SdkErrorCode.RequestAborted, 'aborted'); controller.abort(aborted); const start = Date.now(); await expect(outcome).rejects.toBe(aborted); expect(Date.now() - start).toBeLessThan(REQUEST_STATE_ONLY_LEG_PACING_MS); }); + + test('the requestState-only pacing sleep wraps a non-SdkError abort in RequestAborted', async () => { + const controller = new AbortController(); + const outcome = runInputRequiredDriver({ + config: { autoFulfill: true, maxRounds: 5 }, + method: 'test', + originalParams: undefined, + firstPayload: { inputRequests: {}, requestState: 'opaque' }, + requestOptions: {}, + signal: controller.signal, + hooks: { + dispatchInputRequest: () => Promise.resolve({}), + retry: () => Promise.resolve({ content: [] }) + } + }); + controller.abort('user cancelled pacing'); + const rejection = await outcome.catch(error => error); + expect(rejection).toBeInstanceOf(SdkError); + expect((rejection as SdkError).code).toBe(SdkErrorCode.RequestAborted); + expect((rejection as SdkError).message).toContain('user cancelled pacing'); + }); }); diff --git a/packages/core-internal/test/types/errorSurfacePins.test.ts b/packages/core-internal/test/types/errorSurfacePins.test.ts index e2f52c2410..044f8ac2b0 100644 --- a/packages/core-internal/test/types/errorSurfacePins.test.ts +++ b/packages/core-internal/test/types/errorSurfacePins.test.ts @@ -73,6 +73,7 @@ describe('SdkErrorCode', () => { NotInitialized: 'NOT_INITIALIZED', CapabilityNotSupported: 'CAPABILITY_NOT_SUPPORTED', RequestTimeout: 'REQUEST_TIMEOUT', + RequestAborted: 'REQUEST_ABORTED', ConnectionClosed: 'CONNECTION_CLOSED', SendFailed: 'SEND_FAILED', InvalidResult: 'INVALID_RESULT', diff --git a/skills/mcp-typescript-sdk-upgrade-to-v2/references/behavioral-changes.md b/skills/mcp-typescript-sdk-upgrade-to-v2/references/behavioral-changes.md index e12d54a800..03869a2869 100644 --- a/skills/mcp-typescript-sdk-upgrade-to-v2/references/behavioral-changes.md +++ b/skills/mcp-typescript-sdk-upgrade-to-v2/references/behavioral-changes.md @@ -38,8 +38,11 @@ rewrite required unless noted. fires (v1 let them run to completion). `InMemoryTransport.close()` no longer double-fires `onclose` on the initiating side. - **`Protocol.request()` with an already-aborted signal** rejects with - `SdkError(SdkErrorCode.RequestTimeout, reason)` instead of throwing the raw - `signal.reason`, matching the in-flight-abort path. + `SdkError(SdkErrorCode.RequestAborted, reason)` instead of throwing the raw + `signal.reason`, matching the in-flight-abort path. (A distinct `RequestAborted` + code, separate from `RequestTimeout` — earlier `2.0.0-alpha` releases used + `RequestTimeout` for both, which made a deliberate `controller.abort()` + indistinguishable from the request's own timeout elapsing.) - **OAuth discovery (`discoverOAuthProtectedResourceMetadata` / `discoverOAuthMetadata`, transitively `auth()`) throws on fetch `TypeError`** (DNS failure, `ECONNREFUSED`, invalid URL) in Node and Cloudflare Workers instead of swallowing it as a CORS miss @@ -59,7 +62,7 @@ rewrite required unless noted. verbs on top of it — `callTool()` and the cacheable list verbs — perform async work first (header-mirroring scan, response-cache freshness, output-validator resolution), so an abort fired in the same tick can land before the frame is ever sent: the call - rejects with `SdkError(RequestTimeout, reason)` and **no `notifications/cancelled` is + rejects with `SdkError(RequestAborted, reason)` and **no `notifications/cancelled` is emitted** (nothing was in flight). v1 sent the frame synchronously from these verbs. Once the frame is on the wire, aborting still sends `notifications/cancelled` before rejecting. diff --git a/skills/mcp-typescript-sdk-upgrade-to-v2/references/errors.md b/skills/mcp-typescript-sdk-upgrade-to-v2/references/errors.md index b76a8f4856..9ad19d12dd 100644 --- a/skills/mcp-typescript-sdk-upgrade-to-v2/references/errors.md +++ b/skills/mcp-typescript-sdk-upgrade-to-v2/references/errors.md @@ -39,6 +39,7 @@ class to match per scenario: | Scenario | v1 | v2 | | ------------------------------------------------ | ----------------------------------------- | ------------------------------------------------------------------ | | Request timeout | `McpError` + `ErrorCode.RequestTimeout` | `SdkError` + `SdkErrorCode.RequestTimeout` | +| Request cancelled via `AbortSignal` | `McpError` + `ErrorCode.RequestTimeout` / raw `reason` | `SdkError` + `SdkErrorCode.RequestAborted` | | Connection closed | `McpError` + `ErrorCode.ConnectionClosed` | `SdkError` + `SdkErrorCode.ConnectionClosed` | | Capability not supported | `new Error(...)` | `SdkError` + `SdkErrorCode.CapabilityNotSupported` | | Not connected | `new Error('Not connected')` | `SdkError` + `SdkErrorCode.NotConnected` | @@ -158,6 +159,7 @@ the third argument — `new SdkHttpError(SdkErrorCode.ClientHttpNotImplemented, | `NotInitialized` | Protocol is not initialized | | `CapabilityNotSupported` | Required capability is not supported | | `RequestTimeout` | Request timed out waiting for response | +| `RequestAborted` | The request's `AbortSignal` fired before a response arrived | | `ConnectionClosed` | Connection was closed | | `SendFailed` | Failed to send message | | `InvalidResult` | Response result failed local schema validation | diff --git a/test/integration/test/client/client.test.ts b/test/integration/test/client/client.test.ts index bc5d19b8cf..c04ff2ec75 100644 --- a/test/integration/test/client/client.test.ts +++ b/test/integration/test/client/client.test.ts @@ -1231,6 +1231,13 @@ test('should handle request timeout', async () => { await expect(client.listResources(undefined, { timeout: 0 })).rejects.toMatchObject({ code: SdkErrorCode.RequestTimeout }); + + // Request cancelled with AbortSignal should reject with RequestAborted + const controller = new AbortController(); + controller.abort('user aborted'); + await expect(client.listResources(undefined, { signal: controller.signal })).rejects.toMatchObject({ + code: SdkErrorCode.RequestAborted + }); }); /***