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' } /**