Skip to content

Commit 5f3dbdb

Browse files
refactor(core): dedupe 3-arg dispatch via _parseParamsWithSchema; notification-side throws plain Error
- Extracted _parseParamsWithSchema (strip _meta + parse + throw via factory) shared by _wrapParamsSchemaHandler and the setNotificationHandler 3-arg path. - Notification 3-arg validation failures throw plain Error instead of ProtocolError; they never cross the wire (no JSON-RPC error response), only forwarded to onerror. - Tests: onerror scenario for 3-arg notification handler with invalid params; Server and Client subclass-level tests for 3-arg form parity with per-method wrappers.
1 parent dd1c374 commit 5f3dbdb

4 files changed

Lines changed: 82 additions & 16 deletions

File tree

packages/client/test/client/setRequestHandlerSchemaParity.test.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { describe, expect, it } from 'vitest';
2+
import { z } from 'zod';
23

34
import { CreateMessageRequestSchema, ElicitRequestSchema, InMemoryTransport } from '@modelcontextprotocol/core';
45

@@ -71,4 +72,20 @@ describe('Client.setRequestHandler — Zod-schema form parity', () => {
7172
expect((stringRes.error as { message: string }).message).toContain('Invalid');
7273
expect(schemaRes.error).toEqual(stringRes.error);
7374
});
75+
76+
it('three-arg form gets the same result-validation as string form (elicitation/create)', async () => {
77+
const invalidElicitResult = { action: 'nope' };
78+
const params = { mode: 'form', message: 'q', requestedSchema: { type: 'object', properties: {} } };
79+
const viaThreeArg = await setup(c =>
80+
c.setRequestHandler('elicitation/create', z.object({ mode: z.string() }).passthrough(), () => invalidElicitResult as never)
81+
);
82+
const res = await send(viaThreeArg.ct, 'elicitation/create', params);
83+
expect((res.error as { message: string }).message).toContain('Invalid elicitation result');
84+
});
85+
86+
it('three-arg form handles non-spec methods through Client', async () => {
87+
const { ct } = await setup(c => c.setRequestHandler('acme/echo', z.object({ msg: z.string() }), p => ({ reply: p.msg })));
88+
const res = await send(ct, 'acme/echo', { msg: 'hi' });
89+
expect(res.result).toEqual({ reply: 'hi' });
90+
});
7491
});

packages/core/src/shared/protocol.ts

Lines changed: 29 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1100,8 +1100,28 @@ export abstract class Protocol<ContextT extends BaseContext> {
11001100
}
11011101

11021102
/**
1103-
* Builds a request handler from a `paramsSchema` + params-only user handler. Strips `_meta`,
1104-
* validates `params` against the schema, and invokes the user handler with the parsed params.
1103+
* Shared by the 3-arg `setRequestHandler`/`setNotificationHandler` dispatch:
1104+
* strips protocol-level `_meta`, validates remaining `params` against `paramsSchema`,
1105+
* and returns the parsed value. Throws via `makeError` so each caller can use the
1106+
* error class appropriate for its path (JSON-RPC error for requests, local error for notifications).
1107+
*/
1108+
private async _parseParamsWithSchema(
1109+
method: string,
1110+
paramsSchema: StandardSchemaV1,
1111+
rawParams: unknown,
1112+
makeError: (msg: string) => Error
1113+
): Promise<unknown> {
1114+
const { _meta, ...userParams } = (rawParams ?? {}) as Record<string, unknown>;
1115+
void _meta;
1116+
const parsed = await parseStandardSchema(paramsSchema, userParams);
1117+
if (!parsed.success) {
1118+
throw makeError(`Invalid params for ${method}: ${parsed.error.message}`);
1119+
}
1120+
return parsed.data;
1121+
}
1122+
1123+
/**
1124+
* Builds a request handler from a `paramsSchema` + params-only user handler.
11051125
* Shared by {@linkcode setRequestHandler}'s 3-arg dispatch and `Client`/`Server` overrides
11061126
* so that per-method wrapping can be applied uniformly to the normalized handler.
11071127
*/
@@ -1111,13 +1131,10 @@ export abstract class Protocol<ContextT extends BaseContext> {
11111131
userHandler: (params: unknown, ctx: ContextT) => unknown
11121132
): (request: Request, ctx: ContextT) => Promise<Result> {
11131133
return async (request, ctx) => {
1114-
const { _meta, ...userParams } = (request.params ?? {}) as Record<string, unknown>;
1115-
void _meta;
1116-
const parsed = await parseStandardSchema(paramsSchema, userParams);
1117-
if (!parsed.success) {
1118-
throw new ProtocolError(ProtocolErrorCode.InvalidParams, `Invalid params for ${method}: ${parsed.error.message}`);
1119-
}
1120-
return userHandler(parsed.data, ctx) as Result;
1134+
const data = await this._parseParamsWithSchema(method, paramsSchema, request.params, msg => {
1135+
return new ProtocolError(ProtocolErrorCode.InvalidParams, msg);
1136+
});
1137+
return userHandler(data, ctx) as Result;
11211138
};
11221139
}
11231140

@@ -1209,13 +1226,9 @@ export abstract class Protocol<ContextT extends BaseContext> {
12091226

12101227
const paramsSchema = schemaOrHandler as StandardSchemaV1;
12111228
this._notificationHandlers.set(method, async notification => {
1212-
const { _meta, ...userParams } = (notification.params ?? {}) as Record<string, unknown>;
1213-
void _meta;
1214-
const parsed = await parseStandardSchema(paramsSchema, userParams);
1215-
if (!parsed.success) {
1216-
throw new ProtocolError(ProtocolErrorCode.InvalidParams, `Invalid params for ${method}: ${parsed.error.message}`);
1217-
}
1218-
return maybeHandler(parsed.data);
1229+
// Notification handler errors never cross the wire (no JSON-RPC response); they go to onerror.
1230+
const data = await this._parseParamsWithSchema(method, paramsSchema, notification.params, msg => new Error(msg));
1231+
return maybeHandler(data);
12191232
});
12201233
}
12211234

packages/core/test/shared/threeArgHandlers.test.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,21 @@ describe('setNotificationHandler — three-arg paramsSchema form', () => {
7777
await new Promise(r => setTimeout(r, 0));
7878
expect(received).toEqual([{ n: 1 }, { n: 2 }]);
7979
});
80+
81+
it('forwards param-validation failures to onerror (no JSON-RPC error response)', async () => {
82+
const { a, b } = await makePair();
83+
const errs: Error[] = [];
84+
b.onerror = e => errs.push(e);
85+
let invoked = false;
86+
b.setNotificationHandler('acme/tick', z.object({ n: z.number() }), () => {
87+
invoked = true;
88+
});
89+
await a.notification({ method: 'acme/tick', params: { n: 'not-a-number' } });
90+
await new Promise(r => setTimeout(r, 0));
91+
expect(invoked).toBe(false);
92+
expect(errs.length).toBe(1);
93+
expect(String(errs[0])).toMatch(/Invalid params for acme\/tick/);
94+
});
8095
});
8196

8297
describe('non-Zod StandardSchemaV1', () => {

packages/server/test/server/setRequestHandlerSchemaParity.test.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,4 +63,25 @@ describe('Server.setRequestHandler — Zod-schema form parity', () => {
6363
});
6464
expect(res.result).toEqual({ reply: 'hi' });
6565
});
66+
67+
it('three-arg form gets the same task-result validation as string form', async () => {
68+
const invalidTaskResult = { content: [{ type: 'text' as const, text: 'not a task result' }] };
69+
const viaThreeArg = await setup(s =>
70+
s.setRequestHandler('tools/call', z.object({ name: z.string() }).passthrough(), () => invalidTaskResult)
71+
);
72+
const res = await callToolWithTask(viaThreeArg.ct);
73+
expect((res.error as { message: string }).message).toContain('Invalid task creation result');
74+
});
75+
76+
it('three-arg form handles non-spec methods through Server', async () => {
77+
const { ct } = await setup(s => s.setRequestHandler('acme/echo', z.object({ msg: z.string() }), p => ({ reply: p.msg })));
78+
const res = await new Promise<{ result?: unknown; error?: unknown }>(resolve => {
79+
ct.onmessage = m => {
80+
const msg = m as { result?: unknown; error?: unknown };
81+
if ('result' in msg || 'error' in msg) resolve(msg);
82+
};
83+
ct.send({ jsonrpc: '2.0', id: 1, method: 'acme/echo', params: { msg: 'hi' } });
84+
});
85+
expect(res.result).toEqual({ reply: 'hi' });
86+
});
6687
});

0 commit comments

Comments
 (0)