fix: call onerror callback for all error responses in StreamableHTTPServerTransport

[kdn-posipaka]

Summary

Fixes#1395

Several error paths in StreamableHTTPServerTransport returned JSON error responses via createJsonErrorResponse() without calling the onerror callback, silently swallowing transport errors.

Changes

packages/server/src/server/streamableHttp.ts:

• Added this.onerror?.(new Error(message)) inside createJsonErrorResponse() so every error response automatically triggers the callback
• Removed 4 redundant manual this.onerror?.() calls that preceded createJsonErrorResponse() to avoid double-firing

This ensures all error types are observable — including parse errors, invalid headers, session validation failures, and protocol version mismatches — which were previously invisible to users.

packages/server/test/server/streamableHttp.test.ts:

• Added 5 new tests verifying onerror is called for: invalid Accept header (POST & GET), invalid Content-Type, invalid JSON, and invalid JSON-RPC messages

Test Results

All 42 tests pass (37 existing + 5 new). TypeScript compiles cleanly.

[changeset-bot]

🦋 Changeset detected

Latest commit: 5fcc3b1

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

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

[pkg-pr-new]

Open in StackBlitz

@modelcontextprotocol/client

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/client@1687

@modelcontextprotocol/server

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/server@1687

@modelcontextprotocol/express

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/express@1687

@modelcontextprotocol/hono

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/hono@1687

@modelcontextprotocol/node

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/node@1687

commit: 5fcc3b1

[travisbreaks]

Good change overall. Centralizing onerror in createJsonErrorResponse() is the right structural call: it guarantees coverage for every path that uses that helper, including validateSession, validateProtocolVersion, and the various 4xx checks that were previously silent. The test coverage is solid.

A few things worth flagging:

1. handleUnsupportedRequest() bypasses the centralized path

handleUnsupportedRequest() (the 405 handler for PUT/PATCH/etc.) builds its own Response.json(...) directly instead of calling createJsonErrorResponse(). After this PR, that error path still won't fire onerror. It may be intentional (unsupported HTTP methods aren't really transport errors), but if the goal is "every error response triggers the callback," this is a gap. Worth either converting it to use createJsonErrorResponse() or documenting the exception.

2. Lost error fidelity in catch blocks

The two catch blocks that previously called onerror with the actual caught error (error as Error) now get new Error(message) where message is a generic string like "Error replaying events" or "Parse error". The real exception (with its original message, type, and stack trace) is discarded from the callback. For the handlePostRequest outer catch, the original error still appears as data in the HTTP response body, but anyone relying on onerror for logging/observability loses the underlying cause.

Consider preserving the original error when one is available. For example, the replayEvents catch could pass the caught error to onerror and still call createJsonErrorResponse separately (accepting the "double call" in that one path), or createJsonErrorResponse could accept an optional cause parameter:

privatecreateJsonErrorResponse(status: number,code: number,message: string,options?: {headers?: Record<string,string>;data?: string;cause?: Error}): Response{this.onerror?.(options?.cause??newError(message));// ...}

3. Overlap with #1684

PR #1684 fixes a subset of the same issue (the two inner parse catch blocks in handlePostRequest). That PR takes a different approach: it adds targeted onerror calls at each catch site and preserves the actual caught error object. If this PR lands first, #1684 becomes a no-op for those paths. If #1684 lands first, this PR would need a rebase and could potentially double-fire on those two catch blocks. The maintainers should pick one direction and close the other.

Between the two, this PR has better architectural coverage (all createJsonErrorResponse callers get onerror for free going forward). But #1684 preserves error fidelity. The ideal outcome would be this PR's centralized approach combined with #1684's pattern of forwarding the real error object.

Nice tests. The changeset is included, which #1684 is missing.

[kdn-posipaka]

Thanks for the thorough review, @travisbreaks! Great catches — I've pushed a commit addressing your feedback:

1. handleUnsupportedRequest() now uses createJsonErrorResponse() — fully consistent with the centralized path. The Allow header is passed via the headers option.

2. Error fidelity preserved via cause parameter — Added an optional cause?: Error field to the options. The onerror callback now receives options?.cause ?? new Error(message), so catch blocks that have the original error can pass it through (e.g., the POST parse error catch block now passes the caught error as cause).

3. Overlap with fix(server): surface parse/validation transport errors via onerror in streamable HTTP #1684 — This PR supersedes fix(server): surface parse/validation transport errors via onerror in streamable HTTP #1684's scope (centralizing onerror in createJsonErrorResponse covers what fix(server): surface parse/validation transport errors via onerror in streamable HTTP #1684 aimed to fix). This one should land first; fix(server): surface parse/validation transport errors via onerror in streamable HTTP #1684 can then be closed or rebased if needed.

All tests pass (43/43). Let me know if anything else needs adjustment!

[Maverick-666]

Thanks @travisbreaks and @kai-agent-free — great progress on this.

I closed #1684 to consolidate on this PR and avoid parallel fixes for #1395. I’m happy to actively help push this over the line.

If useful, I can immediately contribute one focused follow-up commit that:

1. adds/expands regression tests for the remaining centralized error paths,
2. verifies original error fidelity (cause) is preserved end-to-end in onerror,
3. updates any related docs/changelog bits if maintainers want that included here.

If you’re good with that scope, I can open the follow-up against this branch today.

[kdn-posipaka]

Thanks @Maverick-666! Appreciate closing #1684 to consolidate — makes sense to avoid parallel fixes.

Your follow-up commit proposal sounds good. The regression test coverage for the new cause parameter + handleUnsupportedRequest path would strengthen the PR. Feel free to push to the branch if you'd like, or I can incorporate specific test cases you have in mind.

The handlePostRequest outer catch already passes the caught error as cause now, so the fidelity issue travisbreaks flagged should be resolved. Happy to iterate on anything else.

[Maverick-666]

Thanks! I can take this.

I’ll add focused regression coverage for:

1. handleUnsupportedRequest path triggers onerror
2. centralized error-response path preserves error fidelity via cause
3. parse-failure paths still surface meaningful original error context to onerror

I’ll keep it test-only and minimal, then share the commit/PR link here.

[Maverick-666]

Quick update: I implemented the test follow-up, but I don’t have push permission to kai-agent-free/typescript-sdk (got 403 when pushing to the PR head branch).

I pushed the changes to my fork instead:

• Branch: Maverick-666:codex/pr1687-onerror-tests
• Commit: 65e3de3 (test(server): add regression coverage for onerror unsupported-method and cause fidelity)

Local verification:

• pnpm --filter @modelcontextprotocol/server test -- test/server/streamableHttp.test.ts (pass)

If convenient, you can cherry-pick this commit:

git fetch https://github.com/Maverick-666/typescript-sdk.git codex/pr1687-onerror-tests
git cherry-pick 65e3de3

[kdn-posipaka]

Thanks @Maverick-666 for the additional test coverage! 🙏 I've cherry-picked your commit (65e3de3) into this branch — the unsupported-method onerror regression test and cause fidelity check are great additions. All 385 tests passing.

[Maverick-666]

Awesome, thanks for cherry-picking and confirming all tests pass.

Glad this helped strengthen the PR. Happy to help with any additional follow-ups if needed.

[felixweinberger]

Thanks for the contribution! Closing in favor of #1433.

[kdn-posipaka]

Summary

Fixes#1395

Several error paths in StreamableHTTPServerTransport returned JSON error responses via createJsonErrorResponse() without calling the onerror callback, silently swallowing transport errors.

Changes

packages/server/src/server/streamableHttp.ts:

• Added this.onerror?.(new Error(message)) inside createJsonErrorResponse() so every error response automatically triggers the callback
• Removed 4 redundant manual this.onerror?.() calls that preceded createJsonErrorResponse() to avoid double-firing

This ensures all error types are observable — including parse errors, invalid headers, session validation failures, and protocol version mismatches — which were previously invisible to users.

packages/server/test/server/streamableHttp.test.ts:

• Added 5 new tests verifying onerror is called for: invalid Accept header (POST & GET), invalid Content-Type, invalid JSON, and invalid JSON-RPC messages

Test Results

All 42 tests pass (37 existing + 5 new). TypeScript compiles cleanly.

[changeset-bot]

🦋 Changeset detected

Latest commit: 5fcc3b1

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

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

[pkg-pr-new]

Open in StackBlitz

@modelcontextprotocol/client

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/client@1687

@modelcontextprotocol/server

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/server@1687

@modelcontextprotocol/express

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/express@1687

@modelcontextprotocol/hono

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/hono@1687

@modelcontextprotocol/node

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/node@1687

commit: 5fcc3b1

[travisbreaks]

Good change overall. Centralizing onerror in createJsonErrorResponse() is the right structural call: it guarantees coverage for every path that uses that helper, including validateSession, validateProtocolVersion, and the various 4xx checks that were previously silent. The test coverage is solid.

A few things worth flagging:

1. handleUnsupportedRequest() bypasses the centralized path

handleUnsupportedRequest() (the 405 handler for PUT/PATCH/etc.) builds its own Response.json(...) directly instead of calling createJsonErrorResponse(). After this PR, that error path still won't fire onerror. It may be intentional (unsupported HTTP methods aren't really transport errors), but if the goal is "every error response triggers the callback," this is a gap. Worth either converting it to use createJsonErrorResponse() or documenting the exception.

2. Lost error fidelity in catch blocks

The two catch blocks that previously called onerror with the actual caught error (error as Error) now get new Error(message) where message is a generic string like "Error replaying events" or "Parse error". The real exception (with its original message, type, and stack trace) is discarded from the callback. For the handlePostRequest outer catch, the original error still appears as data in the HTTP response body, but anyone relying on onerror for logging/observability loses the underlying cause.

Consider preserving the original error when one is available. For example, the replayEvents catch could pass the caught error to onerror and still call createJsonErrorResponse separately (accepting the "double call" in that one path), or createJsonErrorResponse could accept an optional cause parameter:

privatecreateJsonErrorResponse(status: number,code: number,message: string,options?: {headers?: Record<string,string>;data?: string;cause?: Error}): Response{this.onerror?.(options?.cause??newError(message));// ...}

3. Overlap with #1684

PR #1684 fixes a subset of the same issue (the two inner parse catch blocks in handlePostRequest). That PR takes a different approach: it adds targeted onerror calls at each catch site and preserves the actual caught error object. If this PR lands first, #1684 becomes a no-op for those paths. If #1684 lands first, this PR would need a rebase and could potentially double-fire on those two catch blocks. The maintainers should pick one direction and close the other.

Between the two, this PR has better architectural coverage (all createJsonErrorResponse callers get onerror for free going forward). But #1684 preserves error fidelity. The ideal outcome would be this PR's centralized approach combined with #1684's pattern of forwarding the real error object.

Nice tests. The changeset is included, which #1684 is missing.

[kdn-posipaka]

Thanks for the thorough review, @travisbreaks! Great catches — I've pushed a commit addressing your feedback:

1. handleUnsupportedRequest() now uses createJsonErrorResponse() — fully consistent with the centralized path. The Allow header is passed via the headers option.

2. Error fidelity preserved via cause parameter — Added an optional cause?: Error field to the options. The onerror callback now receives options?.cause ?? new Error(message), so catch blocks that have the original error can pass it through (e.g., the POST parse error catch block now passes the caught error as cause).

3. Overlap with fix(server): surface parse/validation transport errors via onerror in streamable HTTP #1684 — This PR supersedes fix(server): surface parse/validation transport errors via onerror in streamable HTTP #1684's scope (centralizing onerror in createJsonErrorResponse covers what fix(server): surface parse/validation transport errors via onerror in streamable HTTP #1684 aimed to fix). This one should land first; fix(server): surface parse/validation transport errors via onerror in streamable HTTP #1684 can then be closed or rebased if needed.

All tests pass (43/43). Let me know if anything else needs adjustment!

[Maverick-666]

Thanks @travisbreaks and @kai-agent-free — great progress on this.

I closed #1684 to consolidate on this PR and avoid parallel fixes for #1395. I’m happy to actively help push this over the line.

If useful, I can immediately contribute one focused follow-up commit that:

1. adds/expands regression tests for the remaining centralized error paths,
2. verifies original error fidelity (cause) is preserved end-to-end in onerror,
3. updates any related docs/changelog bits if maintainers want that included here.

If you’re good with that scope, I can open the follow-up against this branch today.

[kdn-posipaka]

Thanks @Maverick-666! Appreciate closing #1684 to consolidate — makes sense to avoid parallel fixes.

Your follow-up commit proposal sounds good. The regression test coverage for the new cause parameter + handleUnsupportedRequest path would strengthen the PR. Feel free to push to the branch if you'd like, or I can incorporate specific test cases you have in mind.

The handlePostRequest outer catch already passes the caught error as cause now, so the fidelity issue travisbreaks flagged should be resolved. Happy to iterate on anything else.

[Maverick-666]

Thanks! I can take this.

I’ll add focused regression coverage for:

1. handleUnsupportedRequest path triggers onerror
2. centralized error-response path preserves error fidelity via cause
3. parse-failure paths still surface meaningful original error context to onerror

I’ll keep it test-only and minimal, then share the commit/PR link here.

[Maverick-666]

Quick update: I implemented the test follow-up, but I don’t have push permission to kai-agent-free/typescript-sdk (got 403 when pushing to the PR head branch).

I pushed the changes to my fork instead:

• Branch: Maverick-666:codex/pr1687-onerror-tests
• Commit: 65e3de3 (test(server): add regression coverage for onerror unsupported-method and cause fidelity)

Local verification:

• pnpm --filter @modelcontextprotocol/server test -- test/server/streamableHttp.test.ts (pass)

If convenient, you can cherry-pick this commit:

git fetch https://github.com/Maverick-666/typescript-sdk.git codex/pr1687-onerror-tests
git cherry-pick 65e3de3

[kdn-posipaka]

Thanks @Maverick-666 for the additional test coverage! 🙏 I've cherry-picked your commit (65e3de3) into this branch — the unsupported-method onerror regression test and cause fidelity check are great additions. All 385 tests passing.

[Maverick-666]

Awesome, thanks for cherry-picking and confirming all tests pass.

Glad this helped strengthen the PR. Happy to help with any additional follow-ups if needed.

[felixweinberger]

Thanks for the contribution! Closing in favor of #1433.

[kdn-posipaka]

Summary

Fixes#1395

Several error paths in StreamableHTTPServerTransport returned JSON error responses via createJsonErrorResponse() without calling the onerror callback, silently swallowing transport errors.

Changes

packages/server/src/server/streamableHttp.ts:

• Added this.onerror?.(new Error(message)) inside createJsonErrorResponse() so every error response automatically triggers the callback
• Removed 4 redundant manual this.onerror?.() calls that preceded createJsonErrorResponse() to avoid double-firing

This ensures all error types are observable — including parse errors, invalid headers, session validation failures, and protocol version mismatches — which were previously invisible to users.

packages/server/test/server/streamableHttp.test.ts:

• Added 5 new tests verifying onerror is called for: invalid Accept header (POST & GET), invalid Content-Type, invalid JSON, and invalid JSON-RPC messages

Test Results

All 42 tests pass (37 existing + 5 new). TypeScript compiles cleanly.

[changeset-bot]

🦋 Changeset detected

Latest commit: 5fcc3b1

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

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

[pkg-pr-new]

Open in StackBlitz

@modelcontextprotocol/client

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/client@1687

@modelcontextprotocol/server

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/server@1687

@modelcontextprotocol/express

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/express@1687

@modelcontextprotocol/hono

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/hono@1687

@modelcontextprotocol/node

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/node@1687

commit: 5fcc3b1

[travisbreaks]

Good change overall. Centralizing onerror in createJsonErrorResponse() is the right structural call: it guarantees coverage for every path that uses that helper, including validateSession, validateProtocolVersion, and the various 4xx checks that were previously silent. The test coverage is solid.

A few things worth flagging:

1. handleUnsupportedRequest() bypasses the centralized path

handleUnsupportedRequest() (the 405 handler for PUT/PATCH/etc.) builds its own Response.json(...) directly instead of calling createJsonErrorResponse(). After this PR, that error path still won't fire onerror. It may be intentional (unsupported HTTP methods aren't really transport errors), but if the goal is "every error response triggers the callback," this is a gap. Worth either converting it to use createJsonErrorResponse() or documenting the exception.

2. Lost error fidelity in catch blocks

The two catch blocks that previously called onerror with the actual caught error (error as Error) now get new Error(message) where message is a generic string like "Error replaying events" or "Parse error". The real exception (with its original message, type, and stack trace) is discarded from the callback. For the handlePostRequest outer catch, the original error still appears as data in the HTTP response body, but anyone relying on onerror for logging/observability loses the underlying cause.

Consider preserving the original error when one is available. For example, the replayEvents catch could pass the caught error to onerror and still call createJsonErrorResponse separately (accepting the "double call" in that one path), or createJsonErrorResponse could accept an optional cause parameter:

privatecreateJsonErrorResponse(status: number,code: number,message: string,options?: {headers?: Record<string,string>;data?: string;cause?: Error}): Response{this.onerror?.(options?.cause??newError(message));// ...}

3. Overlap with #1684

PR #1684 fixes a subset of the same issue (the two inner parse catch blocks in handlePostRequest). That PR takes a different approach: it adds targeted onerror calls at each catch site and preserves the actual caught error object. If this PR lands first, #1684 becomes a no-op for those paths. If #1684 lands first, this PR would need a rebase and could potentially double-fire on those two catch blocks. The maintainers should pick one direction and close the other.

Between the two, this PR has better architectural coverage (all createJsonErrorResponse callers get onerror for free going forward). But #1684 preserves error fidelity. The ideal outcome would be this PR's centralized approach combined with #1684's pattern of forwarding the real error object.

Nice tests. The changeset is included, which #1684 is missing.

[kdn-posipaka]

Thanks for the thorough review, @travisbreaks! Great catches — I've pushed a commit addressing your feedback:

1. handleUnsupportedRequest() now uses createJsonErrorResponse() — fully consistent with the centralized path. The Allow header is passed via the headers option.

2. Error fidelity preserved via cause parameter — Added an optional cause?: Error field to the options. The onerror callback now receives options?.cause ?? new Error(message), so catch blocks that have the original error can pass it through (e.g., the POST parse error catch block now passes the caught error as cause).

3. Overlap with fix(server): surface parse/validation transport errors via onerror in streamable HTTP #1684 — This PR supersedes fix(server): surface parse/validation transport errors via onerror in streamable HTTP #1684's scope (centralizing onerror in createJsonErrorResponse covers what fix(server): surface parse/validation transport errors via onerror in streamable HTTP #1684 aimed to fix). This one should land first; fix(server): surface parse/validation transport errors via onerror in streamable HTTP #1684 can then be closed or rebased if needed.

All tests pass (43/43). Let me know if anything else needs adjustment!

[Maverick-666]

Thanks @travisbreaks and @kai-agent-free — great progress on this.

I closed #1684 to consolidate on this PR and avoid parallel fixes for #1395. I’m happy to actively help push this over the line.

If useful, I can immediately contribute one focused follow-up commit that:

1. adds/expands regression tests for the remaining centralized error paths,
2. verifies original error fidelity (cause) is preserved end-to-end in onerror,
3. updates any related docs/changelog bits if maintainers want that included here.

If you’re good with that scope, I can open the follow-up against this branch today.

[kdn-posipaka]

Thanks @Maverick-666! Appreciate closing #1684 to consolidate — makes sense to avoid parallel fixes.

Your follow-up commit proposal sounds good. The regression test coverage for the new cause parameter + handleUnsupportedRequest path would strengthen the PR. Feel free to push to the branch if you'd like, or I can incorporate specific test cases you have in mind.

The handlePostRequest outer catch already passes the caught error as cause now, so the fidelity issue travisbreaks flagged should be resolved. Happy to iterate on anything else.

[Maverick-666]

Thanks! I can take this.

I’ll add focused regression coverage for:

1. handleUnsupportedRequest path triggers onerror
2. centralized error-response path preserves error fidelity via cause
3. parse-failure paths still surface meaningful original error context to onerror

I’ll keep it test-only and minimal, then share the commit/PR link here.

[Maverick-666]

Quick update: I implemented the test follow-up, but I don’t have push permission to kai-agent-free/typescript-sdk (got 403 when pushing to the PR head branch).

I pushed the changes to my fork instead:

• Branch: Maverick-666:codex/pr1687-onerror-tests
• Commit: 65e3de3 (test(server): add regression coverage for onerror unsupported-method and cause fidelity)

Local verification:

• pnpm --filter @modelcontextprotocol/server test -- test/server/streamableHttp.test.ts (pass)

If convenient, you can cherry-pick this commit:

git fetch https://github.com/Maverick-666/typescript-sdk.git codex/pr1687-onerror-tests
git cherry-pick 65e3de3

[kdn-posipaka]

Thanks @Maverick-666 for the additional test coverage! 🙏 I've cherry-picked your commit (65e3de3) into this branch — the unsupported-method onerror regression test and cause fidelity check are great additions. All 385 tests passing.

[Maverick-666]

Awesome, thanks for cherry-picking and confirming all tests pass.

Glad this helped strengthen the PR. Happy to help with any additional follow-ups if needed.

[felixweinberger]

Thanks for the contribution! Closing in favor of #1433.

[kdn-posipaka]

Summary

Fixes#1395

Several error paths in StreamableHTTPServerTransport returned JSON error responses via createJsonErrorResponse() without calling the onerror callback, silently swallowing transport errors.

Changes

packages/server/src/server/streamableHttp.ts:

• Added this.onerror?.(new Error(message)) inside createJsonErrorResponse() so every error response automatically triggers the callback
• Removed 4 redundant manual this.onerror?.() calls that preceded createJsonErrorResponse() to avoid double-firing

This ensures all error types are observable — including parse errors, invalid headers, session validation failures, and protocol version mismatches — which were previously invisible to users.

packages/server/test/server/streamableHttp.test.ts:

• Added 5 new tests verifying onerror is called for: invalid Accept header (POST & GET), invalid Content-Type, invalid JSON, and invalid JSON-RPC messages

Test Results

All 42 tests pass (37 existing + 5 new). TypeScript compiles cleanly.

[changeset-bot]

🦋 Changeset detected

Latest commit: 5fcc3b1

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

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

[pkg-pr-new]

Open in StackBlitz

@modelcontextprotocol/client

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/client@1687

@modelcontextprotocol/server

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/server@1687

@modelcontextprotocol/express

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/express@1687

@modelcontextprotocol/hono

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/hono@1687

@modelcontextprotocol/node

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/node@1687

commit: 5fcc3b1

[travisbreaks]

Good change overall. Centralizing onerror in createJsonErrorResponse() is the right structural call: it guarantees coverage for every path that uses that helper, including validateSession, validateProtocolVersion, and the various 4xx checks that were previously silent. The test coverage is solid.

A few things worth flagging:

1. handleUnsupportedRequest() bypasses the centralized path

handleUnsupportedRequest() (the 405 handler for PUT/PATCH/etc.) builds its own Response.json(...) directly instead of calling createJsonErrorResponse(). After this PR, that error path still won't fire onerror. It may be intentional (unsupported HTTP methods aren't really transport errors), but if the goal is "every error response triggers the callback," this is a gap. Worth either converting it to use createJsonErrorResponse() or documenting the exception.

2. Lost error fidelity in catch blocks

The two catch blocks that previously called onerror with the actual caught error (error as Error) now get new Error(message) where message is a generic string like "Error replaying events" or "Parse error". The real exception (with its original message, type, and stack trace) is discarded from the callback. For the handlePostRequest outer catch, the original error still appears as data in the HTTP response body, but anyone relying on onerror for logging/observability loses the underlying cause.

Consider preserving the original error when one is available. For example, the replayEvents catch could pass the caught error to onerror and still call createJsonErrorResponse separately (accepting the "double call" in that one path), or createJsonErrorResponse could accept an optional cause parameter:

privatecreateJsonErrorResponse(status: number,code: number,message: string,options?: {headers?: Record<string,string>;data?: string;cause?: Error}): Response{this.onerror?.(options?.cause??newError(message));// ...}

3. Overlap with #1684

PR #1684 fixes a subset of the same issue (the two inner parse catch blocks in handlePostRequest). That PR takes a different approach: it adds targeted onerror calls at each catch site and preserves the actual caught error object. If this PR lands first, #1684 becomes a no-op for those paths. If #1684 lands first, this PR would need a rebase and could potentially double-fire on those two catch blocks. The maintainers should pick one direction and close the other.

Between the two, this PR has better architectural coverage (all createJsonErrorResponse callers get onerror for free going forward). But #1684 preserves error fidelity. The ideal outcome would be this PR's centralized approach combined with #1684's pattern of forwarding the real error object.

Nice tests. The changeset is included, which #1684 is missing.

[kdn-posipaka]

Thanks for the thorough review, @travisbreaks! Great catches — I've pushed a commit addressing your feedback:

1. handleUnsupportedRequest() now uses createJsonErrorResponse() — fully consistent with the centralized path. The Allow header is passed via the headers option.

2. Error fidelity preserved via cause parameter — Added an optional cause?: Error field to the options. The onerror callback now receives options?.cause ?? new Error(message), so catch blocks that have the original error can pass it through (e.g., the POST parse error catch block now passes the caught error as cause).

3. Overlap with fix(server): surface parse/validation transport errors via onerror in streamable HTTP #1684 — This PR supersedes fix(server): surface parse/validation transport errors via onerror in streamable HTTP #1684's scope (centralizing onerror in createJsonErrorResponse covers what fix(server): surface parse/validation transport errors via onerror in streamable HTTP #1684 aimed to fix). This one should land first; fix(server): surface parse/validation transport errors via onerror in streamable HTTP #1684 can then be closed or rebased if needed.

All tests pass (43/43). Let me know if anything else needs adjustment!

[Maverick-666]

Thanks @travisbreaks and @kai-agent-free — great progress on this.

I closed #1684 to consolidate on this PR and avoid parallel fixes for #1395. I’m happy to actively help push this over the line.

If useful, I can immediately contribute one focused follow-up commit that:

1. adds/expands regression tests for the remaining centralized error paths,
2. verifies original error fidelity (cause) is preserved end-to-end in onerror,
3. updates any related docs/changelog bits if maintainers want that included here.

If you’re good with that scope, I can open the follow-up against this branch today.

[kdn-posipaka]

Thanks @Maverick-666! Appreciate closing #1684 to consolidate — makes sense to avoid parallel fixes.

Your follow-up commit proposal sounds good. The regression test coverage for the new cause parameter + handleUnsupportedRequest path would strengthen the PR. Feel free to push to the branch if you'd like, or I can incorporate specific test cases you have in mind.

The handlePostRequest outer catch already passes the caught error as cause now, so the fidelity issue travisbreaks flagged should be resolved. Happy to iterate on anything else.

[Maverick-666]

Thanks! I can take this.

I’ll add focused regression coverage for:

1. handleUnsupportedRequest path triggers onerror
2. centralized error-response path preserves error fidelity via cause
3. parse-failure paths still surface meaningful original error context to onerror

I’ll keep it test-only and minimal, then share the commit/PR link here.

[Maverick-666]

Quick update: I implemented the test follow-up, but I don’t have push permission to kai-agent-free/typescript-sdk (got 403 when pushing to the PR head branch).

I pushed the changes to my fork instead:

• Branch: Maverick-666:codex/pr1687-onerror-tests
• Commit: 65e3de3 (test(server): add regression coverage for onerror unsupported-method and cause fidelity)

Local verification:

• pnpm --filter @modelcontextprotocol/server test -- test/server/streamableHttp.test.ts (pass)

If convenient, you can cherry-pick this commit:

git fetch https://github.com/Maverick-666/typescript-sdk.git codex/pr1687-onerror-tests
git cherry-pick 65e3de3

[kdn-posipaka]

Thanks @Maverick-666 for the additional test coverage! 🙏 I've cherry-picked your commit (65e3de3) into this branch — the unsupported-method onerror regression test and cause fidelity check are great additions. All 385 tests passing.

[Maverick-666]

Awesome, thanks for cherry-picking and confirming all tests pass.

Glad this helped strengthen the PR. Happy to help with any additional follow-ups if needed.

[felixweinberger]

Thanks for the contribution! Closing in favor of #1433.

[kdn-posipaka]

Summary

Fixes#1395

Several error paths in StreamableHTTPServerTransport returned JSON error responses via createJsonErrorResponse() without calling the onerror callback, silently swallowing transport errors.

Changes

packages/server/src/server/streamableHttp.ts:

• Added this.onerror?.(new Error(message)) inside createJsonErrorResponse() so every error response automatically triggers the callback
• Removed 4 redundant manual this.onerror?.() calls that preceded createJsonErrorResponse() to avoid double-firing

This ensures all error types are observable — including parse errors, invalid headers, session validation failures, and protocol version mismatches — which were previously invisible to users.

packages/server/test/server/streamableHttp.test.ts:

• Added 5 new tests verifying onerror is called for: invalid Accept header (POST & GET), invalid Content-Type, invalid JSON, and invalid JSON-RPC messages

Test Results

All 42 tests pass (37 existing + 5 new). TypeScript compiles cleanly.

[changeset-bot]

🦋 Changeset detected

Latest commit: 5fcc3b1

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

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

[pkg-pr-new]

Open in StackBlitz

@modelcontextprotocol/client

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/client@1687

@modelcontextprotocol/server

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/server@1687

@modelcontextprotocol/express

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/express@1687

@modelcontextprotocol/hono

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/hono@1687

@modelcontextprotocol/node

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/node@1687

commit: 5fcc3b1

[travisbreaks]

Good change overall. Centralizing onerror in createJsonErrorResponse() is the right structural call: it guarantees coverage for every path that uses that helper, including validateSession, validateProtocolVersion, and the various 4xx checks that were previously silent. The test coverage is solid.

A few things worth flagging:

1. handleUnsupportedRequest() bypasses the centralized path

handleUnsupportedRequest() (the 405 handler for PUT/PATCH/etc.) builds its own Response.json(...) directly instead of calling createJsonErrorResponse(). After this PR, that error path still won't fire onerror. It may be intentional (unsupported HTTP methods aren't really transport errors), but if the goal is "every error response triggers the callback," this is a gap. Worth either converting it to use createJsonErrorResponse() or documenting the exception.

2. Lost error fidelity in catch blocks

The two catch blocks that previously called onerror with the actual caught error (error as Error) now get new Error(message) where message is a generic string like "Error replaying events" or "Parse error". The real exception (with its original message, type, and stack trace) is discarded from the callback. For the handlePostRequest outer catch, the original error still appears as data in the HTTP response body, but anyone relying on onerror for logging/observability loses the underlying cause.

Consider preserving the original error when one is available. For example, the replayEvents catch could pass the caught error to onerror and still call createJsonErrorResponse separately (accepting the "double call" in that one path), or createJsonErrorResponse could accept an optional cause parameter:

privatecreateJsonErrorResponse(status: number,code: number,message: string,options?: {headers?: Record<string,string>;data?: string;cause?: Error}): Response{this.onerror?.(options?.cause??newError(message));// ...}

3. Overlap with #1684

PR #1684 fixes a subset of the same issue (the two inner parse catch blocks in handlePostRequest). That PR takes a different approach: it adds targeted onerror calls at each catch site and preserves the actual caught error object. If this PR lands first, #1684 becomes a no-op for those paths. If #1684 lands first, this PR would need a rebase and could potentially double-fire on those two catch blocks. The maintainers should pick one direction and close the other.

Between the two, this PR has better architectural coverage (all createJsonErrorResponse callers get onerror for free going forward). But #1684 preserves error fidelity. The ideal outcome would be this PR's centralized approach combined with #1684's pattern of forwarding the real error object.

Nice tests. The changeset is included, which #1684 is missing.

[kdn-posipaka]

Thanks for the thorough review, @travisbreaks! Great catches — I've pushed a commit addressing your feedback:

1. handleUnsupportedRequest() now uses createJsonErrorResponse() — fully consistent with the centralized path. The Allow header is passed via the headers option.

2. Error fidelity preserved via cause parameter — Added an optional cause?: Error field to the options. The onerror callback now receives options?.cause ?? new Error(message), so catch blocks that have the original error can pass it through (e.g., the POST parse error catch block now passes the caught error as cause).

3. Overlap with fix(server): surface parse/validation transport errors via onerror in streamable HTTP #1684 — This PR supersedes fix(server): surface parse/validation transport errors via onerror in streamable HTTP #1684's scope (centralizing onerror in createJsonErrorResponse covers what fix(server): surface parse/validation transport errors via onerror in streamable HTTP #1684 aimed to fix). This one should land first; fix(server): surface parse/validation transport errors via onerror in streamable HTTP #1684 can then be closed or rebased if needed.

All tests pass (43/43). Let me know if anything else needs adjustment!

[Maverick-666]

Thanks @travisbreaks and @kai-agent-free — great progress on this.

I closed #1684 to consolidate on this PR and avoid parallel fixes for #1395. I’m happy to actively help push this over the line.

If useful, I can immediately contribute one focused follow-up commit that:

1. adds/expands regression tests for the remaining centralized error paths,
2. verifies original error fidelity (cause) is preserved end-to-end in onerror,
3. updates any related docs/changelog bits if maintainers want that included here.

If you’re good with that scope, I can open the follow-up against this branch today.

[kdn-posipaka]

Thanks @Maverick-666! Appreciate closing #1684 to consolidate — makes sense to avoid parallel fixes.

Your follow-up commit proposal sounds good. The regression test coverage for the new cause parameter + handleUnsupportedRequest path would strengthen the PR. Feel free to push to the branch if you'd like, or I can incorporate specific test cases you have in mind.

The handlePostRequest outer catch already passes the caught error as cause now, so the fidelity issue travisbreaks flagged should be resolved. Happy to iterate on anything else.

[Maverick-666]

Thanks! I can take this.

I’ll add focused regression coverage for:

1. handleUnsupportedRequest path triggers onerror
2. centralized error-response path preserves error fidelity via cause
3. parse-failure paths still surface meaningful original error context to onerror

I’ll keep it test-only and minimal, then share the commit/PR link here.

[Maverick-666]

Quick update: I implemented the test follow-up, but I don’t have push permission to kai-agent-free/typescript-sdk (got 403 when pushing to the PR head branch).

I pushed the changes to my fork instead:

• Branch: Maverick-666:codex/pr1687-onerror-tests
• Commit: 65e3de3 (test(server): add regression coverage for onerror unsupported-method and cause fidelity)

Local verification:

• pnpm --filter @modelcontextprotocol/server test -- test/server/streamableHttp.test.ts (pass)

If convenient, you can cherry-pick this commit:

git fetch https://github.com/Maverick-666/typescript-sdk.git codex/pr1687-onerror-tests
git cherry-pick 65e3de3

[kdn-posipaka]

Thanks @Maverick-666 for the additional test coverage! 🙏 I've cherry-picked your commit (65e3de3) into this branch — the unsupported-method onerror regression test and cause fidelity check are great additions. All 385 tests passing.

[Maverick-666]

Awesome, thanks for cherry-picking and confirming all tests pass.

Glad this helped strengthen the PR. Happy to help with any additional follow-ups if needed.

[felixweinberger]

Thanks for the contribution! Closing in favor of #1433.

[kdn-posipaka]

Summary

Fixes#1395

Several error paths in StreamableHTTPServerTransport returned JSON error responses via createJsonErrorResponse() without calling the onerror callback, silently swallowing transport errors.

Changes

packages/server/src/server/streamableHttp.ts:

• Added this.onerror?.(new Error(message)) inside createJsonErrorResponse() so every error response automatically triggers the callback
• Removed 4 redundant manual this.onerror?.() calls that preceded createJsonErrorResponse() to avoid double-firing

This ensures all error types are observable — including parse errors, invalid headers, session validation failures, and protocol version mismatches — which were previously invisible to users.

packages/server/test/server/streamableHttp.test.ts:

• Added 5 new tests verifying onerror is called for: invalid Accept header (POST & GET), invalid Content-Type, invalid JSON, and invalid JSON-RPC messages

Test Results

All 42 tests pass (37 existing + 5 new). TypeScript compiles cleanly.

[changeset-bot]

🦋 Changeset detected

Latest commit: 5fcc3b1

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

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

[pkg-pr-new]

Open in StackBlitz

@modelcontextprotocol/client

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/client@1687

@modelcontextprotocol/server

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/server@1687

@modelcontextprotocol/express

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/express@1687

@modelcontextprotocol/hono

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/hono@1687

@modelcontextprotocol/node

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/node@1687

commit: 5fcc3b1

[travisbreaks]

Good change overall. Centralizing onerror in createJsonErrorResponse() is the right structural call: it guarantees coverage for every path that uses that helper, including validateSession, validateProtocolVersion, and the various 4xx checks that were previously silent. The test coverage is solid.

A few things worth flagging:

1. handleUnsupportedRequest() bypasses the centralized path

handleUnsupportedRequest() (the 405 handler for PUT/PATCH/etc.) builds its own Response.json(...) directly instead of calling createJsonErrorResponse(). After this PR, that error path still won't fire onerror. It may be intentional (unsupported HTTP methods aren't really transport errors), but if the goal is "every error response triggers the callback," this is a gap. Worth either converting it to use createJsonErrorResponse() or documenting the exception.

2. Lost error fidelity in catch blocks

The two catch blocks that previously called onerror with the actual caught error (error as Error) now get new Error(message) where message is a generic string like "Error replaying events" or "Parse error". The real exception (with its original message, type, and stack trace) is discarded from the callback. For the handlePostRequest outer catch, the original error still appears as data in the HTTP response body, but anyone relying on onerror for logging/observability loses the underlying cause.

Consider preserving the original error when one is available. For example, the replayEvents catch could pass the caught error to onerror and still call createJsonErrorResponse separately (accepting the "double call" in that one path), or createJsonErrorResponse could accept an optional cause parameter:

privatecreateJsonErrorResponse(status: number,code: number,message: string,options?: {headers?: Record<string,string>;data?: string;cause?: Error}): Response{this.onerror?.(options?.cause??newError(message));// ...}

3. Overlap with #1684

PR #1684 fixes a subset of the same issue (the two inner parse catch blocks in handlePostRequest). That PR takes a different approach: it adds targeted onerror calls at each catch site and preserves the actual caught error object. If this PR lands first, #1684 becomes a no-op for those paths. If #1684 lands first, this PR would need a rebase and could potentially double-fire on those two catch blocks. The maintainers should pick one direction and close the other.

Between the two, this PR has better architectural coverage (all createJsonErrorResponse callers get onerror for free going forward). But #1684 preserves error fidelity. The ideal outcome would be this PR's centralized approach combined with #1684's pattern of forwarding the real error object.

Nice tests. The changeset is included, which #1684 is missing.

[kdn-posipaka]

Thanks for the thorough review, @travisbreaks! Great catches — I've pushed a commit addressing your feedback:

1. handleUnsupportedRequest() now uses createJsonErrorResponse() — fully consistent with the centralized path. The Allow header is passed via the headers option.

2. Error fidelity preserved via cause parameter — Added an optional cause?: Error field to the options. The onerror callback now receives options?.cause ?? new Error(message), so catch blocks that have the original error can pass it through (e.g., the POST parse error catch block now passes the caught error as cause).

3. Overlap with fix(server): surface parse/validation transport errors via onerror in streamable HTTP #1684 — This PR supersedes fix(server): surface parse/validation transport errors via onerror in streamable HTTP #1684's scope (centralizing onerror in createJsonErrorResponse covers what fix(server): surface parse/validation transport errors via onerror in streamable HTTP #1684 aimed to fix). This one should land first; fix(server): surface parse/validation transport errors via onerror in streamable HTTP #1684 can then be closed or rebased if needed.

All tests pass (43/43). Let me know if anything else needs adjustment!

[Maverick-666]

Thanks @travisbreaks and @kai-agent-free — great progress on this.

I closed #1684 to consolidate on this PR and avoid parallel fixes for #1395. I’m happy to actively help push this over the line.

If useful, I can immediately contribute one focused follow-up commit that:

1. adds/expands regression tests for the remaining centralized error paths,
2. verifies original error fidelity (cause) is preserved end-to-end in onerror,
3. updates any related docs/changelog bits if maintainers want that included here.

If you’re good with that scope, I can open the follow-up against this branch today.

[kdn-posipaka]

Thanks @Maverick-666! Appreciate closing #1684 to consolidate — makes sense to avoid parallel fixes.

Your follow-up commit proposal sounds good. The regression test coverage for the new cause parameter + handleUnsupportedRequest path would strengthen the PR. Feel free to push to the branch if you'd like, or I can incorporate specific test cases you have in mind.

The handlePostRequest outer catch already passes the caught error as cause now, so the fidelity issue travisbreaks flagged should be resolved. Happy to iterate on anything else.

[Maverick-666]

Thanks! I can take this.

I’ll add focused regression coverage for:

1. handleUnsupportedRequest path triggers onerror
2. centralized error-response path preserves error fidelity via cause
3. parse-failure paths still surface meaningful original error context to onerror

I’ll keep it test-only and minimal, then share the commit/PR link here.

[Maverick-666]

Quick update: I implemented the test follow-up, but I don’t have push permission to kai-agent-free/typescript-sdk (got 403 when pushing to the PR head branch).

I pushed the changes to my fork instead:

• Branch: Maverick-666:codex/pr1687-onerror-tests
• Commit: 65e3de3 (test(server): add regression coverage for onerror unsupported-method and cause fidelity)

Local verification:

• pnpm --filter @modelcontextprotocol/server test -- test/server/streamableHttp.test.ts (pass)

If convenient, you can cherry-pick this commit:

git fetch https://github.com/Maverick-666/typescript-sdk.git codex/pr1687-onerror-tests
git cherry-pick 65e3de3

[kdn-posipaka]

Thanks @Maverick-666 for the additional test coverage! 🙏 I've cherry-picked your commit (65e3de3) into this branch — the unsupported-method onerror regression test and cause fidelity check are great additions. All 385 tests passing.

[Maverick-666]

Awesome, thanks for cherry-picking and confirming all tests pass.

Glad this helped strengthen the PR. Happy to help with any additional follow-ups if needed.

[felixweinberger]

Thanks for the contribution! Closing in favor of #1433.

[kdn-posipaka]

Summary

Fixes#1395

Several error paths in StreamableHTTPServerTransport returned JSON error responses via createJsonErrorResponse() without calling the onerror callback, silently swallowing transport errors.

Changes

packages/server/src/server/streamableHttp.ts:

• Added this.onerror?.(new Error(message)) inside createJsonErrorResponse() so every error response automatically triggers the callback
• Removed 4 redundant manual this.onerror?.() calls that preceded createJsonErrorResponse() to avoid double-firing

This ensures all error types are observable — including parse errors, invalid headers, session validation failures, and protocol version mismatches — which were previously invisible to users.

packages/server/test/server/streamableHttp.test.ts:

• Added 5 new tests verifying onerror is called for: invalid Accept header (POST & GET), invalid Content-Type, invalid JSON, and invalid JSON-RPC messages

Test Results

All 42 tests pass (37 existing + 5 new). TypeScript compiles cleanly.

[changeset-bot]

🦋 Changeset detected

Latest commit: 5fcc3b1

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

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

[pkg-pr-new]

Open in StackBlitz

@modelcontextprotocol/client

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/client@1687

@modelcontextprotocol/server

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/server@1687

@modelcontextprotocol/express

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/express@1687

@modelcontextprotocol/hono

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/hono@1687

@modelcontextprotocol/node

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/node@1687

commit: 5fcc3b1

[travisbreaks]

Good change overall. Centralizing onerror in createJsonErrorResponse() is the right structural call: it guarantees coverage for every path that uses that helper, including validateSession, validateProtocolVersion, and the various 4xx checks that were previously silent. The test coverage is solid.

A few things worth flagging:

1. handleUnsupportedRequest() bypasses the centralized path

handleUnsupportedRequest() (the 405 handler for PUT/PATCH/etc.) builds its own Response.json(...) directly instead of calling createJsonErrorResponse(). After this PR, that error path still won't fire onerror. It may be intentional (unsupported HTTP methods aren't really transport errors), but if the goal is "every error response triggers the callback," this is a gap. Worth either converting it to use createJsonErrorResponse() or documenting the exception.

2. Lost error fidelity in catch blocks

The two catch blocks that previously called onerror with the actual caught error (error as Error) now get new Error(message) where message is a generic string like "Error replaying events" or "Parse error". The real exception (with its original message, type, and stack trace) is discarded from the callback. For the handlePostRequest outer catch, the original error still appears as data in the HTTP response body, but anyone relying on onerror for logging/observability loses the underlying cause.

Consider preserving the original error when one is available. For example, the replayEvents catch could pass the caught error to onerror and still call createJsonErrorResponse separately (accepting the "double call" in that one path), or createJsonErrorResponse could accept an optional cause parameter:

privatecreateJsonErrorResponse(status: number,code: number,message: string,options?: {headers?: Record<string,string>;data?: string;cause?: Error}): Response{this.onerror?.(options?.cause??newError(message));// ...}

3. Overlap with #1684

PR #1684 fixes a subset of the same issue (the two inner parse catch blocks in handlePostRequest). That PR takes a different approach: it adds targeted onerror calls at each catch site and preserves the actual caught error object. If this PR lands first, #1684 becomes a no-op for those paths. If #1684 lands first, this PR would need a rebase and could potentially double-fire on those two catch blocks. The maintainers should pick one direction and close the other.

Between the two, this PR has better architectural coverage (all createJsonErrorResponse callers get onerror for free going forward). But #1684 preserves error fidelity. The ideal outcome would be this PR's centralized approach combined with #1684's pattern of forwarding the real error object.

Nice tests. The changeset is included, which #1684 is missing.

[kdn-posipaka]

Thanks for the thorough review, @travisbreaks! Great catches — I've pushed a commit addressing your feedback:

1. handleUnsupportedRequest() now uses createJsonErrorResponse() — fully consistent with the centralized path. The Allow header is passed via the headers option.

2. Error fidelity preserved via cause parameter — Added an optional cause?: Error field to the options. The onerror callback now receives options?.cause ?? new Error(message), so catch blocks that have the original error can pass it through (e.g., the POST parse error catch block now passes the caught error as cause).

3. Overlap with fix(server): surface parse/validation transport errors via onerror in streamable HTTP #1684 — This PR supersedes fix(server): surface parse/validation transport errors via onerror in streamable HTTP #1684's scope (centralizing onerror in createJsonErrorResponse covers what fix(server): surface parse/validation transport errors via onerror in streamable HTTP #1684 aimed to fix). This one should land first; fix(server): surface parse/validation transport errors via onerror in streamable HTTP #1684 can then be closed or rebased if needed.

All tests pass (43/43). Let me know if anything else needs adjustment!

[Maverick-666]

Thanks @travisbreaks and @kai-agent-free — great progress on this.

I closed #1684 to consolidate on this PR and avoid parallel fixes for #1395. I’m happy to actively help push this over the line.

If useful, I can immediately contribute one focused follow-up commit that:

1. adds/expands regression tests for the remaining centralized error paths,
2. verifies original error fidelity (cause) is preserved end-to-end in onerror,
3. updates any related docs/changelog bits if maintainers want that included here.

If you’re good with that scope, I can open the follow-up against this branch today.

[kdn-posipaka]

Thanks @Maverick-666! Appreciate closing #1684 to consolidate — makes sense to avoid parallel fixes.

Your follow-up commit proposal sounds good. The regression test coverage for the new cause parameter + handleUnsupportedRequest path would strengthen the PR. Feel free to push to the branch if you'd like, or I can incorporate specific test cases you have in mind.

The handlePostRequest outer catch already passes the caught error as cause now, so the fidelity issue travisbreaks flagged should be resolved. Happy to iterate on anything else.

[Maverick-666]

Thanks! I can take this.

I’ll add focused regression coverage for:

1. handleUnsupportedRequest path triggers onerror
2. centralized error-response path preserves error fidelity via cause
3. parse-failure paths still surface meaningful original error context to onerror

I’ll keep it test-only and minimal, then share the commit/PR link here.

[Maverick-666]

Quick update: I implemented the test follow-up, but I don’t have push permission to kai-agent-free/typescript-sdk (got 403 when pushing to the PR head branch).

I pushed the changes to my fork instead:

• Branch: Maverick-666:codex/pr1687-onerror-tests
• Commit: 65e3de3 (test(server): add regression coverage for onerror unsupported-method and cause fidelity)

Local verification:

• pnpm --filter @modelcontextprotocol/server test -- test/server/streamableHttp.test.ts (pass)

If convenient, you can cherry-pick this commit:

git fetch https://github.com/Maverick-666/typescript-sdk.git codex/pr1687-onerror-tests
git cherry-pick 65e3de3

[kdn-posipaka]

Thanks @Maverick-666 for the additional test coverage! 🙏 I've cherry-picked your commit (65e3de3) into this branch — the unsupported-method onerror regression test and cause fidelity check are great additions. All 385 tests passing.

[Maverick-666]

Awesome, thanks for cherry-picking and confirming all tests pass.

Glad this helped strengthen the PR. Happy to help with any additional follow-ups if needed.

[felixweinberger]

Thanks for the contribution! Closing in favor of #1433.

[kdn-posipaka]

Summary

Fixes#1395

Several error paths in StreamableHTTPServerTransport returned JSON error responses via createJsonErrorResponse() without calling the onerror callback, silently swallowing transport errors.

Changes

packages/server/src/server/streamableHttp.ts:

• Added this.onerror?.(new Error(message)) inside createJsonErrorResponse() so every error response automatically triggers the callback
• Removed 4 redundant manual this.onerror?.() calls that preceded createJsonErrorResponse() to avoid double-firing

This ensures all error types are observable — including parse errors, invalid headers, session validation failures, and protocol version mismatches — which were previously invisible to users.

packages/server/test/server/streamableHttp.test.ts:

• Added 5 new tests verifying onerror is called for: invalid Accept header (POST & GET), invalid Content-Type, invalid JSON, and invalid JSON-RPC messages

Test Results

All 42 tests pass (37 existing + 5 new). TypeScript compiles cleanly.

[changeset-bot]

🦋 Changeset detected

Latest commit: 5fcc3b1

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

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

[pkg-pr-new]

Open in StackBlitz

@modelcontextprotocol/client

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/client@1687

@modelcontextprotocol/server

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/server@1687

@modelcontextprotocol/express

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/express@1687

@modelcontextprotocol/hono

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/hono@1687

@modelcontextprotocol/node

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/node@1687

commit: 5fcc3b1

[travisbreaks]

Good change overall. Centralizing onerror in createJsonErrorResponse() is the right structural call: it guarantees coverage for every path that uses that helper, including validateSession, validateProtocolVersion, and the various 4xx checks that were previously silent. The test coverage is solid.

A few things worth flagging:

1. handleUnsupportedRequest() bypasses the centralized path

handleUnsupportedRequest() (the 405 handler for PUT/PATCH/etc.) builds its own Response.json(...) directly instead of calling createJsonErrorResponse(). After this PR, that error path still won't fire onerror. It may be intentional (unsupported HTTP methods aren't really transport errors), but if the goal is "every error response triggers the callback," this is a gap. Worth either converting it to use createJsonErrorResponse() or documenting the exception.

2. Lost error fidelity in catch blocks

The two catch blocks that previously called onerror with the actual caught error (error as Error) now get new Error(message) where message is a generic string like "Error replaying events" or "Parse error". The real exception (with its original message, type, and stack trace) is discarded from the callback. For the handlePostRequest outer catch, the original error still appears as data in the HTTP response body, but anyone relying on onerror for logging/observability loses the underlying cause.

Consider preserving the original error when one is available. For example, the replayEvents catch could pass the caught error to onerror and still call createJsonErrorResponse separately (accepting the "double call" in that one path), or createJsonErrorResponse could accept an optional cause parameter:

privatecreateJsonErrorResponse(status: number,code: number,message: string,options?: {headers?: Record<string,string>;data?: string;cause?: Error}): Response{this.onerror?.(options?.cause??newError(message));// ...}

3. Overlap with #1684

PR #1684 fixes a subset of the same issue (the two inner parse catch blocks in handlePostRequest). That PR takes a different approach: it adds targeted onerror calls at each catch site and preserves the actual caught error object. If this PR lands first, #1684 becomes a no-op for those paths. If #1684 lands first, this PR would need a rebase and could potentially double-fire on those two catch blocks. The maintainers should pick one direction and close the other.

Between the two, this PR has better architectural coverage (all createJsonErrorResponse callers get onerror for free going forward). But #1684 preserves error fidelity. The ideal outcome would be this PR's centralized approach combined with #1684's pattern of forwarding the real error object.

Nice tests. The changeset is included, which #1684 is missing.

[kdn-posipaka]

Thanks for the thorough review, @travisbreaks! Great catches — I've pushed a commit addressing your feedback:

1. handleUnsupportedRequest() now uses createJsonErrorResponse() — fully consistent with the centralized path. The Allow header is passed via the headers option.

2. Error fidelity preserved via cause parameter — Added an optional cause?: Error field to the options. The onerror callback now receives options?.cause ?? new Error(message), so catch blocks that have the original error can pass it through (e.g., the POST parse error catch block now passes the caught error as cause).

3. Overlap with fix(server): surface parse/validation transport errors via onerror in streamable HTTP #1684 — This PR supersedes fix(server): surface parse/validation transport errors via onerror in streamable HTTP #1684's scope (centralizing onerror in createJsonErrorResponse covers what fix(server): surface parse/validation transport errors via onerror in streamable HTTP #1684 aimed to fix). This one should land first; fix(server): surface parse/validation transport errors via onerror in streamable HTTP #1684 can then be closed or rebased if needed.

All tests pass (43/43). Let me know if anything else needs adjustment!

[Maverick-666]

Thanks @travisbreaks and @kai-agent-free — great progress on this.

I closed #1684 to consolidate on this PR and avoid parallel fixes for #1395. I’m happy to actively help push this over the line.

If useful, I can immediately contribute one focused follow-up commit that:

1. adds/expands regression tests for the remaining centralized error paths,
2. verifies original error fidelity (cause) is preserved end-to-end in onerror,
3. updates any related docs/changelog bits if maintainers want that included here.

If you’re good with that scope, I can open the follow-up against this branch today.

[kdn-posipaka]

Thanks @Maverick-666! Appreciate closing #1684 to consolidate — makes sense to avoid parallel fixes.

Your follow-up commit proposal sounds good. The regression test coverage for the new cause parameter + handleUnsupportedRequest path would strengthen the PR. Feel free to push to the branch if you'd like, or I can incorporate specific test cases you have in mind.

The handlePostRequest outer catch already passes the caught error as cause now, so the fidelity issue travisbreaks flagged should be resolved. Happy to iterate on anything else.

[Maverick-666]

Thanks! I can take this.

I’ll add focused regression coverage for:

1. handleUnsupportedRequest path triggers onerror
2. centralized error-response path preserves error fidelity via cause
3. parse-failure paths still surface meaningful original error context to onerror

I’ll keep it test-only and minimal, then share the commit/PR link here.

[Maverick-666]

Quick update: I implemented the test follow-up, but I don’t have push permission to kai-agent-free/typescript-sdk (got 403 when pushing to the PR head branch).

I pushed the changes to my fork instead:

• Branch: Maverick-666:codex/pr1687-onerror-tests
• Commit: 65e3de3 (test(server): add regression coverage for onerror unsupported-method and cause fidelity)

Local verification:

• pnpm --filter @modelcontextprotocol/server test -- test/server/streamableHttp.test.ts (pass)

If convenient, you can cherry-pick this commit:

git fetch https://github.com/Maverick-666/typescript-sdk.git codex/pr1687-onerror-tests
git cherry-pick 65e3de3

[kdn-posipaka]

Thanks @Maverick-666 for the additional test coverage! 🙏 I've cherry-picked your commit (65e3de3) into this branch — the unsupported-method onerror regression test and cause fidelity check are great additions. All 385 tests passing.

[Maverick-666]

Awesome, thanks for cherry-picking and confirming all tests pass.

Glad this helped strengthen the PR. Happy to help with any additional follow-ups if needed.

[felixweinberger]

Thanks for the contribution! Closing in favor of #1433.