Skip to content

Commit e3bb7d4

Browse files
committed
fix: toolsHost review updates
1 parent 9309626 commit e3bb7d4

File tree

3 files changed

+181
-1
lines changed

3 files changed

+181
-1
lines changed

src/__tests__/__snapshots__/server.toolsHost.test.ts.snap

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,74 @@ exports[`requestHello should send hello:ack message, with valid request 1`] = `
190190
]
191191
`;
192192

193+
exports[`requestInvoke should attempt tool invocation, handler attempting to return a DOMException-like object, with name, message and multiline line stack 1`] = `
194+
{
195+
"error": "Internal error",
196+
"id": "request-id",
197+
"ok": false,
198+
"t": "invoke:result",
199+
}
200+
`;
201+
202+
exports[`requestInvoke should attempt tool invocation, handler attempting to return a browser-like ErrorEvent-like object, with name, message and multiline line stack 1`] = `
203+
{
204+
"error": "Internal error",
205+
"id": "request-id",
206+
"ok": false,
207+
"t": "invoke:result",
208+
}
209+
`;
210+
211+
exports[`requestInvoke should attempt tool invocation, handler attempting to return an error-like object, with message 1`] = `
212+
{
213+
"id": "request-id",
214+
"ok": true,
215+
"result": {
216+
"message": "Handler error",
217+
},
218+
"t": "invoke:result",
219+
}
220+
`;
221+
222+
exports[`requestInvoke should attempt tool invocation, handler attempting to return an error-like object, with name and multiline line stack 1`] = `
223+
{
224+
"error": "Internal error",
225+
"id": "request-id",
226+
"ok": false,
227+
"t": "invoke:result",
228+
}
229+
`;
230+
231+
exports[`requestInvoke should attempt tool invocation, handler attempting to return an error-like object, with name and single line stack 1`] = `
232+
{
233+
"error": "Internal error",
234+
"id": "request-id",
235+
"ok": false,
236+
"t": "invoke:result",
237+
}
238+
`;
239+
240+
exports[`requestInvoke should attempt tool invocation, handler attempting to return an error-like object, with single line stack 1`] = `
241+
{
242+
"id": "request-id",
243+
"ok": true,
244+
"result": {
245+
"message": "Handler error",
246+
"stack": "Stack trace",
247+
},
248+
"t": "invoke:result",
249+
}
250+
`;
251+
252+
exports[`requestInvoke should attempt tool invocation, handler returning AggregateError 1`] = `
253+
{
254+
"error": "Internal error",
255+
"id": "request-id",
256+
"ok": false,
257+
"t": "invoke:result",
258+
}
259+
`;
260+
193261
exports[`requestInvoke should attempt tool invocation, handler returning error 1`] = `
194262
{
195263
"error": "Internal error",
@@ -199,6 +267,15 @@ exports[`requestInvoke should attempt tool invocation, handler returning error 1
199267
}
200268
`;
201269

270+
exports[`requestInvoke should attempt tool invocation, handler returning null 1`] = `
271+
{
272+
"id": "request-id",
273+
"ok": true,
274+
"result": null,
275+
"t": "invoke:result",
276+
}
277+
`;
278+
202279
exports[`requestInvoke should attempt tool invocation, handler returning promise 1`] = `
203280
{
204281
"id": "request-id",
@@ -210,6 +287,15 @@ exports[`requestInvoke should attempt tool invocation, handler returning promise
210287
}
211288
`;
212289

290+
exports[`requestInvoke should attempt tool invocation, handler returning undefined 1`] = `
291+
{
292+
"id": "request-id",
293+
"ok": true,
294+
"result": undefined,
295+
"t": "invoke:result",
296+
}
297+
`;
298+
213299
exports[`requestInvoke should attempt tool invocation, handler throwing error 1`] = `
214300
{
215301
"error": "Handler error",

src/__tests__/server.toolsHost.test.ts

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,60 @@ describe('requestInvoke', () => {
183183
handlerResult: { data: 'result' },
184184
stateToolId: 'tool-1',
185185
requestToolId: 'tool-2'
186+
},
187+
{
188+
description: 'handler returning AggregateError',
189+
handlerResult: new AggregateError(['Handler error']),
190+
stateToolId: 'tool-1',
191+
requestToolId: 'tool-1'
192+
},
193+
{
194+
description: 'handler attempting to return an error-like object, with message',
195+
handlerResult: { message: 'Handler error' },
196+
stateToolId: 'tool-1',
197+
requestToolId: 'tool-1'
198+
},
199+
{
200+
description: 'handler attempting to return an error-like object, with single line stack',
201+
handlerResult: { message: 'Handler error', stack: 'Stack trace' },
202+
stateToolId: 'tool-1',
203+
requestToolId: 'tool-1'
204+
},
205+
{
206+
description: 'handler attempting to return an error-like object, with name and single line stack',
207+
handlerResult: { name: 'Mock ERROR', message: 'Handler error', stack: 'Stack trace' },
208+
stateToolId: 'tool-1',
209+
requestToolId: 'tool-1'
210+
},
211+
{
212+
description: 'handler attempting to return an error-like object, with name and multiline line stack',
213+
handlerResult: { name: 'Mock', message: 'Handler error', stack: 'Stack trace\nSecond line' },
214+
stateToolId: 'tool-1',
215+
requestToolId: 'tool-1'
216+
},
217+
{
218+
description: 'handler attempting to return a DOMException-like object, with name, message and multiline line stack',
219+
handlerResult: { name: 'DOMException', message: 'Handler error', stack: 'DOMException: message\n at line x' },
220+
stateToolId: 'tool-1',
221+
requestToolId: 'tool-1'
222+
},
223+
{
224+
description: 'handler attempting to return a browser-like ErrorEvent-like object, with name, message and multiline line stack',
225+
handlerResult: { name: 'ErrorEvent', message: 'Handler error', stack: 'ErrorEvent: message\n at line x' },
226+
stateToolId: 'tool-1',
227+
requestToolId: 'tool-1'
228+
},
229+
{
230+
description: 'handler returning undefined',
231+
handlerResult: undefined,
232+
stateToolId: 'tool-1',
233+
requestToolId: 'tool-1'
234+
},
235+
{
236+
description: 'handler returning null',
237+
handlerResult: null,
238+
stateToolId: 'tool-1',
239+
requestToolId: 'tool-1'
186240
}
187241
])('should attempt tool invocation, $description', async ({ handlerResult, stateToolId, requestToolId }) => {
188242
const mockState = {

src/server.toolsHost.ts

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,45 @@ type NormalizeCreatorSchemaResult = {
9595
warnings: string[];
9696
};
9797

98+
/**
99+
* Check if a value is an error or an error-like object.
100+
*
101+
* Handles cross‑realm Error detection via tag checks for `[object Error]`, `[object AggregateError]`,
102+
* and `[object DOMException]`. Does not treat `[object ErrorEvent]` as error‑like in the
103+
* Node context; add if your runtime can emit `ErrorEvent`.
104+
*
105+
* @param value
106+
* @returns True if the value is an error-like object, false otherwise.
107+
*/
108+
const isErrorLike = (value: unknown) => {
109+
if (!value || (typeof value !== 'object' && typeof value !== 'function')) {
110+
return false;
111+
}
112+
113+
if (value instanceof Error || value instanceof AggregateError) {
114+
return true;
115+
}
116+
117+
const tag = Object.prototype.toString.call(value);
118+
119+
if (tag === '[object Error]' || tag === '[object AggregateError]' || tag === '[object DOMException]') {
120+
return true;
121+
}
122+
123+
const val = value as Record<string, unknown>;
124+
const has = (key: string) =>
125+
Object.hasOwn(val, key) && typeof val[key] === 'string' && val[key].length > 0;
126+
127+
if (!has('message')) {
128+
return false;
129+
}
130+
131+
const isNameLike = has('name') && (val.name as string).toLowerCase().endsWith('error');
132+
const isStackLike = has('stack') && (val.stack as string).includes('\n');
133+
134+
return isNameLike || isStackLike;
135+
};
136+
98137
/**
99138
* Normalize a tool creator function and its input schema.
100139
*
@@ -325,7 +364,8 @@ const requestInvoke = async (state: HostState, request: InvokeRequest) => {
325364
// Invoke the tool
326365
const result = await Promise.resolve(handler(updatedRequestArgs));
327366

328-
if (result instanceof Error) {
367+
// Some handlers may mistakenly return an Error instance instead of throwing. Normalize it to a failure.
368+
if (isErrorLike(result)) {
329369
const err: SerializedError = new Error('Internal error', { cause: { details: result } });
330370

331371
err.code = 'INTERNAL_ERROR';

0 commit comments

Comments
 (0)