From 7fbb90747809609f56157380fad9f87aff902301 Mon Sep 17 00:00:00 2001 From: rajanpanth Date: Thu, 6 Aug 2026 16:49:54 +0545 Subject: [PATCH] fix(client): fall back to legacy when the probe answer is an unusable 2xx MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The version-negotiation probe handled HTTP rejections and HTTP successes asymmetrically. A non-2xx answer an older server produces reaches the unparseable-4xx row and degrades to the legacy initialize handshake, but a 2xx whose body is unusable — empty or whitespace JSON, a bare 204, a media type the transport does not accept — surfaced from the transport as a plain SyntaxError or ClientHttpUnexpectedContent, fell into the network-error row, and rejected connect() with EraNegotiationFailed. An intermediary that swallows the unrecognized server/discover POST into an empty 2xx therefore bricked a connection that a plain initialize would have completed, while one that rejected it 4xx degraded gracefully. Give that shape its own probe outcome and classify it like the unparseable 4xx: conservative legacy fallback when one is available, typed EraNegotiationFailed carrying the transport failure as cause otherwise. Genuine network failures keep the network-error row (including its browser-CORS legacy case), and the auth and 5xx rows are untouched. Fixes #2619 --- .../probe-unusable-2xx-reply-falls-back.md | 18 +++++ docs/troubleshooting.md | 1 + packages/client/src/client/probeClassifier.ts | 33 +++++++++ .../client/src/client/versionNegotiation.ts | 32 +++++++++ .../test/client/probeClassifier.test.ts | 38 ++++++++++ .../test/client/versionNegotiation.test.ts | 70 +++++++++++++++++++ 6 files changed, 192 insertions(+) create mode 100644 .changeset/probe-unusable-2xx-reply-falls-back.md diff --git a/.changeset/probe-unusable-2xx-reply-falls-back.md b/.changeset/probe-unusable-2xx-reply-falls-back.md new file mode 100644 index 0000000000..9e8859f223 --- /dev/null +++ b/.changeset/probe-unusable-2xx-reply-falls-back.md @@ -0,0 +1,18 @@ +--- +'@modelcontextprotocol/client': patch +--- + +The version-negotiation probe no longer fails the connect when an intermediary +swallows the `server/discover` POST into an unusable 2xx. An empty or +whitespace JSON body, a bare `204`, or a media type the transport does not +accept previously rejected `connect()` with `EraNegotiationFailed`, while the +same endpoint answering `400` degraded gracefully to the legacy `initialize` +handshake — so a reverse proxy or API gateway in front of a working 2025 server +bricked the connection. + +An HTTP layer that succeeded with an answer the client cannot use is now its +own probe outcome, classified like the unparseable-4xx row: a conservative +legacy fallback when one is available, and a typed `EraNegotiationFailed` +carrying the transport's failure as `cause` for a modern-only client or `pin` +mode. Genuine network failures (DNS, connection reset, CORS) keep their typed +error, and the auth and 5xx rows are unchanged. diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index d9e6e7824d..25c78dcd37 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -75,6 +75,7 @@ With the global in place the [client OAuth](./clients/oauth.md) flows run unchan - `the connection closed during the server/discover probe (this transport probed in place — the disposable sibling probe requires the SDK's base StdioClientTransport)` — a subclass of `StdioClientTransport`, or a custom stdio-shaped transport, probed in place and met a server that exits on any pre-`initialize` request: use the base `StdioClientTransport` (which probes on a disposable sibling), or `mode: 'legacy'`. - `the transport was closed during the server/discover probe` — the caller closed the transport while the probe was in flight; the connect aborted deliberately and the session child was never spawned. - `Version negotiation probe failed: ...` — the probe hit a transport failure (network outage, HTTP connection drop): fix connectivity and retry. +- `the server answered with an unusable reply (...)` — the HTTP layer accepted the probe but the answer could not be used (an empty or whitespace 2xx body, a bare `204`, an unaccepted media type — typically an intermediary swallowing the unrecognized POST). With a legacy fallback available this is not an error at all: the client falls back to `initialize`, and you only see this code from a modern-only client or `pin` mode. - `the server answered the probe with HTTP 5xx` — the server or a proxy in front of it failed (mid-deploy, crashed backend); not era evidence, so no legacy fallback is attempted: retry once the deployment is healthy. A `401`/`403` probe rejection is **not** this code — see the next section. diff --git a/packages/client/src/client/probeClassifier.ts b/packages/client/src/client/probeClassifier.ts index e23d011a2a..a8f764e8d7 100644 --- a/packages/client/src/client/probeClassifier.ts +++ b/packages/client/src/client/probeClassifier.ts @@ -51,6 +51,12 @@ export type ProbeOutcome = /** The HTTP layer rejected the probe POST (non-2xx); `body` is the raw response text and `statusText` the HTTP reason phrase, when available. */ | { kind: 'http-error'; status: number; body?: string; statusText?: string } | { kind: 'network-error'; error: unknown } + /** + * The HTTP layer answered the probe 2xx, but the answer was unusable — an + * empty or unparseable JSON body, or a media type the transport does not + * accept. `error` is the transport's own failure, propagated as the cause. + */ + | { kind: 'unusable-reply'; error: unknown } /** The transport's auth flow challenged or failed during the probe send — an error stamped at a transport auth seam, or an `UnauthorizedError` (the foreign-transport contract). `error` propagates unchanged. */ | { kind: 'auth-required'; error: Error } /** The transport reported close while the probe awaited its reply. */ @@ -139,6 +145,33 @@ export function classifyProbeOutcome(outcome: ProbeOutcome, context: ProbeClassi case 'network-error': { return classifyNetworkError(outcome.error, context); } + case 'unusable-reply': { + // The HTTP layer succeeded and the answer was unusable — an empty + // 2xx, a whitespace body, a bare 204, a media type the transport + // does not accept. That is the same evidence as the unparseable + // 4xx row: the endpoint did not answer `server/discover`, so the + // conservative reading is a server that does not speak the discover + // protocol (an intermediary swallowing the unrecognized POST into + // an empty 2xx is exactly this shape). Fall back rather than brick + // a connection a plain `initialize` would have completed. + // + // Not folded into network-error: a genuine network failure (DNS, + // connection reset, CORS) never reached a server, and that row must + // keep its typed error outside the browser-CORS case. + if (context.fallbackAvailable) { + return { kind: 'legacy' }; + } + // Modern-only client or `pin` mode: no `initialize` to fall back + // to, so the unusable answer is a typed negotiation failure. + return { + kind: 'error', + error: new SdkError( + SdkErrorCode.EraNegotiationFailed, + `Version negotiation probe failed: the server answered with an unusable reply (${describeError(outcome.error)})`, + { cause: outcome.error } + ) + }; + } case 'auth-required': { // Not era evidence: propagate the auth challenge unchanged so the // caller can run finishAuth() and reconnect — the reconnect probes diff --git a/packages/client/src/client/versionNegotiation.ts b/packages/client/src/client/versionNegotiation.ts index 9b89d962ba..aa11208481 100644 --- a/packages/client/src/client/versionNegotiation.ts +++ b/packages/client/src/client/versionNegotiation.ts @@ -375,6 +375,30 @@ export function buildProbeRequest( }; } +/** + * Recognizes the failures a transport raises *after* the HTTP layer answered + * the probe 2xx, when the answer itself is unusable: + * + * - `ClientHttpUnexpectedContent` — a media type the transport does not accept + * (a bare 204, `text/plain`, a missing `content-type`). + * - a JSON `SyntaxError` — an empty or whitespace body parsed as + * `application/json`. + * + * Both mean the endpoint accepted the POST without answering `server/discover`, + * which is era evidence of the same strength as an unparseable 4xx. Auth + * escapes and `SdkHttpError` (a non-2xx rejection) are matched by earlier rows, + * so they never reach this check. + */ +function isUnusableReplyError(error: unknown): boolean { + if (error instanceof SdkError && error.code === SdkErrorCode.ClientHttpUnexpectedContent) { + return true; + } + // Name-based, not `instanceof`: the body parse can reject with a + // SyntaxError from another realm (an injected `fetch`, a differently + // bundled copy). + return error instanceof Error && error.name === 'SyntaxError'; +} + function normalizeReply(reply: RawProbeReply, timeoutMs: number): ProbeOutcome { switch (reply.kind) { case 'response': { @@ -409,6 +433,14 @@ function normalizeReply(reply: RawProbeReply, timeoutMs: number): ProbeOutcome { statusText: error.data.statusText }; } + if (isUnusableReplyError(error)) { + // The HTTP layer answered 2xx and the body was unusable. Kept + // distinct from network-error so the classifier can apply the + // unparseable-4xx reading (conservative legacy fallback) + // instead of failing a connection a plain `initialize` would + // have completed. + return { kind: 'unusable-reply', error }; + } return { kind: 'network-error', error }; } case 'closed': { diff --git a/packages/client/test/client/probeClassifier.test.ts b/packages/client/test/client/probeClassifier.test.ts index 3a65f240b7..6d19863dd7 100644 --- a/packages/client/test/client/probeClassifier.test.ts +++ b/packages/client/test/client/probeClassifier.test.ts @@ -327,3 +327,41 @@ describe('row: browser opaque CORS/preflight TypeError, PROBE PHASE ONLY → leg expect(verdict.kind).toBe('error'); }); }); + +describe('row: unusable-reply — the HTTP layer answered 2xx with an unusable body', () => { + // An intermediary (reverse proxy, API gateway) that swallows the + // unrecognized `server/discover` POST into an empty 2xx is at least as + // strong evidence of "this endpoint does not speak discover" as the + // unparseable 4xx a deployed 2025 server answers, so it takes the same + // conservative legacy fallback instead of bricking the connection. + const jsonSyntaxError = () => { + try { + JSON.parse(''); + throw new Error('unreachable'); + } catch (error) { + return error; + } + }; + + test('an empty JSON body falls back to legacy', () => { + expect(classify({ kind: 'unusable-reply', error: jsonSyntaxError() })).toEqual({ kind: 'legacy' }); + }); + + test('an unaccepted media type (bare 204, text/plain) falls back to legacy', () => { + const error = new SdkError(SdkErrorCode.ClientHttpUnexpectedContent, 'Unexpected content type: text/plain', { + contentType: 'text/plain' + }); + expect(classify({ kind: 'unusable-reply', error })).toEqual({ kind: 'legacy' }); + }); + + test('without a fallback the unusable answer is a typed negotiation error carrying the cause', () => { + const cause = jsonSyntaxError(); + const verdict = classify({ kind: 'unusable-reply', error: cause }, { fallbackAvailable: false }); + expect(verdict.kind).toBe('error'); + if (verdict.kind === 'error') { + expect(verdict.error).toBeInstanceOf(SdkError); + expect((verdict.error as SdkError).code).toBe(SdkErrorCode.EraNegotiationFailed); + expect((verdict.error as SdkError).data).toMatchObject({ cause }); + } + }); +}); diff --git a/packages/client/test/client/versionNegotiation.test.ts b/packages/client/test/client/versionNegotiation.test.ts index 60c9a2aaba..5bc08c2a94 100644 --- a/packages/client/test/client/versionNegotiation.test.ts +++ b/packages/client/test/client/versionNegotiation.test.ts @@ -1390,3 +1390,73 @@ describe('probe window preserves pre-set transport handlers', () => { await client.close(); }); }); + +/* --------------------------------------------------------------------------- + * Probe answers the HTTP layer accepted but could not use: an intermediary + * that swallows the unrecognized `server/discover` POST into an empty 2xx is + * the same evidence as the unparseable 4xx a deployed 2025 server answers, so + * it takes the legacy fallback instead of failing the connect. + * ------------------------------------------------------------------------- */ + +describe('probe unusable-reply classification', () => { + /** Rejects the probe send with `probeError`, then serves legacy initialize. */ + class UnusableReplyTransport extends ScriptedTransport { + constructor(private readonly probeError: Error) { + super(legacyServerScript); + } + + override async send(message: JSONRPCMessage): Promise { + if (isJSONRPCRequest(message) && message.method === 'server/discover') { + throw this.probeError; + } + await super.send(message); + } + } + + const unusableReplies: Array<[string, () => Error]> = [ + [ + 'an empty 2xx body parsed as application/json (a gateway swallowing the probe)', + () => { + try { + JSON.parse(''); + throw new Error('unreachable'); + } catch (error) { + return error as Error; + } + } + ], + [ + 'a bare 204 / text-plain answer the transport does not accept', + () => + new SdkError(SdkErrorCode.ClientHttpUnexpectedContent, 'Unexpected content type: text/plain', { + contentType: 'text/plain' + }) + ] + ]; + + test.each(unusableReplies)('%s falls back to the legacy initialize handshake', async (_label, makeError) => { + const transport = new UnusableReplyTransport(makeError()); + const client = new Client({ name: 'c', version: '0' }, { versionNegotiation: { mode: 'auto' } }); + + await client.connect(transport); + + expect(requests(transport.sent).some(r => r.method === 'initialize')).toBe(true); + await client.close(); + }); + + test('a genuine network failure is still a typed connect error (not folded into the fallback)', async () => { + const transport = new UnusableReplyTransport(new TypeError('fetch failed')); + const client = new Client({ name: 'c', version: '0' }, { versionNegotiation: { mode: 'auto' } }); + + const rejection = await client.connect(transport).then( + () => { + throw new Error('connect unexpectedly resolved'); + }, + (e: unknown) => e + ); + + expect(rejection).toBeInstanceOf(SdkError); + expect((rejection as SdkError).code).toBe(SdkErrorCode.EraNegotiationFailed); + expect(requests(transport.sent).some(r => r.method === 'initialize')).toBe(false); + }); +});