From 91ca2944195d65fb5e1b4894a25a38bff044a13f Mon Sep 17 00:00:00 2001 From: alban bertolini Date: Tue, 10 Feb 2026 15:18:18 +0100 Subject: [PATCH 1/4] fix(mcp-server): return tool errors as isError results per MCP spec Per the MCP specification, tool execution errors should be reported within the result object (isError: true) rather than thrown as protocol-level errors. This ensures the LLM receives the actual error message and can handle it appropriately. See: https://modelcontextprotocol.io/docs/concepts/tools Co-Authored-By: Claude Opus 4.6 --- .../mcp-server/src/utils/tool-with-logging.ts | 19 ++- packages/mcp-server/test/tools/create.test.ts | 24 +++- packages/mcp-server/test/tools/delete.test.ts | 26 +++- .../test/tools/describe-collection.test.ts | 10 +- .../test/tools/execute-action.test.ts | 35 +++-- .../test/tools/get-action-form.test.ts | 38 ++--- .../test/tools/list-related.test.ts | 135 +++++++++++------- packages/mcp-server/test/tools/list.test.ts | 57 +++++--- packages/mcp-server/test/tools/update.test.ts | 30 ++-- .../test/utils/tool-with-logging.test.ts | 10 +- 10 files changed, 246 insertions(+), 138 deletions(-) diff --git a/packages/mcp-server/src/utils/tool-with-logging.ts b/packages/mcp-server/src/utils/tool-with-logging.ts index c0f7d3ce58..b2a1770663 100644 --- a/packages/mcp-server/src/utils/tool-with-logging.ts +++ b/packages/mcp-server/src/utils/tool-with-logging.ts @@ -60,8 +60,8 @@ function logValidationErrorsIfAny( * This wrapper logs validation errors with detailed field information, * which helps debug tool calls when clients send invalid arguments. * - * Note: Execution errors are logged by the SSE response interceptor in server.ts, - * so we don't duplicate that logging here. + * Note: Execution errors are caught and converted to { isError: true } tool results. + * The SSE response interceptor in server.ts additionally logs these errors from the stream. * * @example * registerToolWithLogging( @@ -99,7 +99,20 @@ export default function registerToolWithLogging< (async (args: any, extra: any) => { logValidationErrorsIfAny(args, schema, toolName, logger); - return handler(args as TArgs, extra); + // Return errors as tool results (isError: true) instead of throwing. + // Per MCP spec, tool errors should be reported within the result object, + // not as protocol-level errors, so the LLM can see and handle them. + // See: https://modelcontextprotocol.io/docs/concepts/tools + try { + return await handler(args as TArgs, extra); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + + return { + content: [{ type: 'text', text: message }], + isError: true, + }; + } // eslint-disable-next-line @typescript-eslint/no-explicit-any }) as any, ); diff --git a/packages/mcp-server/test/tools/create.test.ts b/packages/mcp-server/test/tools/create.test.ts index 86d12576f3..c8b2ddcadb 100644 --- a/packages/mcp-server/test/tools/create.test.ts +++ b/packages/mcp-server/test/tools/create.test.ts @@ -264,12 +264,17 @@ describe('declareCreateTool', () => { authData: { userId: 1, renderingId: '123', environmentId: 1, projectId: 1 }, } as unknown as ReturnType); - await expect( - registeredToolHandler({ collectionName: 'users', attributes: {} }, mockExtra), - ).rejects.toThrow('Name is required'); + const result = await registeredToolHandler( + { collectionName: 'users', attributes: {} }, + mockExtra, + ); + expect(result).toEqual({ + content: [{ type: 'text', text: expect.stringContaining('Name is required') }], + isError: true, + }); }); - it('should rethrow original error when no parsable error found', async () => { + it('should return error result when no parsable error found', async () => { const agentError = { unknownProperty: 'some value' }; const mockCreate = jest.fn().mockRejectedValue(agentError); const mockCollection = jest.fn().mockReturnValue({ create: mockCreate }); @@ -278,9 +283,14 @@ describe('declareCreateTool', () => { authData: { userId: 1, renderingId: '123', environmentId: 1, projectId: 1 }, } as unknown as ReturnType); - await expect( - registeredToolHandler({ collectionName: 'users', attributes: {} }, mockExtra), - ).rejects.toEqual(agentError); + const result = await registeredToolHandler( + { collectionName: 'users', attributes: {} }, + mockExtra, + ); + expect(result).toEqual({ + content: [{ type: 'text', text: expect.any(String) }], + isError: true, + }); }); }); }); diff --git a/packages/mcp-server/test/tools/delete.test.ts b/packages/mcp-server/test/tools/delete.test.ts index 84ba6d5bfd..3ddee705e3 100644 --- a/packages/mcp-server/test/tools/delete.test.ts +++ b/packages/mcp-server/test/tools/delete.test.ts @@ -252,12 +252,19 @@ describe('declareDeleteTool', () => { authData: { userId: 1, renderingId: '123', environmentId: 1, projectId: 1 }, } as unknown as ReturnType); - await expect( - registeredToolHandler({ collectionName: 'users', recordIds: [1] }, mockExtra), - ).rejects.toThrow('Cannot delete protected records'); + const result = await registeredToolHandler( + { collectionName: 'users', recordIds: [1] }, + mockExtra, + ); + expect(result).toEqual({ + content: [ + { type: 'text', text: expect.stringContaining('Cannot delete protected records') }, + ], + isError: true, + }); }); - it('should rethrow original error when no parsable error found', async () => { + it('should return error result when no parsable error found', async () => { const agentError = { unknownProperty: 'some value' }; const mockDelete = jest.fn().mockRejectedValue(agentError); const mockCollection = jest.fn().mockReturnValue({ delete: mockDelete }); @@ -266,9 +273,14 @@ describe('declareDeleteTool', () => { authData: { userId: 1, renderingId: '123', environmentId: 1, projectId: 1 }, } as unknown as ReturnType); - await expect( - registeredToolHandler({ collectionName: 'users', recordIds: [1] }, mockExtra), - ).rejects.toEqual(agentError); + const result = await registeredToolHandler( + { collectionName: 'users', recordIds: [1] }, + mockExtra, + ); + expect(result).toEqual({ + content: [{ type: 'text', text: expect.any(String) }], + isError: true, + }); }); }); }); diff --git a/packages/mcp-server/test/tools/describe-collection.test.ts b/packages/mcp-server/test/tools/describe-collection.test.ts index de2c8ce63d..da54adb950 100644 --- a/packages/mcp-server/test/tools/describe-collection.test.ts +++ b/packages/mcp-server/test/tools/describe-collection.test.ts @@ -342,7 +342,7 @@ describe('declareDescribeCollectionTool', () => { ); }); - it('should throw and log Error for non-404 capabilities errors', async () => { + it('should return error result and log Error for non-404 capabilities errors', async () => { const mockCapabilities = jest.fn().mockRejectedValue(new Error('Server error')); const mockCollection = jest.fn().mockReturnValue({ capabilities: mockCapabilities }); mockBuildClient.mockReturnValue({ @@ -353,9 +353,11 @@ describe('declareDescribeCollectionTool', () => { mockFetchForestSchema.mockResolvedValue({ collections: [] }); mockGetFieldsOfCollection.mockReturnValue([]); - await expect(registeredToolHandler({ collectionName: 'users' }, mockExtra)).rejects.toThrow( - 'Server error', - ); + const result = await registeredToolHandler({ collectionName: 'users' }, mockExtra); + expect(result).toEqual({ + content: [{ type: 'text', text: expect.stringContaining('Server error') }], + isError: true, + }); expect(mockLogger).toHaveBeenCalledWith( 'Error', diff --git a/packages/mcp-server/test/tools/execute-action.test.ts b/packages/mcp-server/test/tools/execute-action.test.ts index 7db07fc13d..a013ba1a46 100644 --- a/packages/mcp-server/test/tools/execute-action.test.ts +++ b/packages/mcp-server/test/tools/execute-action.test.ts @@ -417,15 +417,22 @@ describe('declareExecuteActionTool', () => { authData: { userId: 1, renderingId: '123', environmentId: 1, projectId: 1 }, } as unknown as ReturnType); - await expect( - registeredToolHandler( - { collectionName: 'users', actionName: 'sendEmail', recordIds: [1] }, - mockExtra, - ), - ).rejects.toThrow('Cannot execute action on these records'); + const result = await registeredToolHandler( + { collectionName: 'users', actionName: 'sendEmail', recordIds: [1] }, + mockExtra, + ); + expect(result).toEqual({ + content: [ + { + type: 'text', + text: expect.stringContaining('Cannot execute action on these records'), + }, + ], + isError: true, + }); }); - it('should rethrow original error when no parsable error found', async () => { + it('should return error result when no parsable error found', async () => { const agentError = { unknownProperty: 'some value' }; const mockAction = jest.fn().mockRejectedValue(agentError); const mockCollection = jest.fn().mockReturnValue({ action: mockAction }); @@ -434,12 +441,14 @@ describe('declareExecuteActionTool', () => { authData: { userId: 1, renderingId: '123', environmentId: 1, projectId: 1 }, } as unknown as ReturnType); - await expect( - registeredToolHandler( - { collectionName: 'users', actionName: 'sendEmail', recordIds: [1] }, - mockExtra, - ), - ).rejects.toEqual(agentError); + const result = await registeredToolHandler( + { collectionName: 'users', actionName: 'sendEmail', recordIds: [1] }, + mockExtra, + ); + expect(result).toEqual({ + content: [{ type: 'text', text: expect.any(String) }], + isError: true, + }); }); }); }); diff --git a/packages/mcp-server/test/tools/get-action-form.test.ts b/packages/mcp-server/test/tools/get-action-form.test.ts index 95e8f0ae90..b48ad99b86 100644 --- a/packages/mcp-server/test/tools/get-action-form.test.ts +++ b/packages/mcp-server/test/tools/get-action-form.test.ts @@ -729,12 +729,14 @@ describe('declareGetActionFormTool', () => { authData: { userId: 1, renderingId: '123', environmentId: 1, projectId: 1 }, } as unknown as ReturnType); - await expect( - registeredToolHandler( - { collectionName: 'users', actionName: 'nonExistent', recordIds: [1] }, - mockExtra, - ), - ).rejects.toThrow('Action not found'); + const result = await registeredToolHandler( + { collectionName: 'users', actionName: 'nonExistent', recordIds: [1] }, + mockExtra, + ); + expect(result).toEqual({ + content: [{ type: 'text', text: expect.stringContaining('Action not found') }], + isError: true, + }); }); it('should propagate errors from tryToSetFields call', async () => { @@ -751,17 +753,19 @@ describe('declareGetActionFormTool', () => { authData: { userId: 1, renderingId: '123', environmentId: 1, projectId: 1 }, } as unknown as ReturnType); - await expect( - registeredToolHandler( - { - collectionName: 'users', - actionName: 'sendEmail', - recordIds: [1], - values: { invalidField: 'value' }, - }, - mockExtra, - ), - ).rejects.toThrow('Invalid field value'); + const result = await registeredToolHandler( + { + collectionName: 'users', + actionName: 'sendEmail', + recordIds: [1], + values: { invalidField: 'value' }, + }, + mockExtra, + ); + expect(result).toEqual({ + content: [{ type: 'text', text: expect.stringContaining('Invalid field value') }], + isError: true, + }); }); }); }); diff --git a/packages/mcp-server/test/tools/list-related.test.ts b/packages/mcp-server/test/tools/list-related.test.ts index 026abbba6e..9ca2fcf0c6 100644 --- a/packages/mcp-server/test/tools/list-related.test.ts +++ b/packages/mcp-server/test/tools/list-related.test.ts @@ -610,12 +610,14 @@ describe('declareListRelatedTool', () => { mockFetchForestSchema.mockResolvedValue({ collections: [] }); mockGetFieldsOfCollection.mockReturnValue([]); - await expect( - registeredToolHandler( - { collectionName: 'users', relationName: 'orders', parentRecordId: 1 }, - mockExtra, - ), - ).rejects.toThrow('Invalid filters provided'); + const result = await registeredToolHandler( + { collectionName: 'users', relationName: 'orders', parentRecordId: 1 }, + mockExtra, + ); + expect(result).toEqual({ + content: [{ type: 'text', text: expect.stringContaining('Invalid filters provided') }], + isError: true, + }); }); it('should parse error with direct text property in message', async () => { @@ -630,12 +632,14 @@ describe('declareListRelatedTool', () => { mockFetchForestSchema.mockResolvedValue({ collections: [] }); mockGetFieldsOfCollection.mockReturnValue([]); - await expect( - registeredToolHandler( - { collectionName: 'users', relationName: 'orders', parentRecordId: 1 }, - mockExtra, - ), - ).rejects.toThrow('Direct text error'); + const result = await registeredToolHandler( + { collectionName: 'users', relationName: 'orders', parentRecordId: 1 }, + mockExtra, + ); + expect(result).toEqual({ + content: [{ type: 'text', text: expect.stringContaining('Direct text error') }], + isError: true, + }); }); it('should provide helpful error message for Invalid sort errors', async () => { @@ -688,14 +692,21 @@ describe('declareListRelatedTool', () => { mockFetchForestSchema.mockResolvedValue(mockSchema); mockGetFieldsOfCollection.mockReturnValue(mockFields); - await expect( - registeredToolHandler( - { collectionName: 'users', relationName: 'orders', parentRecordId: 1 }, - mockExtra, - ), - ).rejects.toThrow( - 'The sort field provided is invalid for this collection. Available fields for the collection users are: id, total.', + const result = await registeredToolHandler( + { collectionName: 'users', relationName: 'orders', parentRecordId: 1 }, + mockExtra, ); + expect(result).toEqual({ + content: [ + { + type: 'text', + text: expect.stringContaining( + 'The sort field provided is invalid for this collection. Available fields for the collection users are: id, total.', + ), + }, + ], + isError: true, + }); expect(mockFetchForestSchema).toHaveBeenCalledWith(mockForestServerClient); expect(mockGetFieldsOfCollection).toHaveBeenCalledWith(mockSchema, 'users'); @@ -746,14 +757,21 @@ describe('declareListRelatedTool', () => { mockFetchForestSchema.mockResolvedValue(mockSchema); mockGetFieldsOfCollection.mockReturnValue(mockFields); - await expect( - registeredToolHandler( - { collectionName: 'users', relationName: 'invalidRelation', parentRecordId: 1 }, - mockExtra, - ), - ).rejects.toThrow( - 'The relation name provided is invalid for this collection. Available relations for collection users are: orders, reviews.', + const result = await registeredToolHandler( + { collectionName: 'users', relationName: 'invalidRelation', parentRecordId: 1 }, + mockExtra, ); + expect(result).toEqual({ + content: [ + { + type: 'text', + text: expect.stringContaining( + 'The relation name provided is invalid for this collection. Available relations for collection users are: orders, reviews.', + ), + }, + ], + isError: true, + }); }); it('should include BelongsToMany relations in available relations error message', async () => { @@ -790,14 +808,21 @@ describe('declareListRelatedTool', () => { mockFetchForestSchema.mockResolvedValue(mockSchema); mockGetFieldsOfCollection.mockReturnValue(mockFields); - await expect( - registeredToolHandler( - { collectionName: 'users', relationName: 'invalidRelation', parentRecordId: 1 }, - mockExtra, - ), - ).rejects.toThrow( - 'The relation name provided is invalid for this collection. Available relations for collection users are: orders, tags.', + const result = await registeredToolHandler( + { collectionName: 'users', relationName: 'invalidRelation', parentRecordId: 1 }, + mockExtra, ); + expect(result).toEqual({ + content: [ + { + type: 'text', + text: expect.stringContaining( + 'The relation name provided is invalid for this collection. Available relations for collection users are: orders, tags.', + ), + }, + ], + isError: true, + }); }); it('should not show relation error when relation exists but error is different', async () => { @@ -823,13 +848,15 @@ describe('declareListRelatedTool', () => { mockFetchForestSchema.mockResolvedValue(mockSchema); mockGetFieldsOfCollection.mockReturnValue(mockFields); - // Should throw the original error message since 'orders' relation exists - await expect( - registeredToolHandler( - { collectionName: 'users', relationName: 'orders', parentRecordId: 1 }, - mockExtra, - ), - ).rejects.toThrow('Some other error'); + // Should return the original error message since 'orders' relation exists + const result = await registeredToolHandler( + { collectionName: 'users', relationName: 'orders', parentRecordId: 1 }, + mockExtra, + ); + expect(result).toEqual({ + content: [{ type: 'text', text: expect.stringContaining('Some other error') }], + isError: true, + }); }); it('should fall back to error.message when message is not valid JSON', async () => { @@ -854,12 +881,14 @@ describe('declareListRelatedTool', () => { }); mockGetFieldsOfCollection.mockReturnValue(mockFields); - await expect( - registeredToolHandler( - { collectionName: 'users', relationName: 'orders', parentRecordId: 1 }, - mockExtra, - ), - ).rejects.toThrow('Plain error message'); + const result = await registeredToolHandler( + { collectionName: 'users', relationName: 'orders', parentRecordId: 1 }, + mockExtra, + ); + expect(result).toEqual({ + content: [{ type: 'text', text: expect.stringContaining('Plain error message') }], + isError: true, + }); }); it('should handle string errors thrown directly', async () => { @@ -884,12 +913,14 @@ describe('declareListRelatedTool', () => { }); mockGetFieldsOfCollection.mockReturnValue(mockFields); - await expect( - registeredToolHandler( - { collectionName: 'users', relationName: 'orders', parentRecordId: 1 }, - mockExtra, - ), - ).rejects.toThrow('Connection failed'); + const result = await registeredToolHandler( + { collectionName: 'users', relationName: 'orders', parentRecordId: 1 }, + mockExtra, + ); + expect(result).toEqual({ + content: [{ type: 'text', text: expect.stringContaining('Connection failed') }], + isError: true, + }); }); }); }); diff --git a/packages/mcp-server/test/tools/list.test.ts b/packages/mcp-server/test/tools/list.test.ts index efd0a7f3d1..a9f64d6b31 100644 --- a/packages/mcp-server/test/tools/list.test.ts +++ b/packages/mcp-server/test/tools/list.test.ts @@ -907,9 +907,11 @@ describe('declareListTool', () => { const agentError = new Error(JSON.stringify(errorPayload)); mockList.mockRejectedValue(agentError); - await expect(registeredToolHandler({ collectionName: 'users' }, mockExtra)).rejects.toThrow( - 'Invalid filters provided', - ); + const result = await registeredToolHandler({ collectionName: 'users' }, mockExtra); + expect(result).toEqual({ + content: [{ type: 'text', text: expect.stringContaining('Invalid filters provided') }], + isError: true, + }); }); it('should parse error with direct text property in message', async () => { @@ -922,9 +924,11 @@ describe('declareListTool', () => { const agentError = new Error(JSON.stringify(errorPayload)); mockList.mockRejectedValue(agentError); - await expect(registeredToolHandler({ collectionName: 'users' }, mockExtra)).rejects.toThrow( - 'Direct text error', - ); + const result = await registeredToolHandler({ collectionName: 'users' }, mockExtra); + expect(result).toEqual({ + content: [{ type: 'text', text: expect.stringContaining('Direct text error') }], + isError: true, + }); }); it('should use message property from parsed JSON when no text field', async () => { @@ -935,9 +939,13 @@ describe('declareListTool', () => { const agentError = new Error(JSON.stringify(errorPayload)); mockList.mockRejectedValue(agentError); - await expect(registeredToolHandler({ collectionName: 'users' }, mockExtra)).rejects.toThrow( - 'Error message from JSON payload', - ); + const result = await registeredToolHandler({ collectionName: 'users' }, mockExtra); + expect(result).toEqual({ + content: [ + { type: 'text', text: expect.stringContaining('Error message from JSON payload') }, + ], + isError: true, + }); }); it('should fall back to error.message when message is not valid JSON', async () => { @@ -945,18 +953,22 @@ describe('declareListTool', () => { const agentError = new Error('Plain error message'); mockList.mockRejectedValue(agentError); - await expect(registeredToolHandler({ collectionName: 'users' }, mockExtra)).rejects.toThrow( - 'Plain error message', - ); + const result = await registeredToolHandler({ collectionName: 'users' }, mockExtra); + expect(result).toEqual({ + content: [{ type: 'text', text: expect.stringContaining('Plain error message') }], + isError: true, + }); }); it('should handle string errors thrown directly', async () => { // Some libraries throw string errors directly mockList.mockRejectedValue('Connection failed'); - await expect(registeredToolHandler({ collectionName: 'users' }, mockExtra)).rejects.toThrow( - 'Connection failed', - ); + const result = await registeredToolHandler({ collectionName: 'users' }, mockExtra); + expect(result).toEqual({ + content: [{ type: 'text', text: expect.stringContaining('Connection failed') }], + isError: true, + }); }); it('should provide helpful error message for Invalid sort errors', async () => { @@ -1021,9 +1033,18 @@ describe('declareListTool', () => { mockFetchForestSchema.mockResolvedValue(mockSchema); mockGetFieldsOfCollection.mockReturnValue(mockFields); - await expect(registeredToolHandler({ collectionName: 'users' }, mockExtra)).rejects.toThrow( - 'The sort field provided is invalid for this collection. Available fields for the collection users are: id, name, email.', - ); + const result = await registeredToolHandler({ collectionName: 'users' }, mockExtra); + expect(result).toEqual({ + content: [ + { + type: 'text', + text: expect.stringContaining( + 'The sort field provided is invalid for this collection. Available fields for the collection users are: id, name, email.', + ), + }, + ], + isError: true, + }); expect(mockFetchForestSchema).toHaveBeenCalledWith(mockForestServerClient); expect(mockGetFieldsOfCollection).toHaveBeenCalledWith(mockSchema, 'users'); diff --git a/packages/mcp-server/test/tools/update.test.ts b/packages/mcp-server/test/tools/update.test.ts index 4e71aae5ef..e6c114e6e5 100644 --- a/packages/mcp-server/test/tools/update.test.ts +++ b/packages/mcp-server/test/tools/update.test.ts @@ -279,15 +279,17 @@ describe('declareUpdateTool', () => { authData: { userId: 1, renderingId: '123', environmentId: 1, projectId: 1 }, } as unknown as ReturnType); - await expect( - registeredToolHandler( - { collectionName: 'users', recordId: 999, attributes: {} }, - mockExtra, - ), - ).rejects.toThrow('Record not found'); + const result = await registeredToolHandler( + { collectionName: 'users', recordId: 999, attributes: {} }, + mockExtra, + ); + expect(result).toEqual({ + content: [{ type: 'text', text: expect.stringContaining('Record not found') }], + isError: true, + }); }); - it('should rethrow original error when no parsable error found', async () => { + it('should return error result when no parsable error found', async () => { const agentError = { unknownProperty: 'some value' }; const mockUpdate = jest.fn().mockRejectedValue(agentError); const mockCollection = jest.fn().mockReturnValue({ update: mockUpdate }); @@ -296,12 +298,14 @@ describe('declareUpdateTool', () => { authData: { userId: 1, renderingId: '123', environmentId: 1, projectId: 1 }, } as unknown as ReturnType); - await expect( - registeredToolHandler( - { collectionName: 'users', recordId: 1, attributes: {} }, - mockExtra, - ), - ).rejects.toEqual(agentError); + const result = await registeredToolHandler( + { collectionName: 'users', recordId: 1, attributes: {} }, + mockExtra, + ); + expect(result).toEqual({ + content: [{ type: 'text', text: expect.any(String) }], + isError: true, + }); }); }); }); diff --git a/packages/mcp-server/test/utils/tool-with-logging.test.ts b/packages/mcp-server/test/utils/tool-with-logging.test.ts index bbf140ec59..1d9f4aa8fb 100644 --- a/packages/mcp-server/test/utils/tool-with-logging.test.ts +++ b/packages/mcp-server/test/utils/tool-with-logging.test.ts @@ -121,15 +121,17 @@ describe('registerToolWithLogging', () => { expect(result).toBe(expectedResult); }); - it('should propagate handler errors without logging', async () => { + it('should catch handler errors and return isError result without logging', async () => { const error = new Error('Handler failed'); const handler = jest.fn().mockRejectedValue(error); registerToolWithLogging(mockMcpServer as never, 'test-tool', toolConfig, handler, mockLogger); - await expect(registeredHandler({ name: 'test', count: 42 }, {})).rejects.toThrow( - 'Handler failed', - ); + const result = await registeredHandler({ name: 'test', count: 42 }, {}); + expect(result).toEqual({ + content: [{ type: 'text', text: expect.stringContaining('Handler failed') }], + isError: true, + }); expect(mockLogger).not.toHaveBeenCalled(); }); From f611c920ca5ffd8932d87daa269107aed0d8c343 Mon Sep 17 00:00:00 2001 From: alban bertolini Date: Wed, 11 Feb 2026 15:44:26 +0100 Subject: [PATCH 2/4] fix(mcp-server): use JSON.stringify for non-Error throws in tool handler Co-Authored-By: Claude Opus 4.6 --- packages/mcp-server/src/utils/tool-with-logging.ts | 2 +- .../mcp-server/test/utils/tool-with-logging.test.ts | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/packages/mcp-server/src/utils/tool-with-logging.ts b/packages/mcp-server/src/utils/tool-with-logging.ts index b2a1770663..11c6b7954e 100644 --- a/packages/mcp-server/src/utils/tool-with-logging.ts +++ b/packages/mcp-server/src/utils/tool-with-logging.ts @@ -106,7 +106,7 @@ export default function registerToolWithLogging< try { return await handler(args as TArgs, extra); } catch (error) { - const message = error instanceof Error ? error.message : String(error); + const message = error instanceof Error ? error.message : JSON.stringify(error); return { content: [{ type: 'text', text: message }], diff --git a/packages/mcp-server/test/utils/tool-with-logging.test.ts b/packages/mcp-server/test/utils/tool-with-logging.test.ts index 1d9f4aa8fb..cc6abd0d9f 100644 --- a/packages/mcp-server/test/utils/tool-with-logging.test.ts +++ b/packages/mcp-server/test/utils/tool-with-logging.test.ts @@ -135,6 +135,18 @@ describe('registerToolWithLogging', () => { expect(mockLogger).not.toHaveBeenCalled(); }); + it('should stringify non-Error throws in isError result', async () => { + const handler = jest.fn().mockRejectedValue({ code: 'FAIL', detail: 'something broke' }); + + registerToolWithLogging(mockMcpServer as never, 'test-tool', toolConfig, handler, mockLogger); + + const result = await registeredHandler({ name: 'test', count: 42 }, {}); + expect(result).toEqual({ + content: [{ type: 'text', text: '{"code":"FAIL","detail":"something broke"}' }], + isError: true, + }); + }); + it('should call handler even when validation fails', async () => { const handler = jest.fn().mockResolvedValue({ content: [{ type: 'text', text: 'ok' }] }); From 3be6265e0590857f7b40c4753a7f3d8a8cea1b01 Mon Sep 17 00:00:00 2001 From: alban bertolini Date: Wed, 11 Feb 2026 16:00:18 +0100 Subject: [PATCH 3/4] fix(mcp-server): handle JSON.stringify edge cases in error catch JSON.stringify returns undefined for undefined/Symbol/Function values, and throws on circular references. Add fallback to String() for both cases to ensure error message is always a string. Co-Authored-By: Claude Opus 4.6 --- .../mcp-server/src/utils/tool-with-logging.ts | 8 +++++- .../test/utils/tool-with-logging.test.ts | 26 +++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/packages/mcp-server/src/utils/tool-with-logging.ts b/packages/mcp-server/src/utils/tool-with-logging.ts index 11c6b7954e..94cdaa063c 100644 --- a/packages/mcp-server/src/utils/tool-with-logging.ts +++ b/packages/mcp-server/src/utils/tool-with-logging.ts @@ -106,7 +106,13 @@ export default function registerToolWithLogging< try { return await handler(args as TArgs, extra); } catch (error) { - const message = error instanceof Error ? error.message : JSON.stringify(error); + let message: string; + + try { + message = error instanceof Error ? error.message : JSON.stringify(error) ?? String(error); + } catch { + message = String(error); + } return { content: [{ type: 'text', text: message }], diff --git a/packages/mcp-server/test/utils/tool-with-logging.test.ts b/packages/mcp-server/test/utils/tool-with-logging.test.ts index cc6abd0d9f..00113e3297 100644 --- a/packages/mcp-server/test/utils/tool-with-logging.test.ts +++ b/packages/mcp-server/test/utils/tool-with-logging.test.ts @@ -147,6 +147,32 @@ describe('registerToolWithLogging', () => { }); }); + it('should fallback to String() when JSON.stringify returns undefined', async () => { + const handler = jest.fn().mockRejectedValue(undefined); + + registerToolWithLogging(mockMcpServer as never, 'test-tool', toolConfig, handler, mockLogger); + + const result = await registeredHandler({ name: 'test', count: 42 }, {}); + expect(result).toEqual({ + content: [{ type: 'text', text: 'undefined' }], + isError: true, + }); + }); + + it('should fallback to String() when thrown value has circular references', async () => { + const circular: Record = { name: 'loop' }; + circular.self = circular; + const handler = jest.fn().mockRejectedValue(circular); + + registerToolWithLogging(mockMcpServer as never, 'test-tool', toolConfig, handler, mockLogger); + + const result = await registeredHandler({ name: 'test', count: 42 }, {}); + expect(result).toEqual({ + content: [{ type: 'text', text: '[object Object]' }], + isError: true, + }); + }); + it('should call handler even when validation fails', async () => { const handler = jest.fn().mockResolvedValue({ content: [{ type: 'text', text: 'ok' }] }); From d8be9b9ce30dfa83a79f166d6df1ce78742cae04 Mon Sep 17 00:00:00 2001 From: alban bertolini Date: Wed, 11 Feb 2026 16:12:09 +0100 Subject: [PATCH 4/4] test(mcp-server): strengthen weak assertions in non-parsable error tests Replace expect.any(String) with exact expected value since the output is deterministic (JSON.stringify of the thrown object). Co-Authored-By: Claude Opus 4.6 --- packages/mcp-server/test/tools/create.test.ts | 2 +- packages/mcp-server/test/tools/delete.test.ts | 2 +- packages/mcp-server/test/tools/execute-action.test.ts | 2 +- packages/mcp-server/test/tools/update.test.ts | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/mcp-server/test/tools/create.test.ts b/packages/mcp-server/test/tools/create.test.ts index c8b2ddcadb..f56ca77095 100644 --- a/packages/mcp-server/test/tools/create.test.ts +++ b/packages/mcp-server/test/tools/create.test.ts @@ -288,7 +288,7 @@ describe('declareCreateTool', () => { mockExtra, ); expect(result).toEqual({ - content: [{ type: 'text', text: expect.any(String) }], + content: [{ type: 'text', text: '{"unknownProperty":"some value"}' }], isError: true, }); }); diff --git a/packages/mcp-server/test/tools/delete.test.ts b/packages/mcp-server/test/tools/delete.test.ts index 3ddee705e3..79e5a03e03 100644 --- a/packages/mcp-server/test/tools/delete.test.ts +++ b/packages/mcp-server/test/tools/delete.test.ts @@ -278,7 +278,7 @@ describe('declareDeleteTool', () => { mockExtra, ); expect(result).toEqual({ - content: [{ type: 'text', text: expect.any(String) }], + content: [{ type: 'text', text: '{"unknownProperty":"some value"}' }], isError: true, }); }); diff --git a/packages/mcp-server/test/tools/execute-action.test.ts b/packages/mcp-server/test/tools/execute-action.test.ts index a013ba1a46..c883c572c4 100644 --- a/packages/mcp-server/test/tools/execute-action.test.ts +++ b/packages/mcp-server/test/tools/execute-action.test.ts @@ -446,7 +446,7 @@ describe('declareExecuteActionTool', () => { mockExtra, ); expect(result).toEqual({ - content: [{ type: 'text', text: expect.any(String) }], + content: [{ type: 'text', text: '{"unknownProperty":"some value"}' }], isError: true, }); }); diff --git a/packages/mcp-server/test/tools/update.test.ts b/packages/mcp-server/test/tools/update.test.ts index e6c114e6e5..ada9d276cd 100644 --- a/packages/mcp-server/test/tools/update.test.ts +++ b/packages/mcp-server/test/tools/update.test.ts @@ -303,7 +303,7 @@ describe('declareUpdateTool', () => { mockExtra, ); expect(result).toEqual({ - content: [{ type: 'text', text: expect.any(String) }], + content: [{ type: 'text', text: '{"unknownProperty":"some value"}' }], isError: true, }); });