Skip to content

Commit 05d7e6e

Browse files
fix(client): propagate UnauthorizedError from the negotiation probe instead of taking the legacy fallback
The scoped-tools example caught the previous commit classifying a probe-time auth challenge as a plain HTTP 401, which routed to the conservative legacy fallback: the fallback initialize re-triggered the transport's auth flow inside the same connect() — a second authorization prompt whose PKCE verifier overwrote the first — and would settle an auth-gated modern server on the legacy era with no evidence. An auth challenge is not era evidence and is not the probe's to absorb: add an auth-required probe outcome that propagates the original UnauthorizedError unchanged, so connect() rejects with it after exactly one authorization round — the caller runs finishAuth() and reconnects, and the retry probes with the token and settles the era for real. (Older releases surfaced the same challenge wrapped as the data.cause of an SdkError(EraNegotiationFailed); callers that unwrap keep working, as the oauth example shows.) Examples e2e: 73/73 legs green, including scoped-tools http/modern.
1 parent 7b8f3ec commit 05d7e6e

5 files changed

Lines changed: 49 additions & 22 deletions

File tree

.changeset/cross-bundle-error-instanceof.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,4 @@
33
'@modelcontextprotocol/server': patch
44
---
55

6-
`instanceof` on the SDK error classes (`ProtocolError` and its typed subclasses, `SdkError`/`SdkHttpError`, `OAuthError`, and the client's `SseError`, `UnauthorizedError`, and OAuth-client-flow error family — `OAuthClientFlowError` and its subclasses) now works across separately bundled copies of the SDK. The classes match by a stable brand (via `Symbol.hasInstance` and a registry symbol) instead of prototype identity, so a process that uses both `@modelcontextprotocol/client` and `@modelcontextprotocol/server` - a gateway, host, or in-process test - can check errors constructed by either package against the class re-exported by the other. Ordinary prototype-based `instanceof` is preserved as a fallback; user-defined subclasses keep plain prototype semantics. Notes: cross-bundle matching requires both copies to be at or after this release; brands assert identity, not field shape, across versions - keep reading fields defensively. As a side effect, a foreign-bundle `SdkError` used as an abort reason is now rethrown as-is instead of being wrapped as a `RequestTimeout`. Also: `UnauthorizedError` now sets `error.name` to `'UnauthorizedError'` (previously `'Error'`), and per-package conformance tests enforce that every exported error class participates in branding. Version-negotiation probing now recognizes `UnauthorizedError` (previously a dead name-string check) and classifies an auth-gated server as an HTTP 401 outcome — taking the conservative legacy fallback instead of failing negotiation with a network error.
6+
`instanceof` on the SDK error classes (`ProtocolError` and its typed subclasses, `SdkError`/`SdkHttpError`, `OAuthError`, and the client's `SseError`, `UnauthorizedError`, and OAuth-client-flow error family — `OAuthClientFlowError` and its subclasses) now works across separately bundled copies of the SDK. The classes match by a stable brand (via `Symbol.hasInstance` and a registry symbol) instead of prototype identity, so a process that uses both `@modelcontextprotocol/client` and `@modelcontextprotocol/server` - a gateway, host, or in-process test - can check errors constructed by either package against the class re-exported by the other. Ordinary prototype-based `instanceof` is preserved as a fallback; user-defined subclasses keep plain prototype semantics. Notes: cross-bundle matching requires both copies to be at or after this release; brands assert identity, not field shape, across versions - keep reading fields defensively. As a side effect, a foreign-bundle `SdkError` used as an abort reason is now rethrown as-is instead of being wrapped as a `RequestTimeout`. Also: `UnauthorizedError` now sets `error.name` to `'UnauthorizedError'` (previously `'Error'`), and per-package conformance tests enforce that every exported error class participates in branding. Version-negotiation probing now recognizes `UnauthorizedError` (previously a dead name-string check) and propagates it unchanged, so `connect()` on an auth-gated server rejects with the original `UnauthorizedError` (previously wrapped as the `cause` of an `SdkError(EraNegotiationFailed)`) — run `finishAuth()` and reconnect, and the retry probes with the token.

examples/oauth/client.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -113,11 +113,12 @@ let challenged = false;
113113
try {
114114
await client.connect(firstTransport);
115115
} catch (error) {
116-
// Under `--legacy` the transport surfaces `UnauthorizedError` directly;
117-
// under `mode: 'auto'` the version-negotiation probe is what got 401'd
118-
// and wraps it in an EraNegotiationFailed `SdkError` whose `data.cause`
119-
// is the original `UnauthorizedError`. Either way the auth driver has
120-
// already run by the time we land here — DCR done, auth URL captured.
116+
// Both `--legacy` and `mode: 'auto'` surface the original
117+
// `UnauthorizedError` directly (the negotiation probe propagates it
118+
// unchanged; older releases wrapped it as the `data.cause` of an
119+
// EraNegotiationFailed `SdkError`, which the unwrap below still
120+
// tolerates). Either way the auth driver has already run by the time we
121+
// land here — DCR done, auth URL captured.
121122
const root = error instanceof UnauthorizedError ? error : (error as { data?: { cause?: unknown } }).data?.cause;
122123
if (!(root instanceof UnauthorizedError)) throw error;
123124
challenged = true;

packages/client/src/client/probeClassifier.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ export type ProbeOutcome =
4646
/** The HTTP layer rejected the probe POST (non-2xx); `body` is the raw response text, when available. */
4747
| { kind: 'http-error'; status: number; body?: string }
4848
| { kind: 'network-error'; error: unknown }
49+
/** The transport's auth flow challenged during the probe send (`UnauthorizedError`). */
50+
| { kind: 'auth-required'; error: Error }
4951
/** No response arrived within the probe timeout. */
5052
| { kind: 'timeout'; timeoutMs: number };
5153

@@ -114,6 +116,15 @@ export function classifyProbeOutcome(outcome: ProbeOutcome, context: ProbeClassi
114116
case 'network-error': {
115117
return classifyNetworkError(outcome.error, context);
116118
}
119+
case 'auth-required': {
120+
// Not era evidence: propagate the auth challenge unchanged so the
121+
// caller can run finishAuth() and reconnect — the reconnect probes
122+
// again with the token and settles the era with real evidence.
123+
// Converting to a legacy fallback here would re-run the auth flow
124+
// inside the same connect (a second authorization prompt) and then
125+
// handshake an auth-gated modern server as legacy.
126+
return { kind: 'error', error: outcome.error };
127+
}
117128
case 'timeout': {
118129
if (context.transportKind === 'stdio') {
119130
// Per the stdio transport's backward-compatibility rule, a probe

packages/client/src/client/versionNegotiation.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -301,11 +301,11 @@ function normalizeReply(reply: RawProbeReply, timeoutMs: number): ProbeOutcome {
301301
// skewed version, or an auth middleware's own class.
302302
(error instanceof Error && error.name === 'UnauthorizedError');
303303
if (isUnauthorized) {
304-
// Auth-gated server: not era evidence — the conservative legacy
305-
// fallback re-runs the auth flow through the plain connect path.
306-
// (The pre-branding name-string check alone could never fire
307-
// for the SDK's own class — it did not set `.name`.)
308-
return { kind: 'http-error', status: 401 };
304+
// Auth-gated server. (The pre-branding name-string check alone
305+
// could never fire for the SDK's own class — it did not set
306+
// `.name` — so these send failures fell through to the generic
307+
// network-error wrap.)
308+
return { kind: 'auth-required', error: error as Error };
309309
}
310310
return { kind: 'network-error', error };
311311
}

packages/client/test/client/versionNegotiation.test.ts

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -728,27 +728,42 @@ describe('probe send-error classification', () => {
728728
}
729729
}
730730

731-
test('UnauthorizedError from the probe send is an auth-gated server — legacy fallback on the same connection', async () => {
732-
const transport = new AuthGatedTransport(new UnauthorizedError());
731+
test('UnauthorizedError from the probe send propagates unchanged — no fallback, no second auth round (finishAuth + reconnect probes again)', async () => {
732+
const reason = new UnauthorizedError();
733+
const transport = new AuthGatedTransport(reason);
733734
const client = new Client({ name: 'c', version: '0' }, { versionNegotiation: { mode: 'auto' } });
734735

735-
await client.connect(transport);
736+
const rejection = await client.connect(transport).then(
737+
() => {
738+
throw new Error('connect unexpectedly resolved');
739+
},
740+
(e: unknown) => e
741+
);
736742

737-
expect(requests(transport.sent).some(r => r.method === 'initialize')).toBe(true);
738-
expect(client.getNegotiatedProtocolVersion()).toBe('2025-11-25');
743+
// The original error, unwrapped: callers dispatch finishAuth() on it.
744+
expect(rejection).toBe(reason);
745+
// No initialize fallback ran — that would re-trigger the transport's
746+
// auth flow (a second authorization prompt) and pin an auth-gated
747+
// modern server to the legacy era.
748+
expect(requests(transport.sent).some(r => r.method === 'initialize')).toBe(false);
739749
});
740750

741-
test("a foreign auth error matching only by name === 'UnauthorizedError' also takes the legacy fallback", async () => {
751+
test("a foreign auth error matching only by name === 'UnauthorizedError' propagates the same way", async () => {
742752
class ForeignUnauthorizedError extends Error {
743753
override readonly name = 'UnauthorizedError';
744754
}
745-
const transport = new AuthGatedTransport(new ForeignUnauthorizedError('401 from middleware'));
755+
const reason = new ForeignUnauthorizedError('401 from middleware');
756+
const transport = new AuthGatedTransport(reason);
746757
const client = new Client({ name: 'c', version: '0' }, { versionNegotiation: { mode: 'auto' } });
747758

748-
await client.connect(transport);
749-
750-
expect(requests(transport.sent).some(r => r.method === 'initialize')).toBe(true);
751-
expect(client.getNegotiatedProtocolVersion()).toBe('2025-11-25');
759+
const rejection = await client.connect(transport).then(
760+
() => {
761+
throw new Error('connect unexpectedly resolved');
762+
},
763+
(e: unknown) => e
764+
);
765+
expect(rejection).toBe(reason);
766+
expect(requests(transport.sent).some(r => r.method === 'initialize')).toBe(false);
752767
});
753768

754769
test('a plain send failure stays a typed negotiation error — no fallback runs', async () => {

0 commit comments

Comments
 (0)