Skip to content

feat(compat): McpError/ErrorCode/JSONRPCError/StreamableHTTPError + OAuth subclass aliases - #1903

Closed
felixweinberger wants to merge 8 commits into
mainfrom
fweinberger/v2-bc-error-aliases
Closed

feat(compat): McpError/ErrorCode/JSONRPCError/StreamableHTTPError + OAuth subclass aliases#1903
felixweinberger wants to merge 8 commits into
mainfrom
fweinberger/v2-bc-error-aliases

Conversation

@felixweinberger

@felixweinbergerfelixweinberger commented Apr 15, 2026

Copy link
Copy Markdown
Contributor

Part of the v2 backwards-compatibility series — see reviewer guide.

v2 split McpError into ProtocolError/SdkError and consolidated 17 OAuth error classes into OAuthError+OAuthErrorCode. This restores the v1 names as @deprecated aliases/subclasses.

Motivation and Context

v2 split McpError into ProtocolError/SdkError and consolidated 17 OAuth error classes into OAuthError+OAuthErrorCode. This restores the v1 names as @deprecated aliases/subclasses.

v1 vs v2 pattern & evidence

v1 pattern:

`if (err instanceof McpError && err.code === ErrorCode.InvalidParams)`;`throw new InvalidTokenError('...')`

v2-native:

`if (err instanceof ProtocolError && err.code === ProtocolErrorCode.InvalidParams)`;`throw new OAuthError(OAuthErrorCode.InvalidToken, '...')`

Evidence: GitHub code search: ~1,540 files reference McpError/ErrorCode. ErrorCode.ConnectionClosed/RequestTimeout shimmed to SdkErrorCode values so comparisons work (string-vs-number assignment to error.code: number is irreducible — 1-line consumer fix).

How Has This Been Tested?

  • packages/core/test/errors/compat.test.ts — instanceof + code equivalence
  • Integration: validated bump-only against 5 OSS repos via the v2-bc-integration validation branch
  • pnpm typecheck:all && pnpm lint:all && pnpm test:all green

Breaking Changes

None — additive @deprecated shim.

Types of changes

  • New feature (non-breaking change which adds functionality)

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added or updated documentation as needed

Additional context

Stacks on: C1

@felixweinbergerfelixweinberger added the v2-bc v2 backwards-compatibility series label Apr 15, 2026
@changeset-bot

changeset-botBot commented Apr 15, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 481bf4f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages
NameType
@modelcontextprotocol/clientPatch
@modelcontextprotocol/serverPatch
@modelcontextprotocol/expressPatch
@modelcontextprotocol/fastifyPatch
@modelcontextprotocol/honoPatch
@modelcontextprotocol/nodePatch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@felixweinbergerfelixweinberger added this to the v2.0.0-bc milestone Apr 15, 2026
@pkg-pr-new

pkg-pr-newBot commented Apr 15, 2026

Copy link
Copy Markdown

Open in StackBlitz

@modelcontextprotocol/client

npm i https://pkg.pr.new/@modelcontextprotocol/client@1903

@modelcontextprotocol/server

npm i https://pkg.pr.new/@modelcontextprotocol/server@1903

@modelcontextprotocol/express

npm i https://pkg.pr.new/@modelcontextprotocol/express@1903

@modelcontextprotocol/fastify

npm i https://pkg.pr.new/@modelcontextprotocol/fastify@1903

@modelcontextprotocol/hono

npm i https://pkg.pr.new/@modelcontextprotocol/hono@1903

@modelcontextprotocol/node

npm i https://pkg.pr.new/@modelcontextprotocol/node@1903

commit: 481bf4f

@felixweinberger
felixweinbergerforce-pushed the fweinberger/v2-bc-error-aliases branch 3 times, most recently from 8a205d8 to e14ec72CompareApril 16, 2026 18:12
@felixweinberger

Copy link
Copy Markdown
ContributorAuthor

@claude review

Comment threadpackages/core/src/errors/streamableHttpErrorCompat.ts
Comment thread.changeset/error-compat-aliases.md Outdated
@felixweinberger

Copy link
Copy Markdown
ContributorAuthor

@claude review

Comment thread.changeset/error-compat-aliases.md Outdated
@felixweinberger

Copy link
Copy Markdown
ContributorAuthor

@claude review

Comment threadpackages/core/src/errors/streamableHttpErrorCompat.ts
@felixweinberger
felixweinberger marked this pull request as ready for review April 17, 2026 10:14
@felixweinberger
felixweinberger requested a review from a team as a code ownerApril 17, 2026 10:14
Comment threadpackages/core/src/errors/oauthErrorsCompat.ts
Comment thread.changeset/error-compat-aliases.md Outdated
Comment threadpackages/core/src/errors/oauthErrorsCompat.ts
@felixweinberger
felixweinberger marked this pull request as draft April 17, 2026 10:45
@felixweinberger

Copy link
Copy Markdown
ContributorAuthor

@claude review

Comment threadpackages/core/src/errors/oauthErrorsCompat.ts
@felixweinberger
felixweinbergerforce-pushed the fweinberger/v2-bc-error-aliases branch from 7cab445 to 36d4fb1CompareApril 17, 2026 12:43
@felixweinberger

Copy link
Copy Markdown
ContributorAuthor

@claude review

Comment thread.changeset/error-compat-aliases.md Outdated
@felixweinberger
felixweinberger marked this pull request as ready for review April 27, 2026 12:16
Comment threadpackages/core/src/exports/public/index.ts
Comment on lines +107 to +112
export class CustomOAuthError extends OAuthError {
static errorCode: string;
constructor(message: string, errorUri?: string) {
super((new.target as typeof CustomOAuthError).errorCode, message, errorUri);
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 User subclasses of CustomOAuthError get .name === 'OAuthError' instead of the subclass name — v1's base OAuthError constructor did this.name = this.constructor.name, so class MyErr extends CustomOAuthError { static errorCode = '…' } produced new MyErr('x').name === 'MyErr'. This is the same .name-fidelity gap already fixed for the 17 sub() classes in 473d2d8, but CustomOAuthError was missed; one-line fix: add this.name = new.target.name; after the super() call.

Extended reasoning...

What's the gap

v1's OAuthError base constructor (verified against src/server/auth/errors.ts @ 1.21.x) did this.name = this.constructor.name. So for the documented v1 pattern this file's JSDoc explicitly preserves —

classMyErrextendsCustomOAuthError{staticerrorCode='my_code'}

— v1 produced new MyErr('x').name === 'MyErr', and err.toString() / stack traces showed MyErr: x.

In this PR, v2's OAuthError constructor (packages/core/src/auth/errors.ts:108) hardcodes this.name = 'OAuthError', and CustomOAuthError's constructor (lines 109-111) only does super((new.target as typeof CustomOAuthError).errorCode, message, errorUri) without resetting .name. So new MyErr('x').name === 'OAuthError'.

Why nothing else covers it

The 17 sub()-generated classes already handle this — line 23 sets this.name = name in each subclass constructor, and 473d2d8 added Object.defineProperty(Sub, 'name', …) for constructor.name. CustomOAuthError is the one OAuth compat class that doesn't go through sub(), and its constructor was simply not given the equivalent line. Note that CustomOAuthError is a named class declaration, so MyErr.name / err.constructor.name are already correct ('MyErr') — only the instance.name property (and therefore .toString() / stack-trace label) is wrong.

The existing test in compat.test.ts ("CustomOAuthError reads static errorCode from concrete subclass") only asserts e.code and instanceof, so it doesn't catch this.

Step-by-step proof

  1. auth/errors.ts:108 — v2 OAuthError ctor: this.name = 'OAuthError' (hardcoded, not this.constructor.name).
  2. oauthErrorsCompat.ts:109-111CustomOAuthError ctor: super((new.target as …).errorCode, message, errorUri); no this.name = … afterward.
  3. User defines class MyErr extends CustomOAuthError { static override errorCode = 'my_code' }.
  4. const e = new MyErr('nope') → super chain runs step 1 → e.name === 'OAuthError'.
  5. e.toString()'OAuthError: nope'; stack trace header reads OAuthError: nope.
  6. Under v1, step 1 instead did this.name = this.constructor.namee.name === 'MyErr', e.toString()'MyErr: nope'.

Impact

Low — only affects err.name, err.toString(), and stack-trace labeling for the niche user-defined-subclass escape hatch on a @deprecated shim. instanceof MyErr, instanceof OAuthError, .code, .errorCode, and toResponseObject() all work correctly. But it's the same v1-fidelity category the author already chose to fix for sub() in 473d2d8 (resolved thread #3099495886), and this is the sibling code path that was overlooked.

Fix

exportclassCustomOAuthErrorextendsOAuthError{staticerrorCode: string;constructor(message: string,errorUri?: string){super((new.targetastypeofCustomOAuthError).errorCode,message,errorUri);this.name=new.target.name;}}

new.target already points at the user's concrete subclass (it's used on the line above for errorCode), so new.target.name gives 'MyErr' exactly as v1's this.constructor.name did. Optionally add expect(e.name).toBe('MyError') to the existing CustomOAuthError test.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v2-bcv2 backwards-compatibility series

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@felixweinberger