Skip to content

Commit dbf9b54

Browse files
docs,test: reconcile stale cross-bundle instanceof notes; fix vacuous brand-leak assertion
Now that the error classes brand-match under instanceof, several comments and doc passages still taught the pre-branding contract ("instanceof breaks across bundled copies") — soften them to the precise post-change contract: brand matching needs both copies at a brand-aware release, and data-parse recognition stays the version-agnostic path for plain wire shapes and pre-brand copies. Also replace the enumeration-leak assertion that used an asymmetric matcher inside toContain (vacuously passing) with a real check, plus an explicit non-enumerability assertion on the brand symbol.
1 parent 368cca3 commit dbf9b54

5 files changed

Lines changed: 15 additions & 10 deletions

File tree

docs/migration/upgrade-to-v2.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -984,7 +984,8 @@ peers as `-32602` — a server can no longer emit `-32002` on the wire.
984984
`ProtocolErrorCode.ResourceNotFound` (`-32002`) stays importable as
985985
receive-tolerated vocabulary — accept both `-32602` and `-32002` from peers.
986986
`ProtocolError.fromError(code, message, data)` reconstructs the typed subclass from
987-
code + data alone, so it works across bundle boundaries where `instanceof` doesn't.
987+
code + data alone — the version-agnostic path: it also works on plain wire shapes and
988+
against SDK copies that predate brand-matched `instanceof`.
988989
The default message text changed alongside: v1's unknown-resource error read
989990
`Resource <uri> not found`; v2's `ResourceNotFoundError` default is
990991
`Resource not found: <uri>` (the code is unchanged). Tests pinning the exact string

docs/servers/errors.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ Three more subclasses cover the other structured protocol errors:
179179
- `UnsupportedProtocolVersionError({ supported, requested })``-32022`; `data.supported` lets the peer pick a version and retry.
180180
- `MissingRequiredClientCapabilityError({ requiredCapabilities })``-32021`; `data.requiredCapabilities` names exactly what the client must declare.
181181

182-
Match these by `code` and `data` shape, not by `instanceof``instanceof` fails across separately bundled copies of the SDK.
182+
Match these by `code` and `data` shape when peers may run pre-brand SDK copies or hand you plain wire shapes; on brand-aware releases `instanceof` also matches across separately bundled copies of the SDK.
183183

184184
## Look up a protocol error code
185185

packages/core-internal/test/errors/crossBundleBrand.test.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,9 @@ describe('cross-bundle error instanceof (branded)', () => {
112112

113113
it('does not leak the brand through enumeration or serialization', () => {
114114
const err = new SdkHttpError(SdkErrorCode.ClientHttpForbidden, 'nope', { status: 403 });
115-
expect(Object.keys(err)).not.toContain(expect.stringContaining('mcp'));
115+
expect(Object.keys(err).some(k => k.includes('mcp'))).toBe(false);
116+
const brandKey = Symbol.for('mcp.sdk.errorBrands');
117+
expect(Object.prototype.propertyIsEnumerable.call(err, brandKey)).toBe(false);
116118
const json = JSON.stringify({ ...err });
117119
expect(json).not.toContain('mcp.Sdk');
118120
});

packages/core-internal/test/types/errorSurfacePins.test.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,9 @@
22
* Behavior-surface pins: error codes, error classes, and version constants.
33
*
44
* Consumers match SDK errors by literal numeric code, `error.name`, and message
5-
* text — not only by enum member or `instanceof` (which breaks across bundled
6-
* package boundaries). These tests pin the literal values so that a renumber,
5+
* text — not only by enum member or `instanceof` (brand-matched `instanceof`
6+
* needs both bundled copies at a brand-aware release; the literal values are
7+
* the version-agnostic contract). These tests pin the literal values so that a renumber,
78
* rename, or membership change turns CI red instead of landing silently. A
89
* failing pin here means the change is deliberate: update the pin in the same
910
* change, together with a changeset and a migration-doc entry.
@@ -92,9 +93,9 @@ describe('SdkErrorCode', () => {
9293

9394
describe('ProtocolError', () => {
9495
test('sets error.name, carries code/data, and leaves the message verbatim', () => {
95-
// Consumers classify errors via err.name (instanceof breaks when core is
96-
// bundled into both the client and server dists), and read .code/.data as
97-
// a duck shape. The constructor must not decorate the message.
96+
// Consumers classify errors via err.name (`instanceof` brand-matches
97+
// across bundles only when both copies are brand-aware), and read
98+
// .code/.data as a duck shape. The constructor must not decorate the message.
9899
const error = new ProtocolError(ProtocolErrorCode.InvalidParams, 'oops', { extra: 1 });
99100
expect(error.name).toBe('ProtocolError');
100101
expect(error.code).toBe(-32602);

packages/core-internal/test/types/missingClientCapabilityError.test.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,9 @@
22
* The `-32021` MissingRequiredClientCapability typed error.
33
*
44
* Recognition is data-parse based: a peer (or another bundled copy of the SDK)
5-
* is recognized by the error code plus the `data.requiredCapabilities` shape,
6-
* never by `instanceof` across bundles.
5+
* is recognized by the error code plus the `data.requiredCapabilities` shape —
6+
* the version-agnostic path that also covers plain wire shapes and pre-brand
7+
* SDK copies (branded `instanceof` needs both copies brand-aware).
78
*/
89
import { describe, expect, test } from 'vitest';
910

0 commit comments

Comments
 (0)