Skip to content

Commit f2fb031

Browse files
lntutorclaude
authored andcommitted
fix(core): send an error response when a request handler rejects with a nullish reason
Protocol's inbound-request error path read error['code'] directly. When a handler rejects with null/undefined (a bare reject() or throw null), that indexing throws a TypeError inside the rejection callback, which propagates to the outer .catch — so no JSON-RPC error response is ever sent and the requester hangs until its own timeout. Coalesce the reason to a safe object before reading the code/message/data, so a -32603 Internal error response is always returned. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N6RtoHuxrDqTUo9Mw9h4Cv
1 parent f130e1a commit f2fb031

3 files changed

Lines changed: 37 additions & 3 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@modelcontextprotocol/core': patch
3+
---
4+
5+
A request handler that rejects with a nullish reason (a bare `reject()` or `throw null`) no longer strands the peer without a reply. `Protocol`'s inbound-request error path indexed `error['code']` directly, which throws a `TypeError` when the rejection reason is `null`/`undefined`; that throw propagated to the outer `.catch`, so no JSON-RPC error response was ever sent and the requester hung until its own timeout. The reason is now coalesced to a safe object before the error code and message are read, so a `-32603` (Internal error) response is always returned.

packages/core-internal/src/shared/protocol.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1138,14 +1138,19 @@ export abstract class Protocol<ContextT extends BaseContext> {
11381138
// the wire code for a handler-thrown error, so per-era
11391139
// wire-code policy lives in the codec rather than in any
11401140
// handler. Non-integer codes still fall through to −32603.
1141-
const thrownCode = Number.isSafeInteger(error['code']) ? (error['code'] as number) : ProtocolErrorCode.InternalError;
1141+
// Coalesce the reason first: a handler may reject with a
1142+
// nullish value (a bare `reject()` or `throw null`), and
1143+
// indexing `error['code']` on it would throw here and strand
1144+
// the peer with no error response at all.
1145+
const reason = (error ?? {}) as { code?: unknown; message?: string; data?: unknown };
1146+
const thrownCode = Number.isSafeInteger(reason.code) ? (reason.code as number) : ProtocolErrorCode.InternalError;
11421147
const errorResponse: JSONRPCErrorResponse = {
11431148
jsonrpc: '2.0',
11441149
id: request.id,
11451150
error: {
11461151
code: codec.encodeErrorCode(thrownCode),
1147-
message: error.message ?? 'Internal error',
1148-
...(error['data'] !== undefined && { data: error['data'] })
1152+
message: reason.message ?? 'Internal error',
1153+
...(reason.data !== undefined && { data: reason.data })
11491154
}
11501155
};
11511156
await capturedTransport?.send(errorResponse);

packages/core-internal/test/shared/protocol.test.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,30 @@ describe('protocol tests', () => {
135135
expect((abortReason as SdkError).code).toBe(SdkErrorCode.ConnectionClosed);
136136
});
137137

138+
test('sends an error response when a request handler rejects with a nullish reason', async () => {
139+
await protocol.connect(transport);
140+
141+
// A handler that rejects with `undefined` (a bare `reject()` or
142+
// `throw null` in library code) must still yield a JSON-RPC error
143+
// response; otherwise the error-response encode itself throws while
144+
// indexing the nullish reason and the requester hangs until timeout.
145+
protocol.setRequestHandler('ping', async () => Promise.reject(undefined));
146+
147+
transport.onmessage?.({ jsonrpc: '2.0', id: 1, method: 'ping', params: {} });
148+
149+
await vi.waitFor(() => {
150+
const errorSend = sendSpy.mock.calls.find(
151+
([msg]) => (msg as JSONRPCErrorResponse)?.id === 1 && (msg as JSONRPCErrorResponse)?.error !== undefined
152+
);
153+
expect(errorSend).toBeDefined();
154+
});
155+
156+
const errorSend = sendSpy.mock.calls.find(
157+
([msg]) => (msg as JSONRPCErrorResponse)?.id === 1 && (msg as JSONRPCErrorResponse)?.error !== undefined
158+
)!;
159+
expect((errorSend[0] as JSONRPCErrorResponse).error.message).toBeTruthy();
160+
});
161+
138162
test('should remove abort listener from caller signal when request settles', async () => {
139163
await protocol.connect(transport);
140164

0 commit comments

Comments
 (0)