diff --git a/.changeset/nullish-handler-rejection.md b/.changeset/nullish-handler-rejection.md new file mode 100644 index 0000000000..62ee8ec3ec --- /dev/null +++ b/.changeset/nullish-handler-rejection.md @@ -0,0 +1,5 @@ +--- +'@modelcontextprotocol/core': patch +--- + +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. diff --git a/packages/core-internal/src/shared/protocol.ts b/packages/core-internal/src/shared/protocol.ts index ffab6e8df8..1d33c45ae3 100644 --- a/packages/core-internal/src/shared/protocol.ts +++ b/packages/core-internal/src/shared/protocol.ts @@ -1137,14 +1137,19 @@ export abstract class Protocol { // the wire code for a handler-thrown error, so per-era // wire-code policy lives in the codec rather than in any // handler. Non-integer codes still fall through to −32603. - const thrownCode = Number.isSafeInteger(error['code']) ? (error['code'] as number) : ProtocolErrorCode.InternalError; + // Coalesce the reason first: a handler may reject with a + // nullish value (a bare `reject()` or `throw null`), and + // indexing `error['code']` on it would throw here and strand + // the peer with no error response at all. + const reason = (error ?? {}) as { code?: unknown; message?: string; data?: unknown }; + const thrownCode = Number.isSafeInteger(reason.code) ? (reason.code as number) : ProtocolErrorCode.InternalError; const errorResponse: JSONRPCErrorResponse = { jsonrpc: '2.0', id: request.id, error: { code: codec.encodeErrorCode(thrownCode), - message: error.message ?? 'Internal error', - ...(error['data'] !== undefined && { data: error['data'] }) + message: reason.message ?? 'Internal error', + ...(reason.data !== undefined && { data: reason.data }) } }; await capturedTransport?.send(errorResponse); diff --git a/packages/core-internal/test/shared/protocol.test.ts b/packages/core-internal/test/shared/protocol.test.ts index 2ecdc40adc..8f798b840c 100644 --- a/packages/core-internal/test/shared/protocol.test.ts +++ b/packages/core-internal/test/shared/protocol.test.ts @@ -135,6 +135,30 @@ describe('protocol tests', () => { expect((abortReason as SdkError).code).toBe(SdkErrorCode.ConnectionClosed); }); + test('sends an error response when a request handler rejects with a nullish reason', async () => { + await protocol.connect(transport); + + // A handler that rejects with `undefined` (a bare `reject()` or + // `throw null` in library code) must still yield a JSON-RPC error + // response; otherwise the error-response encode itself throws while + // indexing the nullish reason and the requester hangs until timeout. + protocol.setRequestHandler('ping', async () => Promise.reject(undefined)); + + transport.onmessage?.({ jsonrpc: '2.0', id: 1, method: 'ping', params: {} }); + + await vi.waitFor(() => { + const errorSend = sendSpy.mock.calls.find( + ([msg]) => (msg as JSONRPCErrorResponse)?.id === 1 && (msg as JSONRPCErrorResponse)?.error !== undefined + ); + expect(errorSend).toBeDefined(); + }); + + const errorSend = sendSpy.mock.calls.find( + ([msg]) => (msg as JSONRPCErrorResponse)?.id === 1 && (msg as JSONRPCErrorResponse)?.error !== undefined + )!; + expect((errorSend[0] as JSONRPCErrorResponse).error.message).toBeTruthy(); + }); + test('should remove abort listener from caller signal when request settles', async () => { await protocol.connect(transport);