Skip to content

feat(errors): standardize MCP JSON-RPC codes into -32020..-32099 range - #223

Open
JayDS22 wants to merge 1 commit into
accordproject:mainfrom
JayDS22:jay/feat/mcp-error-code-range-32020
Open

feat(errors): standardize MCP JSON-RPC codes into -32020..-32099 range#223
JayDS22 wants to merge 1 commit into
accordproject:mainfrom
JayDS22:jay/feat/mcp-error-code-range-32020

Conversation

@JayDS22

@JayDS22 JayDS22 commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

Summary

The MCP spec reserves -32020..-32099 for implementation-defined error codes. Previously the ServiceError -> McpError mapping in serviceErrorToResourceError was a coarse 2-way split (404 -> InvalidParams, else -> InternalError), losing per-subclass identity.

This threads a jsonRpcCode: number through the ServiceError hierarchy so each subclass carries its own MCP code:

Subclass Code
TemplateNotFoundError -32020
AgreementNotFoundError -32021
TemplateDuplicateError -32022
AgreementConversionError -32023
AgreementTriggerError -32024
InvalidPayloadError -32025
ValidationError -32026
UpstreamApiError -32027
ServiceError (bare default) -32028

MCP clients can now branch on the integer code without parsing message strings; the string code field (e.g. TEMPLATE_NOT_FOUND) stays as the machine-readable form for REST + MCP consumers.

What this PR does

  • Adds MCP_ERROR_CODES central constant table in services/errors.ts, with tests asserting all values sit inside the reserved range and no two subclasses collide on the same code.
  • Threads jsonRpcCode through every ServiceError subclass constructor.
  • Updates serviceErrorToResourceError and serviceErrorToCallToolResult in handlers/mcp.ts to emit the typed code instead of the coarse 2-way split.
  • Extends services/errors.test.ts with per-subclass code assertions + range-invariant tests.

Stack

Base of a three-PR chain that closes Proposal Core and aligns MCP with the 2026-07-28 spec finalisation:

  1. This PR - JSON-RPC error range standardisation
  2. Subscriptions/listen SEP-2575 upstream port (adds SubscriptionInvalidUriError -32029 in this range)
  3. Slice 3 REST list unification (closes Proposal Core service-layer port)

Merge earliest first; later branches will rebase forward.

Validation

  • npm ci in server/: clean
  • npm run build: clean (tsc -p .)
  • npm test: 10 suites / 153 tests, all pass (was 141 pre-branch, +12 from errors.test.ts)

Related

Sets the JSON-RPC code convention that Issue #221 ("MCP SDK upgrade") builds on for the 2.0-beta.5 migration.

Author Checklist

  • DCO sign-off on every commit
  • Branch rebased on latest main
  • npm test green locally
  • No em-dashes in commit message or PR body

@niallroche

Copy link
Copy Markdown
Contributor

This is a clear improvement over the coarse 404→InvalidParams / else→InternalError mapping — typed, specific codes are the right direction, and -32020..-32099 sits correctly inside JSON-RPC's -32000..-32099 "reserved for implementation-defined server errors" band, so the range is spec-legal. The collision/range tests in errors.test.ts are a nice touch for keeping it that way.

Three things worth a look before merge:

1. This is a wire-visible change — call it out. Any MCP client currently branching on the numeric code will see different values: a bare ServiceError that used to surface as -32603 (InternalError) now comes back as -32028. That's fine for a pre-1.0 MCP surface, but it deserves a line in the PR body / changelog so downstream client authors aren't surprised.

2. Confirm the fallback for non-ServiceError throws. I can see MCP_ERROR_CODES.SERVICE_ERROR = -32028 as the fallback for a bare ServiceError, but I couldn't see serviceErrorToResourceError / serviceErrorToCallToolResult in errors.ts — they're in mcp.ts. Can you confirm that something thrown that isn't a ServiceError at all (a raw Error, a Concerto exception) still maps to -32603 InternalError, and doesn't fall through to -32028? -32028 should mean "a ServiceError we didn't specialize," not "anything unexpected," otherwise you lose the "this was genuinely internal" signal.

3. Standard codes where they fit? INVALID_PAYLOAD (-32025) and VALIDATION_ERROR (-32026) map conceptually onto JSON-RPC's predefined -32602 Invalid params. Using custom codes uniformly is a defensible choice (one namespace, more specific), but a generic JSON-RPC client won't recognize -32025 as "bad params" the way it would -32602. Worth a sentence on the rationale — I'd lean toward keeping the custom codes for specificity, just documenting that they carry the same data.code string so clients have a stable non-numeric key.

Note: for UpstreamApiError (-32027), consider carrying the upstream HTTP status in error.data — a 502 from an upstream is diagnostically different from a 404, and the numeric JSON-RPC code alone flattens that.

The MCP spec reserves `-32020..-32099` for implementation-defined error
codes. Previously the ServiceError -> McpError mapping in
`serviceErrorToResourceError` was a coarse 2-way split (404 ->
InvalidParams, else -> InternalError), losing per-subclass identity.

This threads a `jsonRpcCode: number` through the `ServiceError`
hierarchy so each subclass carries its own MCP code:

  TEMPLATE_NOT_FOUND          -> -32020
  AGREEMENT_NOT_FOUND         -> -32021
  TEMPLATE_DUPLICATE          -> -32022
  AGREEMENT_CONVERSION_FAILED -> -32023
  AGREEMENT_TRIGGER_FAILED    -> -32024
  INVALID_PAYLOAD             -> -32025
  VALIDATION_ERROR            -> -32026
  UPSTREAM_API_ERROR          -> -32027
  (bare ServiceError default) -> -32028

MCP clients can now branch on the integer code without parsing message
strings; the string `code` field (e.g. `TEMPLATE_NOT_FOUND`) stays as
the machine-readable form for REST + MCP consumers.

Central codes live in `MCP_ERROR_CODES` (`services/errors.ts`), with
tests asserting all values sit inside the reserved range and that no two
subclasses collide on the same code.

Validation:
- npm run build: clean
- npm test: 10 suites / 153 tests, all pass

Signed-off-by: Jay Guwalani <guwalanijj@gmail.com>
@JayDS22
JayDS22 force-pushed the jay/feat/mcp-error-code-range-32020 branch from fb63e7b to 990704f Compare July 31, 2026 06:32
@JayDS22

JayDS22 commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator Author

Amended 990704f addressing the three review notes:

1. Wire-visibility change flagged. Confirmed: any MCP client that was branching on the numeric code from a bare ServiceError will see -32028 instead of the old -32603. Subclassed errors (which is everything the RI actually throws today) already carried typed strings via error.code, so the code string field stays a stable non-numeric key across the change. Called this out here so downstream client authors have a heads-up.

2. Non-ServiceError fallback confirmed. The -32028 code applies ONLY to callers that explicitly construct a bare ServiceError. Raw Error, Concerto exceptions, and every other throw path still bubble unwrapped through resource handlers (getAgreement, getTemplates, etc. all guard with if (err instanceof ServiceError) before wrapping), so the SDK default handler maps them to -32603 InternalError as before. Made this contract explicit in the serviceErrorToResourceError JSDoc (new "Invariant" paragraph in mcp.ts) so a future editor can't accidentally widen the wrapper.

3. UpstreamApiError carries HTTP status in error.data. Already threaded through: UpstreamApiError constructor passes { upstreamUrl, httpStatus, upstreamBody } into details, ServiceError.toJSON() spreads details into the serialized payload, and serviceErrorToResourceError writes error.toJSON() into the McpError.data field. Added a targeted test in mcp.test.ts ('UpstreamApiError surfaces httpStatus in error.data.details for diagnostic parity with the REST 502') so this contract is pinned against regression.

On the INVALID_PAYLOAD / VALIDATION_ERROR vs -32602 observation: keeping the custom -32025 / -32026 codes for specificity per your lean. The data.error.code string (INVALID_PAYLOAD / VALIDATION_ERROR) is the stable non-numeric key clients can branch on if they want the generic "bad params" bucket.

npm test: 10 suites / 154 tests, all pass.

@github-actions github-actions Bot added the maintainer-engaged A maintainer has commented or reviewed this item label Aug 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintainer-engaged A maintainer has commented or reviewed this item

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants