diff --git a/clients/web/src/test/core/mcp/inspectorClient-peer-handler-timing.test.ts b/clients/web/src/test/core/mcp/inspectorClient-peer-handler-timing.test.ts index c1ddd36c8..e0021b09c 100644 --- a/clients/web/src/test/core/mcp/inspectorClient-peer-handler-timing.test.ts +++ b/clients/web/src/test/core/mcp/inspectorClient-peer-handler-timing.test.ts @@ -1,4 +1,5 @@ import { describe, it, expect, vi } from "vitest"; +import { SdkErrorCode, SdkHttpError } from "@modelcontextprotocol/client"; import type { JSONRPCMessage, Root, @@ -1240,4 +1241,64 @@ describe("InspectorClient peer-handler timing (#1797)", () => { expect(silent.getRoots()).toEqual([{ uri: "file:///a" }]); await silent.disconnect(); }); + + /* + * The SDK's StreamableHTTP `_send` reports a failed POST through `onerror` + * and then rethrows it to the sender. A deterministic HTTP refusal is a + * refused request, which its rejection already surfaces, so it must not + * error the whole connection (#2776). Anything else reported here still does. + */ + describe("transport onerror (#2776)", () => { + async function connectedClient() { + const transport = new SampleAfterConnectTransport(); + const client = new InspectorClient( + { type: "stdio", command: "noop", args: [] }, + { environment: { transport: () => ({ transport }) } }, + ); + await client.connect(); + const errors: unknown[] = []; + client.addEventListener("error", (e) => errors.push(e.detail)); + return { client, transport, errors }; + } + + const postFailure = (status: number, statusText: string) => + new SdkHttpError( + SdkErrorCode.ClientHttpNotImplemented, + `Error POSTing to endpoint: ${statusText}`, + { status, statusText }, + ); + + it.each([ + [400, "Bad Request"], + [403, "Forbidden"], + [404, "Not Found"], + [405, "Method Not Allowed"], + ])( + "keeps the connection up for a POST refused with HTTP %i", + async (status, statusText) => { + const { client, transport, errors } = await connectedClient(); + transport.onerror?.(postFailure(status, statusText)); + expect(client.getStatus()).toBe("connected"); + expect(errors).toEqual([]); + await client.disconnect(); + }, + ); + + it.each([ + ["a transient POST failure (HTTP 503)", postFailure(503, "Unavailable")], + [ + "an auth failure", + new SdkHttpError(SdkErrorCode.ClientHttpAuthentication, "Forbidden", { + status: 403, + }), + ], + ["a non-HTTP transport error", new Error("stream broke")], + ])("still errors the connection for %s", async (_label, error) => { + const { client, transport, errors } = await connectedClient(); + transport.onerror?.(error); + expect(client.getStatus()).toBe("error"); + expect(errors).toEqual([error]); + await client.disconnect(); + }); + }); }); diff --git a/clients/web/src/test/integration/mcp/inspectorClient-subscriptions-era.test.ts b/clients/web/src/test/integration/mcp/inspectorClient-subscriptions-era.test.ts index 2d95734e0..4da2c92cb 100644 --- a/clients/web/src/test/integration/mcp/inspectorClient-subscriptions-era.test.ts +++ b/clients/web/src/test/integration/mcp/inspectorClient-subscriptions-era.test.ts @@ -1275,6 +1275,9 @@ describe("resource subscriptions era fork (#1630)", () => { refusal: { status: 403, statusText: "Forbidden" }, }); expect(internals(connected).modernReconnectAttempts).toBe(0); + // The Node transport reports the refused POST through `onerror` too; + // a refused request is not a failed connection (#2776). + expect(connected.getStatus()).toBe("connected"); }); it("reports the refusal even when the rollback empties the filter", async () => { @@ -1307,6 +1310,13 @@ describe("resource subscriptions era fork (#1630)", () => { status: "refused", refusal: { status: 403 }, }); + // This is the TUI's path to its refusal line: on the Node transport + // the session must survive the refused connect-time listen (#2776). + expect(connected.getStatus()).toBe("connected"); + expect(connected.getResourceSubscriptionStreamState().refusal).toEqual({ + status: 403, + statusText: "Forbidden", + }); }); it.each([400, 404, 405])( @@ -1333,10 +1343,11 @@ describe("resource subscriptions era fork (#1630)", () => { /* * The tests above drive the Node transport, which reports a failed POST - * through `onerror` as well as rejecting it — so the client is already - * `error` by the time the reconnect timer would fire, and the timer bails - * on the terminal status either way. The web client's remote transport - * rejects without `onerror`, which is the path the smoke observed the + * through `onerror` as well as rejecting it. Since #2776 that report no + * longer errors the client for a refusal, so the session stays connected + * there too; before it, the client was already `error` by the time the + * reconnect timer would fire. The web client's remote transport rejects + * without `onerror`, which is the path the smoke observed the * eight-attempt run on. These stub `listen()` with the error that * transport rebuilds (#2660) to model it: the session stays connected, * and only the classification stands between the refusal and the retry. diff --git a/core/mcp/inspectorClient.ts b/core/mcp/inspectorClient.ts index 5c918cd46..a202a748f 100644 --- a/core/mcp/inspectorClient.ts +++ b/core/mcp/inspectorClient.ts @@ -1345,6 +1345,16 @@ export class InspectorClient extends InspectorClientEventTarget { // from any non-"connecting" state captures the reason regardless of // ordering. if (this.status === "connecting") return; + // A deterministic HTTP refusal of a POST is a refused REQUEST, not a + // failed connection (#2776). The SDK's StreamableHTTP `_send` reports it + // here and then rethrows it to the sender, so the request's rejection + // already surfaces it, and #2722's listen refusal settles its own stream + // status from that rejection. Flipping the client to "error" here made + // that status unreachable on the Node transport. The classifier matches + // only the code the SDK throws from its POST path, so a failure with no + // other surface (an SSE GET stream, a dropped connection, a 5xx) and + // every auth failure still land below. + if (subscriptionListenRefusal(error) !== undefined) return; this.status = "error"; this.dispatchTypedEvent("statusChange", this.status); this.dispatchTypedEvent("error", error);