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..208835842f 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,27 @@ 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`. + +`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 cd643c96dc..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,6 +294,18 @@ export class StreamableHTTPClientTransport implements Transport { return; } + // 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, + statusText: response.statusText + }); + } + throw new SdkError(SdkErrorCode.ClientHttpFailedToOpenStream, `Failed to open SSE stream: ${response.statusText}`, { status: response.status, statusText: response.statusText @@ -552,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 @@ -629,6 +653,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 && requestHadSessionId) { + 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 @@ -722,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 b2138b3fa8..cd246e50a4 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,126 @@ 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' + }); + + (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 sessionTransport.close().catch(() => {}); }); - const errorSpy = vi.fn(); - transport.onerror = errorSpy; + 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, + statusText: 'Not Found', + text: () => Promise.resolve('Not Found'), + headers: new Headers() + }); - await expect(transport.send(message)).rejects.toThrow( - new SdkError(SdkErrorCode.ClientHttpNotImplemented, 'Error POSTing to endpoint: Session not found', { + 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('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, - text: 'Session not found' - }) - ); - expect(errorSpy).toHaveBeenCalled(); + 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 () => { 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' } /**