Skip to content

Commit 9ba03d8

Browse files
fix(core): make instanceof on SDK error classes work across bundled copies
@modelcontextprotocol/client and @modelcontextprotocol/server each bundle their own copy of the runtime internals, so an error constructed by one package failed prototype-identity instanceof against the same class re-exported by the other - exactly the check a dual-role process (gateway, host, in-process test) writes. ProtocolError and its typed subclasses, SdkError/SdkHttpError, and OAuthError now stamp instances with the brand strings of their class chain under a registry symbol (Symbol.for, shared across bundles) and resolve instanceof via Symbol.hasInstance against the brand set, with ordinary prototype instanceof as the fallback. User-defined subclasses that do not declare a brand keep plain prototype semantics, so a foreign base-class instance never satisfies instanceof UserSubclass.
1 parent 79dc162 commit 9ba03d8

7 files changed

Lines changed: 251 additions & 6 deletions

File tree

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
'@modelcontextprotocol/client': patch
3+
'@modelcontextprotocol/server': patch
4+
---
5+
6+
`instanceof` on the SDK error classes (`ProtocolError` and its typed subclasses, `SdkError`/`SdkHttpError`, `OAuthError`) 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.

docs/migration/upgrade-to-v2.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -823,6 +823,10 @@ The SDK now distinguishes three error kinds:
823823
3. **`SdkHttpError`** (extends `SdkError`) — HTTP transport errors with typed `.status`
824824
and `.statusText`.
825825
826+
These classes (and `OAuthError`) brand-match under `instanceof`, so checks work across
827+
separately bundled copies of the SDK — e.g. a process using both
828+
`@modelcontextprotocol/client` and `@modelcontextprotocol/server`.
829+
826830
The codemod renames `McpError` → `ProtocolError`, `ErrorCode` → `ProtocolErrorCode`
827831
(routing `RequestTimeout` / `ConnectionClosed` to `SdkErrorCode`), and
828832
`StreamableHTTPError` → `SdkHttpError`. After the codemod runs, your `instanceof`

packages/core-internal/src/auth/errors.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { brandedHasInstance, stampErrorBrands } from '../errors/crossBundleBrand';
12
import type { OAuthErrorResponse } from '../shared/auth';
23

34
/**
@@ -103,13 +104,20 @@ export enum OAuthErrorCode {
103104
* OAuth error class for all OAuth-related errors.
104105
*/
105106
export class OAuthError extends Error {
107+
protected static readonly mcpBrand: string = 'mcp.OAuthError';
108+
109+
static override [Symbol.hasInstance](value: unknown): boolean {
110+
return brandedHasInstance(this, value);
111+
}
112+
106113
constructor(
107114
public readonly code: OAuthErrorCode | string,
108115
message: string,
109116
public readonly errorUri?: string
110117
) {
111118
super(message);
112119
this.name = 'OAuthError';
120+
stampErrorBrands(this, new.target);
113121
}
114122

115123
/**
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
/**
2+
* Cross-bundle `instanceof` support for the SDK error classes.
3+
*
4+
* `@modelcontextprotocol/client` and `@modelcontextprotocol/server` each bundle their
5+
* own copy of `core-internal`, so an error constructed by one package fails a
6+
* prototype-identity `instanceof` against the same class re-exported by the other —
7+
* exactly the check a dual-role process (gateway, host, in-process test) writes.
8+
*
9+
* Instead of prototype identity, branded classes stamp every instance with the brand
10+
* strings of its class chain under a registry symbol (`Symbol.for`, shared across
11+
* bundles and realms), and resolve `instanceof` via `Symbol.hasInstance` against the
12+
* brand set. Ordinary prototype-based `instanceof` is kept as a fallback so behavior
13+
* is unchanged for anything unbranded.
14+
*
15+
* A class participates by declaring an **own** `static readonly mcpBrand` and (for
16+
* hierarchy roots) installing {@linkcode brandedHasInstance} as `Symbol.hasInstance`.
17+
* User-defined subclasses that do not declare their own brand keep plain prototype
18+
* semantics — a foreign base-class instance never satisfies `instanceof UserSubclass`.
19+
*/
20+
21+
/** Registry symbol — identical across bundled copies and realms. */
22+
const BRANDS: unique symbol = Symbol.for('mcp.sdk.errorBrands') as never;
23+
24+
interface BrandCarrier {
25+
[BRANDS]?: { has(brand: string): boolean };
26+
}
27+
28+
interface BrandedConstructor {
29+
mcpBrand?: string;
30+
}
31+
32+
/**
33+
* Stamp `instance` with the brand of every class in `ctor`'s chain that declares an
34+
* own `mcpBrand`. Call once from the hierarchy root's constructor with `new.target` —
35+
* subclasses inherit the stamping without touching their constructors.
36+
*/
37+
export function stampErrorBrands(instance: object, ctor: unknown): void {
38+
const brands = new Set<string>();
39+
let current: unknown = ctor;
40+
while (typeof current === 'function') {
41+
const brand = (current as BrandedConstructor).mcpBrand;
42+
if (Object.prototype.hasOwnProperty.call(current, 'mcpBrand') && typeof brand === 'string') {
43+
brands.add(brand);
44+
}
45+
current = Object.getPrototypeOf(current);
46+
}
47+
if (brands.size === 0) return;
48+
Object.defineProperty(instance, BRANDS, { value: brands, enumerable: false, configurable: true });
49+
}
50+
51+
/**
52+
* `Symbol.hasInstance` implementation for branded hierarchy roots. Matches when the
53+
* value carries the **own** brand of the class being tested against (cross-bundle
54+
* path), falling back to ordinary prototype-based `instanceof` otherwise.
55+
*/
56+
export function brandedHasInstance(cls: object, value: unknown): boolean {
57+
if (
58+
typeof value === 'object' &&
59+
value !== null &&
60+
Object.prototype.hasOwnProperty.call(cls, 'mcpBrand') &&
61+
typeof (cls as BrandedConstructor).mcpBrand === 'string'
62+
) {
63+
const carried = (value as BrandCarrier)[BRANDS];
64+
if (carried && typeof carried.has === 'function' && carried.has((cls as BrandedConstructor).mcpBrand!)) {
65+
return true;
66+
}
67+
}
68+
return Function.prototype[Symbol.hasInstance].call(cls, value);
69+
}

packages/core-internal/src/errors/sdkErrors.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import { brandedHasInstance, stampErrorBrands } from './crossBundleBrand';
2+
13
/**
24
* Error codes for SDK errors (local errors that never cross the wire).
35
* Unlike {@linkcode ProtocolErrorCode} which uses numeric JSON-RPC codes, `SdkErrorCode` uses
@@ -98,13 +100,20 @@ export enum SdkErrorCode {
98100
* ```
99101
*/
100102
export class SdkError extends Error {
103+
protected static readonly mcpBrand: string = 'mcp.SdkError';
104+
105+
static override [Symbol.hasInstance](value: unknown): boolean {
106+
return brandedHasInstance(this, value);
107+
}
108+
101109
constructor(
102110
public readonly code: SdkErrorCode,
103111
message: string,
104112
public readonly data?: unknown
105113
) {
106114
super(message);
107115
this.name = 'SdkError';
116+
stampErrorBrands(this, new.target);
108117
}
109118
}
110119

@@ -134,6 +143,8 @@ export interface SdkHttpErrorData {
134143
* ```
135144
*/
136145
export class SdkHttpError extends SdkError {
146+
protected static override readonly mcpBrand: string = 'mcp.SdkHttpError';
147+
137148
declare readonly data: SdkHttpErrorData;
138149

139150
constructor(code: SdkErrorCode, message: string, data: SdkHttpErrorData) {

packages/core-internal/src/types/errors.ts

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { brandedHasInstance, stampErrorBrands } from '../errors/crossBundleBrand';
12
import { ProtocolErrorCode } from './enums';
23
import type {
34
ClientCapabilities,
@@ -9,15 +10,27 @@ import type {
910
/**
1011
* Protocol errors are JSON-RPC errors that cross the wire as error responses.
1112
* They use numeric error codes from the {@linkcode ProtocolErrorCode} enum.
13+
*
14+
* `instanceof` on this class (and its subclasses) is brand-matched, so it works
15+
* across separately bundled copies of the SDK — e.g. an error constructed by
16+
* `@modelcontextprotocol/client` matches the class re-exported by
17+
* `@modelcontextprotocol/server` in the same process.
1218
*/
1319
export class ProtocolError extends Error {
20+
protected static readonly mcpBrand: string = 'mcp.ProtocolError';
21+
22+
static override [Symbol.hasInstance](value: unknown): boolean {
23+
return brandedHasInstance(this, value);
24+
}
25+
1426
constructor(
1527
public readonly code: number,
1628
message: string,
1729
public readonly data?: unknown
1830
) {
1931
super(message);
2032
this.name = 'ProtocolError';
33+
stampErrorBrands(this, new.target);
2134
}
2235

2336
/**
@@ -86,11 +99,12 @@ export class ProtocolError extends Error {
8699
* accept `-32002` as resource not found — earlier SDK builds emitted that
87100
* code, and {@linkcode ProtocolError.fromError} reconstructs this class for
88101
* either code **when `error.data` carries `uri`** (a bare `-32002` without
89-
* `data.uri` stays a generic {@linkcode ProtocolError}). Do not rely on
90-
* `instanceof` — it does not work across separately bundled copies of the
91-
* SDK.
102+
* `data.uri` stays a generic {@linkcode ProtocolError}). `instanceof` checks
103+
* are brand-matched and work across separately bundled copies of the SDK.
92104
*/
93105
export class ResourceNotFoundError extends ProtocolError {
106+
protected static override readonly mcpBrand: string = 'mcp.ResourceNotFoundError';
107+
94108
constructor(uri: string, message: string = `Resource not found: ${uri}`) {
95109
super(ProtocolErrorCode.InvalidParams, message, { uri });
96110
}
@@ -106,6 +120,8 @@ export class ResourceNotFoundError extends ProtocolError {
106120
* This makes it nicer for the client to handle since there is specific data to work with instead of just a code to check against.
107121
*/
108122
export class UrlElicitationRequiredError extends ProtocolError {
123+
protected static override readonly mcpBrand: string = 'mcp.UrlElicitationRequiredError';
124+
109125
constructor(elicitations: ElicitRequestURLParams[], message: string = `URL elicitation${elicitations.length > 1 ? 's' : ''} required`) {
110126
super(ProtocolErrorCode.UrlElicitationRequired, message, {
111127
elicitations: elicitations
@@ -127,6 +143,8 @@ export class UrlElicitationRequiredError extends ProtocolError {
127143
* version that was requested (`requested`).
128144
*/
129145
export class UnsupportedProtocolVersionError extends ProtocolError {
146+
protected static override readonly mcpBrand: string = 'mcp.UnsupportedProtocolVersionError';
147+
130148
constructor(data: UnsupportedProtocolVersionErrorData, message: string = `Unsupported protocol version: ${data.requested}`) {
131149
super(ProtocolErrorCode.UnsupportedProtocolVersion, message, data);
132150
}
@@ -156,11 +174,13 @@ export class UnsupportedProtocolVersionError extends ProtocolError {
156174
* have to declare for the request to be served. On HTTP, the response status
157175
* is `400 Bad Request`.
158176
*
159-
* Recognize this error by its code and `data.requiredCapabilities` rather than
160-
* by class identity (`instanceof` does not work across separately bundled
161-
* copies of the SDK).
177+
* Recognize this error by its code and `data.requiredCapabilities`, or by
178+
* `instanceof` — checks are brand-matched and work across separately bundled
179+
* copies of the SDK.
162180
*/
163181
export class MissingRequiredClientCapabilityError extends ProtocolError {
182+
protected static override readonly mcpBrand: string = 'mcp.MissingRequiredClientCapabilityError';
183+
164184
constructor(
165185
data: MissingRequiredClientCapabilityErrorData,
166186
message: string = `Missing required client capabilities: ${Object.keys(data.requiredCapabilities).join(', ')}`
Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
import { describe, expect, it, vi } from 'vitest';
2+
3+
import { OAuthError, OAuthErrorCode } from '../../src/auth/errors';
4+
import { SdkError, SdkErrorCode, SdkHttpError } from '../../src/errors/sdkErrors';
5+
import { ProtocolErrorCode } from '../../src/types/enums';
6+
import {
7+
MissingRequiredClientCapabilityError,
8+
ProtocolError,
9+
ResourceNotFoundError,
10+
UnsupportedProtocolVersionError,
11+
UrlElicitationRequiredError
12+
} from '../../src/types/errors';
13+
14+
// `@modelcontextprotocol/client` and `@modelcontextprotocol/server` each bundle their
15+
// own copy of core-internal; `vi.resetModules()` + a dynamic import reproduces that
16+
// dual-copy situation faithfully (separate class objects, shared registry symbol).
17+
async function loadForeignCopies() {
18+
vi.resetModules();
19+
const sdkErrors = await import('../../src/errors/sdkErrors');
20+
const protocolErrors = await import('../../src/types/errors');
21+
const authErrors = await import('../../src/auth/errors');
22+
return { sdkErrors, protocolErrors, authErrors };
23+
}
24+
25+
describe('cross-bundle error instanceof (branded)', () => {
26+
it('same-bundle instanceof keeps working for every class', () => {
27+
const http = new SdkHttpError(SdkErrorCode.ClientHttpNotImplemented, 'boom', { status: 404 });
28+
expect(http instanceof SdkHttpError).toBe(true);
29+
expect(http instanceof SdkError).toBe(true);
30+
expect(http instanceof Error).toBe(true);
31+
32+
const proto = new ProtocolError(ProtocolErrorCode.InvalidParams, 'bad');
33+
expect(proto instanceof ProtocolError).toBe(true);
34+
35+
const rnf = new ResourceNotFoundError('file:///missing');
36+
expect(rnf instanceof ResourceNotFoundError).toBe(true);
37+
expect(rnf instanceof ProtocolError).toBe(true);
38+
39+
const oauth = new OAuthError(OAuthErrorCode.InvalidToken, 'nope');
40+
expect(oauth instanceof OAuthError).toBe(true);
41+
});
42+
43+
it('hierarchies stay disjoint', () => {
44+
const sdk = new SdkError(SdkErrorCode.NotConnected, 'x');
45+
const proto = new ProtocolError(ProtocolErrorCode.InvalidParams, 'x');
46+
expect(sdk instanceof ProtocolError).toBe(false);
47+
expect(proto instanceof SdkError).toBe(false);
48+
expect(proto instanceof OAuthError).toBe(false);
49+
50+
const rnf = new ResourceNotFoundError('file:///missing');
51+
expect(rnf instanceof UrlElicitationRequiredError).toBe(false);
52+
expect(rnf instanceof UnsupportedProtocolVersionError).toBe(false);
53+
expect(rnf instanceof MissingRequiredClientCapabilityError).toBe(false);
54+
});
55+
56+
it('an instance from a second module copy satisfies instanceof against this copy (and vice versa)', async () => {
57+
const { sdkErrors, protocolErrors, authErrors } = await loadForeignCopies();
58+
59+
// Sanity: these really are different class objects.
60+
expect(sdkErrors.SdkHttpError).not.toBe(SdkHttpError);
61+
expect(protocolErrors.ProtocolError).not.toBe(ProtocolError);
62+
63+
const foreignHttp = new sdkErrors.SdkHttpError(sdkErrors.SdkErrorCode.ClientHttpAuthentication, '401', { status: 401 });
64+
expect(foreignHttp instanceof SdkHttpError).toBe(true);
65+
expect(foreignHttp instanceof SdkError).toBe(true);
66+
expect(foreignHttp instanceof ProtocolError).toBe(false);
67+
68+
const localHttp = new SdkHttpError(SdkErrorCode.ClientHttpAuthentication, '401', { status: 401 });
69+
expect(localHttp instanceof sdkErrors.SdkHttpError).toBe(true);
70+
expect(localHttp instanceof sdkErrors.SdkError).toBe(true);
71+
72+
const foreignRnf = new protocolErrors.ResourceNotFoundError('file:///missing');
73+
expect(foreignRnf instanceof ResourceNotFoundError).toBe(true);
74+
expect(foreignRnf instanceof ProtocolError).toBe(true);
75+
expect(foreignRnf instanceof UrlElicitationRequiredError).toBe(false);
76+
77+
const foreignPlainProto = new protocolErrors.ProtocolError(ProtocolErrorCode.InvalidParams, 'x');
78+
expect(foreignPlainProto instanceof ProtocolError).toBe(true);
79+
expect(foreignPlainProto instanceof ResourceNotFoundError).toBe(false);
80+
81+
const foreignOauth = new authErrors.OAuthError(authErrors.OAuthErrorCode.InvalidGrant, 'x');
82+
expect(foreignOauth instanceof OAuthError).toBe(true);
83+
expect(foreignOauth instanceof SdkError).toBe(false);
84+
});
85+
86+
it('ProtocolError.fromError from a second copy reconstructs instances this copy recognizes', async () => {
87+
const { protocolErrors } = await loadForeignCopies();
88+
const reconstructed = protocolErrors.ProtocolError.fromError(ProtocolErrorCode.InvalidParams, 'gone', {
89+
uri: 'file:///missing'
90+
});
91+
expect(reconstructed instanceof ResourceNotFoundError).toBe(true);
92+
expect(reconstructed instanceof ProtocolError).toBe(true);
93+
});
94+
95+
it('user-defined subclasses keep plain prototype semantics (no cross-class false positives)', () => {
96+
class MyError extends ProtocolError {}
97+
const mine = new MyError(ProtocolErrorCode.InternalError, 'mine');
98+
expect(mine instanceof MyError).toBe(true);
99+
expect(mine instanceof ProtocolError).toBe(true);
100+
101+
// A base-class instance — same or foreign bundle — must never satisfy the subclass.
102+
const base = new ProtocolError(ProtocolErrorCode.InternalError, 'base');
103+
expect(base instanceof MyError).toBe(false);
104+
});
105+
106+
it('a foreign base instance does not satisfy a user-defined subclass of this copy', async () => {
107+
const { protocolErrors } = await loadForeignCopies();
108+
class MyError extends ProtocolError {}
109+
const foreignBase = new protocolErrors.ProtocolError(ProtocolErrorCode.InternalError, 'x');
110+
expect(foreignBase instanceof MyError).toBe(false);
111+
});
112+
113+
it('does not leak the brand through enumeration or serialization', () => {
114+
const err = new SdkHttpError(SdkErrorCode.ClientHttpForbidden, 'nope', { status: 403 });
115+
expect(Object.keys(err)).not.toContain(expect.stringContaining('mcp'));
116+
const json = JSON.stringify({ ...err });
117+
expect(json).not.toContain('mcp.Sdk');
118+
});
119+
120+
it('tolerates primitives, null, and brandless objects', () => {
121+
for (const value of [null, undefined, 42, 'x', Symbol('s'), {}, new Error('plain')]) {
122+
expect((value as unknown) instanceof SdkError).toBe(false);
123+
expect((value as unknown) instanceof ProtocolError).toBe(false);
124+
expect((value as unknown) instanceof OAuthError).toBe(false);
125+
}
126+
});
127+
});

0 commit comments

Comments
 (0)