From 690d90270fa689f60994102af4423e563b8f31ef Mon Sep 17 00:00:00 2001 From: David Soria Parra Date: Tue, 19 May 2026 22:54:03 +0100 Subject: [PATCH 1/2] fix(client): treat HTTP 404 with session ID as session expiry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per the MCP spec (Streamable HTTP, Session Management), when a client receives an HTTP 404 in response to a request that carried an Mcp-Session-Id, the session has expired or been terminated server-side and the client must start a new session. StreamableHTTPClientTransport previously surfaced every non-401/403 error status — including 404 — as a generic ClientHttpNotImplemented (POST) or ClientHttpFailedToOpenStream (GET) error, with no way to distinguish session expiry from other failures. Consumers were left matching the response body, which only works against the reference server; servers that report expiry with a different body (e.g. a -32002 JSON-RPC code, or a plain-text/HTML proxy response) slipped through. Detect session expiry by status code alone, scoped to requests that actually carried a session ID: on a 404 when a session ID is set, clear the stale session ID (so a subsequent connect() issues a fresh initialize) and throw SdkError with the new SdkErrorCode.ClientHttpSessionExpired. A 404 without a session ID (e.g. wrong URL on initial connect) is unchanged and still surfaces as ClientHttpNotImplemented. Applied to both the POST (_send) and standalone GET SSE (_startOrAuthSse) paths; documents the new code in the migration guides. --- docs/migration-SKILL.md | 2 + docs/migration.md | 23 ++++++ packages/client/src/client/streamableHttp.ts | 27 +++++++ .../client/test/client/streamableHttp.test.ts | 80 +++++++++++++++---- packages/core/src/errors/sdkErrors.ts | 10 ++- 5 files changed, 125 insertions(+), 17 deletions(-) diff --git a/docs/migration-SKILL.md b/docs/migration-SKILL.md index dbe6a4e9f7..68377b8d4c 100644 --- a/docs/migration-SKILL.md +++ b/docs/migration-SKILL.md @@ -121,6 +121,7 @@ Two error classes now exist: | 403 after upscoping | `StreamableHTTPError` | `SdkError` with `SdkErrorCode.ClientHttpForbidden` | | Unexpected content type | `StreamableHTTPError` | `SdkError` with `SdkErrorCode.ClientHttpUnexpectedContent` | | Session termination failed | `StreamableHTTPError` | `SdkError` with `SdkErrorCode.ClientHttpFailedToTerminateSession` | +| Session expired (404 w/ session) | `StreamableHTTPError` (status 404) | `SdkError` with `SdkErrorCode.ClientHttpSessionExpired` | | Response result fails schema | `ZodError` (raw) | `SdkError` with `SdkErrorCode.InvalidResult` | New `SdkErrorCode` enum values: @@ -139,6 +140,7 @@ New `SdkErrorCode` enum values: - `SdkErrorCode.ClientHttpUnexpectedContent` = `'CLIENT_HTTP_UNEXPECTED_CONTENT'` - `SdkErrorCode.ClientHttpFailedToOpenStream` = `'CLIENT_HTTP_FAILED_TO_OPEN_STREAM'` - `SdkErrorCode.ClientHttpFailedToTerminateSession` = `'CLIENT_HTTP_FAILED_TO_TERMINATE_SESSION'` +- `SdkErrorCode.ClientHttpSessionExpired` = `'CLIENT_HTTP_SESSION_EXPIRED'` (thrown on HTTP 404 when a session ID was set; transport clears `sessionId` so reconnect re-`initialize`s; detection is status-only, body-agnostic) Update error handling: diff --git a/docs/migration.md b/docs/migration.md index cd3da6dcda..874faff184 100644 --- a/docs/migration.md +++ b/docs/migration.md @@ -722,6 +722,7 @@ The new `SdkErrorCode` enum contains string-valued codes for local SDK errors: | `SdkErrorCode.ClientHttpUnexpectedContent` | Unexpected content type in HTTP response | | `SdkErrorCode.ClientHttpFailedToOpenStream` | Failed to open SSE stream | | `SdkErrorCode.ClientHttpFailedToTerminateSession` | Failed to terminate session | +| `SdkErrorCode.ClientHttpSessionExpired` | Server returned 404 for a request carrying a session ID — the session expired, start a new one | #### `StreamableHTTPError` removed @@ -760,6 +761,12 @@ try { case SdkErrorCode.ClientHttpFailedToOpenStream: console.log('Failed to open SSE stream'); break; + case SdkErrorCode.ClientHttpSessionExpired: + // Server returned 404 for a request carrying a session ID. + // The transport already cleared its session ID; reconnect to + // start a fresh session (per the MCP spec, Session Management). + console.log('Session expired — reconnecting'); + break; case SdkErrorCode.ClientHttpNotImplemented: console.log('HTTP request failed'); break; @@ -770,6 +777,22 @@ try { } ``` +#### Session expiry now surfaces as `ClientHttpSessionExpired` + +Per the MCP spec (Streamable HTTP, Session Management): when a client receives an +HTTP `404` in response to a request that carried an `Mcp-Session-Id`, the session +has expired or been terminated server-side and the client must start a new session. + +`StreamableHTTPClientTransport` now detects this by status code alone — it no longer +inspects the response body, so servers that report expiry with a non-reference body +(a different JSON-RPC error code, plain text, or HTML) are handled correctly. On such +a `404` the transport clears its stale session ID (so `transport.sessionId` becomes +`undefined` and a subsequent `client.connect(transport)` issues a fresh `initialize`) +and throws `SdkError` with `SdkErrorCode.ClientHttpSessionExpired`. + +A `404` for a request that did **not** carry a session ID (for example a wrong URL on +the initial connection) is unchanged: it still surfaces as `SdkErrorCode.ClientHttpNotImplemented`. + #### Why this change? Previously, `ErrorCode.RequestTimeout` (-32001) and `ErrorCode.ConnectionClosed` (-32000) were used for local timeout/connection errors. However, these errors never cross the wire as JSON-RPC responses - they are rejected locally. Using protocol error codes for local errors was diff --git a/packages/client/src/client/streamableHttp.ts b/packages/client/src/client/streamableHttp.ts index cd643c96dc..c5fc9bc96a 100644 --- a/packages/client/src/client/streamableHttp.ts +++ b/packages/client/src/client/streamableHttp.ts @@ -288,6 +288,17 @@ export class StreamableHTTPClientTransport implements Transport { return; } + // A 404 on the standalone GET stream while a session ID is set means the + // session expired server-side (same spec rule as the POST path in `_send`). + // Clear the dead session ID and surface the session-expired error code. + if (response.status === 404 && this._sessionId !== undefined) { + this._sessionId = undefined; + throw new SdkError(SdkErrorCode.ClientHttpSessionExpired, 'Failed to open SSE stream: session expired (HTTP 404)', { + status: 404, + statusText: response.statusText + }); + } + throw new SdkError(SdkErrorCode.ClientHttpFailedToOpenStream, `Failed to open SSE stream: ${response.statusText}`, { status: response.status, statusText: response.statusText @@ -629,6 +640,22 @@ export class StreamableHTTPClientTransport implements Transport { } } + // Per the MCP spec (Streamable HTTP, Session Management): a 404 in + // response to a request that carried an `Mcp-Session-Id` means the + // session has expired or been terminated server-side, and the client + // must start a new session. Detect this by the status code alone — + // not the response body — since non-reference servers report it with + // varying bodies (different JSON-RPC error codes, plain text, HTML). + // Clear the dead session ID so a subsequent reconnect issues a fresh + // `initialize`, and surface a distinct, body-agnostic error code. + if (response.status === 404 && this._sessionId !== undefined) { + this._sessionId = undefined; + throw new SdkError(SdkErrorCode.ClientHttpSessionExpired, `Session expired (HTTP 404): ${text}`, { + status: 404, + text + }); + } + throw new SdkError(SdkErrorCode.ClientHttpNotImplemented, `Error POSTing to endpoint: ${text}`, { status: response.status, text diff --git a/packages/client/test/client/streamableHttp.test.ts b/packages/client/test/client/streamableHttp.test.ts index b2138b3fa8..2623771b90 100644 --- a/packages/client/test/client/streamableHttp.test.ts +++ b/packages/client/test/client/streamableHttp.test.ts @@ -220,7 +220,7 @@ describe('StreamableHTTPClientTransport', () => { await expect(transport.terminateSession()).resolves.not.toThrow(); }); - it('should handle 404 response when session expires', async () => { + describe('session expiry (HTTP 404)', () => { const message: JSONRPCMessage = { jsonrpc: '2.0', method: 'test', @@ -228,24 +228,72 @@ describe('StreamableHTTPClientTransport', () => { id: 'test-id' }; - (globalThis.fetch as Mock).mockResolvedValueOnce({ - ok: false, - status: 404, - statusText: 'Not Found', - text: () => Promise.resolve('Session not found'), - headers: new Headers() - }); + // Per the MCP spec (Streamable HTTP, Session Management): a 404 in response + // to a request that carried an `Mcp-Session-Id` means the session expired and + // a new one must be started. Detection is by status code alone — non-reference + // servers report it with varying bodies, so we must not require a body shape. + it.each([ + ['plain text', 'Session not found'], + ['JSON-RPC -32002 (Figma Desktop)', '{"jsonrpc":"2.0","error":{"code":-32002,"message":"Session not found"},"id":null}'], + ['JSON-RPC -32001 (reference server)', '{"jsonrpc":"2.0","error":{"code":-32001,"message":"Session not found"},"id":null}'], + ['arbitrary HTML', '404 page not found'], + ['empty body', ''] + ])('treats 404 with a session ID as session expiry regardless of body (%s)', async (_label, body) => { + const sessionTransport = new StreamableHTTPClientTransport(new URL('http://localhost:1234/mcp'), { + sessionId: 'existing-session-id' + }); - const errorSpy = vi.fn(); - transport.onerror = errorSpy; + (globalThis.fetch as Mock).mockResolvedValueOnce({ + ok: false, + status: 404, + statusText: 'Not Found', + text: () => Promise.resolve(body), + headers: new Headers() + }); + + const errorSpy = vi.fn(); + sessionTransport.onerror = errorSpy; + + const error = await sessionTransport.send(message).then( + () => null, + e => e + ); + + expect(error).toBeInstanceOf(SdkError); + expect((error as SdkError).code).toBe(SdkErrorCode.ClientHttpSessionExpired); + expect((error as SdkError).data).toEqual({ status: 404, text: body }); + expect(errorSpy).toHaveBeenCalled(); + // The dead session ID is cleared so a subsequent reconnect issues a fresh `initialize`. + expect(sessionTransport.sessionId).toBeUndefined(); - await expect(transport.send(message)).rejects.toThrow( - new SdkError(SdkErrorCode.ClientHttpNotImplemented, 'Error POSTing to endpoint: Session not found', { + await sessionTransport.close().catch(() => {}); + }); + + it('treats a 404 without a session ID as a generic HTTP error, not session expiry', async () => { + // No session ID was ever established (e.g. a 404 on the initial connect, or a + // wrong URL). The spec rule only applies to requests carrying an Mcp-Session-Id, + // so this must remain a generic error rather than triggering a session reset. + (globalThis.fetch as Mock).mockResolvedValueOnce({ + ok: false, status: 404, - text: 'Session not found' - }) - ); - expect(errorSpy).toHaveBeenCalled(); + statusText: 'Not Found', + text: () => Promise.resolve('Not Found'), + headers: new Headers() + }); + + const errorSpy = vi.fn(); + transport.onerror = errorSpy; + + const error = await transport.send(message).then( + () => null, + e => e + ); + + expect(error).toBeInstanceOf(SdkError); + expect((error as SdkError).code).toBe(SdkErrorCode.ClientHttpNotImplemented); + expect(errorSpy).toHaveBeenCalled(); + expect(transport.sessionId).toBeUndefined(); + }); }); it('should handle non-streaming JSON response', async () => { diff --git a/packages/core/src/errors/sdkErrors.ts b/packages/core/src/errors/sdkErrors.ts index 8d5e34c14e..36a7a87f7f 100644 --- a/packages/core/src/errors/sdkErrors.ts +++ b/packages/core/src/errors/sdkErrors.ts @@ -35,7 +35,15 @@ export enum SdkErrorCode { ClientHttpForbidden = 'CLIENT_HTTP_FORBIDDEN', ClientHttpUnexpectedContent = 'CLIENT_HTTP_UNEXPECTED_CONTENT', ClientHttpFailedToOpenStream = 'CLIENT_HTTP_FAILED_TO_OPEN_STREAM', - ClientHttpFailedToTerminateSession = 'CLIENT_HTTP_FAILED_TO_TERMINATE_SESSION' + ClientHttpFailedToTerminateSession = 'CLIENT_HTTP_FAILED_TO_TERMINATE_SESSION', + /** + * Server returned HTTP 404 for a request that carried an `Mcp-Session-Id`. + * Per the MCP spec (Streamable HTTP, Session Management), this means the + * session has expired or been terminated server-side and the client must + * start a new session. The transport clears its stale session ID before + * throwing this, so reconnecting issues a fresh `initialize`. + */ + ClientHttpSessionExpired = 'CLIENT_HTTP_SESSION_EXPIRED' } /** From 6cc7726e8c74946ac3313b33477d9ba5a6c463c9 Mon Sep 17 00:00:00 2001 From: David Soria Parra Date: Fri, 22 May 2026 13:21:36 +0100 Subject: [PATCH 2/2] =?UTF-8?q?fix(client):=20address=20review=20=E2=80=94?= =?UTF-8?q?=20snapshot=20request=20session=20ID,=20handle=20404=20in=20ter?= =?UTF-8?q?minateSession?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Snapshot whether the request carried a session ID before the fetch, so the 404 session-expiry check reflects the request rather than post-response state (the response handler may write a new mcp-session-id into _sessionId). Apply the same snapshot in the GET path for symmetry. terminateSession() now treats a 404 like the existing 405 case: the session is already gone server-side, which is the caller's intent, so it clears the session ID and resolves instead of throwing ClientHttpFailedToTerminateSession. Add tests for the GET-stream 404 path (distinct error data shape) and the terminateSession 404 path. --- docs/migration.md | 5 ++ packages/client/src/client/streamableHttp.ts | 32 ++++++++--- .../client/test/client/streamableHttp.test.ts | 54 +++++++++++++++++++ 3 files changed, 83 insertions(+), 8 deletions(-) diff --git a/docs/migration.md b/docs/migration.md index 874faff184..208835842f 100644 --- a/docs/migration.md +++ b/docs/migration.md @@ -793,6 +793,11 @@ and throws `SdkError` with `SdkErrorCode.ClientHttpSessionExpired`. A `404` for a request that did **not** carry a session ID (for example a wrong URL on the initial connection) is unchanged: it still surfaces as `SdkErrorCode.ClientHttpNotImplemented`. +`terminateSession()` follows the same rule: a `404` to the `DELETE` means the session is +already gone server-side — which is what the caller asked for — so it now resolves and clears +the session ID instead of throwing `ClientHttpFailedToTerminateSession` (mirroring the existing +`405 Method Not Allowed` handling). + #### Why this change? Previously, `ErrorCode.RequestTimeout` (-32001) and `ErrorCode.ConnectionClosed` (-32000) were used for local timeout/connection errors. However, these errors never cross the wire as JSON-RPC responses - they are rejected locally. Using protocol error codes for local errors was diff --git a/packages/client/src/client/streamableHttp.ts b/packages/client/src/client/streamableHttp.ts index c5fc9bc96a..450cb787b0 100644 --- a/packages/client/src/client/streamableHttp.ts +++ b/packages/client/src/client/streamableHttp.ts @@ -246,6 +246,12 @@ export class StreamableHTTPClientTransport implements Transport { headers.set('last-event-id', resumptionToken); } + // Capture whether this request carried a session ID (see `_send` for the + // 404 session-expiry rule). The GET path does not write a response + // `mcp-session-id` back into `this._sessionId`, but snapshotting keeps the + // two branches symmetric. + const requestHadSessionId = this._sessionId !== undefined; + const response = await (this._fetch ?? fetch)(this._url, { ...this._requestInit, method: 'GET', @@ -288,10 +294,11 @@ export class StreamableHTTPClientTransport implements Transport { return; } - // A 404 on the standalone GET stream while a session ID is set means the - // session expired server-side (same spec rule as the POST path in `_send`). - // Clear the dead session ID and surface the session-expired error code. - if (response.status === 404 && this._sessionId !== undefined) { + // A 404 on the standalone GET stream for a request that carried a session + // ID means the session expired server-side (same spec rule as the POST + // path in `_send`). Clear the dead session ID and surface the + // session-expired error code. + if (response.status === 404 && requestHadSessionId) { this._sessionId = undefined; throw new SdkError(SdkErrorCode.ClientHttpSessionExpired, 'Failed to open SSE stream: session expired (HTTP 404)', { status: 404, @@ -563,6 +570,12 @@ export class StreamableHTTPClientTransport implements Transport { signal: this._abortController?.signal }; + // Capture whether *this request* carried a session ID before processing the + // response — the response handling below may write a new `mcp-session-id` + // into `this._sessionId`, and the 404 session-expiry rule is defined in terms + // of the request, not the post-response state. + const requestHadSessionId = this._sessionId !== undefined; + const response = await (this._fetch ?? fetch)(this._url, init); // Handle session ID received during initialization @@ -648,7 +661,7 @@ export class StreamableHTTPClientTransport implements Transport { // varying bodies (different JSON-RPC error codes, plain text, HTML). // Clear the dead session ID so a subsequent reconnect issues a fresh // `initialize`, and surface a distinct, body-agnostic error code. - if (response.status === 404 && this._sessionId !== undefined) { + if (response.status === 404 && requestHadSessionId) { this._sessionId = undefined; throw new SdkError(SdkErrorCode.ClientHttpSessionExpired, `Session expired (HTTP 404): ${text}`, { status: 404, @@ -749,9 +762,12 @@ export class StreamableHTTPClientTransport implements Transport { const response = await (this._fetch ?? fetch)(this._url, init); await response.text?.().catch(() => {}); - // We specifically handle 405 as a valid response according to the spec, - // meaning the server does not support explicit session termination - if (!response.ok && response.status !== 405) { + // 405 Method Not Allowed: per the spec the server does not support explicit + // session termination — treat as success. + // 404 Not Found: the session is already gone server-side, which is exactly + // what the caller asked for — treat as success rather than a failure. In both + // cases fall through to clear the local session ID. + if (!response.ok && response.status !== 405 && response.status !== 404) { throw new SdkError(SdkErrorCode.ClientHttpFailedToTerminateSession, `Failed to terminate session: ${response.statusText}`, { status: response.status, statusText: response.statusText diff --git a/packages/client/test/client/streamableHttp.test.ts b/packages/client/test/client/streamableHttp.test.ts index 2623771b90..cd246e50a4 100644 --- a/packages/client/test/client/streamableHttp.test.ts +++ b/packages/client/test/client/streamableHttp.test.ts @@ -294,6 +294,60 @@ describe('StreamableHTTPClientTransport', () => { expect(errorSpy).toHaveBeenCalled(); expect(transport.sessionId).toBeUndefined(); }); + + it('treats a 404 on the standalone GET stream as session expiry', async () => { + const sessionTransport = new StreamableHTTPClientTransport(new URL('http://localhost:1234/mcp'), { + sessionId: 'existing-session-id' + }); + + (globalThis.fetch as Mock).mockResolvedValueOnce({ + ok: false, + status: 404, + statusText: 'Not Found', + text: () => Promise.resolve('Session not found'), + headers: new Headers() + }); + + const errorSpy = vi.fn(); + sessionTransport.onerror = errorSpy; + + await sessionTransport.start(); + // Trigger the GET stream directly using the internal method for a clean test. + const error = await sessionTransport['_startOrAuthSse']({}).then( + () => null, + e => e + ); + + expect(error).toBeInstanceOf(SdkError); + expect((error as SdkError).code).toBe(SdkErrorCode.ClientHttpSessionExpired); + // The GET path carries `statusText` in its error data rather than the body `text`. + expect((error as SdkError).data).toEqual({ status: 404, statusText: 'Not Found' }); + expect(errorSpy).toHaveBeenCalled(); + expect(sessionTransport.sessionId).toBeUndefined(); + + await sessionTransport.close().catch(() => {}); + }); + + it('treats a 404 from terminateSession as already-terminated (clears session, no throw)', async () => { + const sessionTransport = new StreamableHTTPClientTransport(new URL('http://localhost:1234/mcp'), { + sessionId: 'existing-session-id' + }); + + (globalThis.fetch as Mock).mockResolvedValueOnce({ + ok: false, + status: 404, + statusText: 'Not Found', + text: () => Promise.resolve('Session not found'), + headers: new Headers() + }); + + // The session is already gone server-side — terminating it is exactly the + // caller's intent, so this must resolve rather than throw, and clear the ID. + await expect(sessionTransport.terminateSession()).resolves.toBeUndefined(); + expect(sessionTransport.sessionId).toBeUndefined(); + + await sessionTransport.close().catch(() => {}); + }); }); it('should handle non-streaming JSON response', async () => {