SSE stream retries a 401 forever when onUnauthorized keeps resolving

[feiiiiii5]

What happens

SSEClientTransport retries the SSE connection after onUnauthorized() resolves, and the retry has no cap. If the server keeps answering the EventSource GET with 401 and the refresh keeps resolving, onUnauthorized() is called forever, the EventSource is rebuilt forever, and connect() never settles. The simple bearer provider the JSDoc recommends — { token, onUnauthorized } — is exactly that provider.

Measured on main (dd22ba25), local server always 401, provider that always resolves:

EventSource GET attempts (1.5 s) : 25
onUnauthorized() invocations     : 25
start() settled?                 : never

The 25 is a backstop I had to add to end the test. Without it the event loop is starved by the retry loop and the run times out.

The same transport's POST path does not do this:

POST attempts                : 2
onUnauthorized() invocations : 1
send() rejected with         : SdkHttpError: Server returned 401 after re-authentication

The conflict

Three parts of the repo disagree about what the second 401 should do.

The documented contract — packages/client/src/client/auth.ts:88-91 and the same text on the transport's authProvider option in packages/client/src/client/sse.ts:73-77:

Called when the server responds with 401. If provided, the transport will await this, then retry the request once. If the retry also gets 401, or if this method is not provided, the transport throws UnauthorizedError.

The other two implementations follow it: sse.ts:408 guards with !isAuthRetry and throws SdkHttpError(ClientHttpAuthentication, ...) at :425; streamableHttp.ts:538,582,599-606 has the same shape. packages/client/test/client/tokenProvider.test.ts:66,89,177 pins "retry once, then throw" for those.

Two tests in sse.test.ts pin per-attempt refresh for the SSE stream instead. :1815 records that an earlier version did pass isAuthRetry into _startOrAuth(), and that a later 401 on the retried connection then threw instead of refreshing, so the retry was changed to call _startOrAuth() fresh — that fixed token expiry on reconnect and, as a side effect, removed the cap. :1854 expects onUnauthorized twice against a server that always 401s. Neither says "retry forever is the intent"; both are regression tests for specific past bugs. But together they mean changing the documented behaviour would have to change both, which is a maintainer call rather than a bug fix.

Two ways out

A. Enforce the documented contract on the SSE stream. Thread the flag into _startOrAuth() the way streamableHttp.ts does, but reset it as soon as the retried connection opens, so a later 401 on a healthy connection still refreshes — the case the :1815 regression test protects. The :1854 test would need its expectation updated from two onUnauthorized calls to one, and the second failure becomes the SdkHttpError rather than refresh failed. This makes the three transports agree and removes the loop.

B. Keep refreshing per attempt, but bound it. Add a cap (attempts or a deadline) so a provider that can never satisfy the server fails diagnosably instead of looping. The repo already has this shape in the streamable HTTP path (sse.test.ts:1763 enforces circuit breaker on double-401), so B is the smaller behavioural change and keeps both existing tests as they are.

I lean towards A, because the interface contract, the two sibling implementations and their tests all say "once", and B still leaves a caller waiting on a connection that can never succeed. But either way the maintainers should pick rather than have a third interpretation land in a PR. I will write it for whichever is chosen, with the reproduction above turned into a test in packages/client/test/client/sse.test.ts.

Environment

@modelcontextprotocol/sdk on main (dd22ba25), Node 22, macOS, local HTTP server only.

[felixweinberger]

Fixed by #2905.

[0xamlab]

I independently reproduced the loop and verified that #2905 resolves it.

Repro snippet (local server always 401 + onUnauthorized that resolves without changing auth state):

import http from 'http';
import { SSEClientTransport } from '@modelcontextprotocol/client';

let attempts = 0, onUnauthorizedCalls = 0;
const server = http.createServer((req, res) => { attempts++; res.writeHead(401); res.end(); });
await new Promise(r => server.listen(0, '127.0.0.1', r));
const url = new URL(`http://127.0.0.1:${server.address().port}/sse`);
const transport = new SSEClientTransport(url, {
  authProvider: {
    token: async () => 'token',
    onUnauthorized: async () => { onUnauthorizedCalls++; }
  }
});
transport.start().catch(() => {});
await new Promise(r => setTimeout(r, 1500));
await transport.close(); server.close();
console.log({ attempts, onUnauthorizedCalls });

Observed

• Before fix(client): retry the SSE connection once after onUnauthorized #2905 (dd22ba25): attempts: 13873, onUnauthorizedCalls: 13873, start() never settled.
• After fix(client): retry the SSE connection once after onUnauthorized #2905 (main): attempts: 2, onUnauthorizedCalls: 1, start() rejected with SdkHttpError: Server returned 401 after re-authentication.

Mechanism

The loop is in packages/client/src/client/sse.ts inside _startOrAuth(). The EventSource fetch wrapper records a 401 in this._last401Response (~line 270), and the onerror handler recursively calls _startOrAuth() whenever onUnauthorized resolves (~line 290). Before the fix there was no retry-once flag, so a resolving but no-op callback rebuilt the SSE connection indefinitely.

Fix direction / cross-SDK consistency

#2905 adds this._connectAuthRetried, guards the refresh at sse.ts:285 with !this._connectAuthRetried, and throws SdkHttpError(ClientHttpAuthentication, ...) on the second 401. This matches the POST path and the Streamable HTTP transport; the Python SDK took the same stance for 401/403/405 after the handler declines (#3551/#3592), so aligning the TS SSE path here is good cross-SDK consistency.

Disclosure: I contribute to thyn-ai/Algenta (open-source agent tooling).