Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/migration-SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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:

Expand Down
28 changes: 28 additions & 0 deletions docs/migration.md
Original file line number Diff line number Diff line change
Expand Up @@ -722,6 +722,7 @@
| `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

Expand Down Expand Up @@ -760,6 +761,12 @@
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;
Expand All @@ -770,6 +777,27 @@
}
```

#### 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

Check warning on line 793 in docs/migration.md

View check run for this annotation

Claude / Claude Code Review

migration.md misstates 404-without-session error code for GET SSE path

The new sentence stating a 404 without a session ID "still surfaces as `SdkErrorCode.ClientHttpNotImplemented`" is only true for the POST path (`_send`); on the standalone GET SSE path (`_startOrAuthSse`) the same scenario falls through to `SdkErrorCode.ClientHttpFailedToOpenStream`. Consider qualifying the sentence as POST-only or listing both codes so consumers writing recovery logic from the docs handle both paths.
Comment on lines +792 to +793
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 The new sentence stating a 404 without a session ID "still surfaces as SdkErrorCode.ClientHttpNotImplemented" is only true for the POST path (_send); on the standalone GET SSE path (_startOrAuthSse) the same scenario falls through to SdkErrorCode.ClientHttpFailedToOpenStream. Consider qualifying the sentence as POST-only or listing both codes so consumers writing recovery logic from the docs handle both paths.

Extended reasoning...

What the prose claims vs. what the code does

The added paragraph under Session expiry now surfaces as ClientHttpSessionExpired states unconditionally:

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.

That sentence reads as a general claim about every 404-without-session path in StreamableHTTPClientTransport, but it is only accurate for the POST path. The transport has two places where the new 404 branch was added, and their fall-through error codes differ:

  • _send (POST): after the new if (response.status === 404 && requestHadSessionId) branch, the catch-all throws SdkError(SdkErrorCode.ClientHttpNotImplemented, ...) — matches the prose.
  • _startOrAuthSse (standalone GET SSE): after the same new 404 branch, the catch-all throws SdkError(SdkErrorCode.ClientHttpFailedToOpenStream, ...)not ClientHttpNotImplemented.

Step-by-step proof for the GET path

  1. A transport is constructed with no sessionId option, so this._sessionId is undefined.
  2. _startOrAuthSse({}) runs. The new snapshot const requestHadSessionId = this._sessionId !== undefined evaluates to false.
  3. The server replies 404 Not Found.
  4. response.ok is false, so the error block runs. The 401, 405, and the new 404 && requestHadSessionId checks are all skipped (the last one because requestHadSessionId is false).
  5. Execution reaches the fall-through: throw new SdkError(SdkErrorCode.ClientHttpFailedToOpenStream, \Failed to open SSE stream: ${response.statusText}`, ...)`.

So the GET path produces ClientHttpFailedToOpenStream, not the ClientHttpNotImplemented the prose promises.

Why it matters (and why it's only a nit)

A consumer writing reconnect/recovery logic from the migration guide could reasonably write a handler that branches on ClientHttpSessionExpired vs. ClientHttpNotImplemented to decide "session expired — reconnect" vs. "plain 404 — give up." The GET path's plain-404 case would slip through neither branch.

That said, this is a low-impact documentation imprecision rather than a code bug:

  • The prose's own parenthetical example ("a wrong URL on the initial connection") points the reader at the POST initialize path, where the claim is correct.
  • The GET-without-session-then-404 scenario is rare in practice — the standalone GET stream is normally opened post-initialize against the same URL, by which point a session ID would be set if the server uses sessions.
  • The GET path's 404-without-session error code is unchanged from before this PR; only the unqualified attribution to ClientHttpNotImplemented is new and misleading.

How to fix

Qualify the sentence or list both codes. For example:

A 404 for a request that did not carry a session ID is unchanged: it still surfaces as SdkErrorCode.ClientHttpNotImplemented on POST and SdkErrorCode.ClientHttpFailedToOpenStream on the standalone GET stream.

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
Expand Down
49 changes: 46 additions & 3 deletions packages/client/src/client/streamableHttp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,12 @@
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',
Expand Down Expand Up @@ -288,6 +294,18 @@
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
Expand Down Expand Up @@ -552,6 +570,12 @@
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
Expand Down Expand Up @@ -629,6 +653,22 @@
}
}

// 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
});
}

Comment thread
claude[bot] marked this conversation as resolved.
Comment thread
claude[bot] marked this conversation as resolved.
throw new SdkError(SdkErrorCode.ClientHttpNotImplemented, `Error POSTing to endpoint: ${text}`, {
status: response.status,
text
Expand Down Expand Up @@ -722,9 +762,12 @@
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) {

Check warning on line 770 in packages/client/src/client/streamableHttp.ts

View check run for this annotation

Claude / Claude Code Review

terminateSession() JSDoc not updated for new 404-as-success behavior

The `terminateSession()` JSDoc still only documents the 405 Method Not Allowed special case, but this PR adds a sibling success case: a 404 now also resolves and clears the session ID instead of throwing `ClientHttpFailedToTerminateSession`. Add a one-sentence note mirroring the 405 sentence so the public-facing JSDoc matches the implementation.
Comment on lines +765 to +770
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 The terminateSession() JSDoc still only documents the 405 Method Not Allowed special case, but this PR adds a sibling success case: a 404 now also resolves and clears the session ID instead of throwing ClientHttpFailedToTerminateSession. Add a one-sentence note mirroring the 405 sentence so the public-facing JSDoc matches the implementation.

Extended reasoning...

What changed vs. what the JSDoc says

This PR changes terminateSession() so that an HTTP 404 response is treated as a second success case alongside the existing 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, ... — treat as success ...
if (!response.ok && response.status !== 405 && response.status !== 404) {
    throw new SdkError(SdkErrorCode.ClientHttpFailedToTerminateSession, ...);
}
this._sessionId = undefined;

The behavior is documented in three places: the inline implementation comment, docs/migration.md ("Session expiry now surfaces as ClientHttpSessionExpired" section), and docs/migration-SKILL.md. But the public-facing JSDoc on terminateSession() — the text consumers see in editor hover tooltips and generated API docs — still reads:

The server MAY respond with HTTP 405 Method Not Allowed, indicating that the server does not allow clients to terminate sessions.

It enumerates exactly one tolerated non-OK status, which now reads as a complete list when it isn't.

Why it matters (and why it's a nit, not a blocker)

The refutation correctly notes there's no contradiction — the JSDoc never claimed 404 throws, and the 405 sentence is a near-verbatim spec quote about server behavior, not a claim about SDK error handling. So this isn't a prose-vs-implementation mismatch in the strict sense.

But the JSDoc does go out of its way to call out one specific non-OK status that is silently tolerated. By enumerating one special case, it implies a closed set. Adding a sibling special case in the implementation while leaving the doc untouched makes the doc misleadingly incomplete: a consumer reading only the hover tooltip would reasonably assume any non-405, non-OK response (including 404) throws ClientHttpFailedToTerminateSession, and might write try/catch logic for a case that no longer fires.

Step-by-step

  1. A consumer with an active session calls transport.terminateSession() while the server has already evicted the session (idle timeout, restart, etc.).
  2. The server returns 404. Pre-PR this threw ClientHttpFailedToTerminateSession; post-PR it resolves and clears _sessionId.
  3. A consumer who reads the JSDoc to understand what failure modes to handle sees only the 405 carve-out and doesn't learn that 404 is now a silent success — they'd have to dig into the migration guide or the source to discover it.

Fix

Add one sentence to the JSDoc at packages/client/src/client/streamableHttp.ts around lines 757–759, mirroring the existing 405 line:

The server may also respond with HTTP `404 Not Found` if the session has
already expired or been terminated; this is treated as successful termination.

This keeps the public-facing doc in sync with both the implementation and the migration guide. Pure documentation-completeness nit; non-blocking.

throw new SdkError(SdkErrorCode.ClientHttpFailedToTerminateSession, `Failed to terminate session: ${response.statusText}`, {
status: response.status,
statusText: response.statusText
Expand Down
132 changes: 117 additions & 15 deletions packages/client/test/client/streamableHttp.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -220,32 +220,134 @@ 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',
params: {},
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', '<html><body>404 page not found</body></html>'],
['empty body', '']
Comment thread
claude[bot] marked this conversation as resolved.
])('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 () => {
Expand Down
10 changes: 9 additions & 1 deletion packages/core/src/errors/sdkErrors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
}

/**
Expand Down
Loading