From 9caee5151a09c191c6bbf5a0e7e16129c6728817 Mon Sep 17 00:00:00 2001 From: Brian Fox Date: Tue, 7 Jul 2026 12:08:42 +0200 Subject: [PATCH] fix(mcp-server): include agent error detail and status in tool errors Tool error results returned the generic 'Agent responded with HTTP {status}' instead of the JSON:API errors[0].detail. Route the tool-error catch through parseAgentError and append the HTTP status when a real detail is present, so the underlying cause reaches all MCP clients. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/mcp-server/src/utils/error-parser.ts | 7 +++- .../mcp-server/src/utils/tool-with-logging.ts | 4 +- .../test/utils/error-parser.test.ts | 14 +++++-- .../test/utils/tool-with-logging.test.ts | 41 +++++++++++++++++++ .../test/utils/with-activity-log.test.ts | 2 +- 5 files changed, 60 insertions(+), 8 deletions(-) diff --git a/packages/mcp-server/src/utils/error-parser.ts b/packages/mcp-server/src/utils/error-parser.ts index 512d1335f3..78f0a65916 100644 --- a/packages/mcp-server/src/utils/error-parser.ts +++ b/packages/mcp-server/src/utils/error-parser.ts @@ -17,10 +17,13 @@ function jsonApiDetail(body: unknown, text?: string): string | null { return null; } -// Turn an agent RPC error into a human-readable message (AgentHttpError detail, else raw message). +// Turn an agent RPC error into a human-readable message: the JSON:API detail with its HTTP status +// when one can be extracted, else the raw message (which already carries the status). export default function parseAgentError(error: unknown): string | null { if (error instanceof AgentHttpError) { - return jsonApiDetail(error.body, error.responseText) ?? error.message; + const detail = jsonApiDetail(error.body, error.responseText); + + return detail ? `${detail} (HTTP ${error.status})` : error.message; } if (error && typeof error === 'object' && 'message' in error) { diff --git a/packages/mcp-server/src/utils/tool-with-logging.ts b/packages/mcp-server/src/utils/tool-with-logging.ts index b6eeff47c4..a0eaa36aad 100644 --- a/packages/mcp-server/src/utils/tool-with-logging.ts +++ b/packages/mcp-server/src/utils/tool-with-logging.ts @@ -10,6 +10,8 @@ import type { import { z } from 'zod'; +import parseAgentError from './error-parser'; + // ----------------------------------------------------------------------------- // Types // ----------------------------------------------------------------------------- @@ -114,7 +116,7 @@ export default function registerToolWithLogging< let message: string; try { - message = error instanceof Error ? error.message : JSON.stringify(error) ?? String(error); + message = parseAgentError(error) ?? JSON.stringify(error) ?? String(error); } catch { message = String(error); } diff --git a/packages/mcp-server/test/utils/error-parser.test.ts b/packages/mcp-server/test/utils/error-parser.test.ts index 45907b03a3..bc9b1a2400 100644 --- a/packages/mcp-server/test/utils/error-parser.test.ts +++ b/packages/mcp-server/test/utils/error-parser.test.ts @@ -3,22 +3,22 @@ import { AgentHttpError } from '@forestadmin/agent-client'; import parseAgentError from '../../src/utils/error-parser'; describe('parseAgentError', () => { - it('extracts the JSON:API detail from the parsed body', () => { + it('appends the HTTP status to the JSON:API detail from the parsed body', () => { const error = new AgentHttpError(400, { errors: [{ name: 'ValidationError', detail: 'Invalid filters provided' }], }); - expect(parseAgentError(error)).toBe('Invalid filters provided'); + expect(parseAgentError(error)).toBe('Invalid filters provided (HTTP 400)'); }); - it('falls back to parsing responseText when the body is not a parsed object', () => { + it('appends the HTTP status to the JSON:API detail parsed from responseText', () => { const error = new AgentHttpError( 400, undefined, JSON.stringify({ errors: [{ detail: 'From raw text' }] }), ); - expect(parseAgentError(error)).toBe('From raw text'); + expect(parseAgentError(error)).toBe('From raw text (HTTP 400)'); }); it('returns the generic HTTP message when the body has no JSON:API detail', () => { @@ -33,6 +33,12 @@ describe('parseAgentError', () => { ); }); + it('does not append a second HTTP status when the body is not JSON:API', () => { + expect(parseAgentError(new AgentHttpError(500, 'Internal Server Error'))).toBe( + 'Agent responded with HTTP 500', + ); + }); + it('returns the message of a plain Error', () => { expect(parseAgentError(new Error('Plain error message'))).toBe('Plain error 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 00113e3297..01e2cd00fa 100644 --- a/packages/mcp-server/test/utils/tool-with-logging.test.ts +++ b/packages/mcp-server/test/utils/tool-with-logging.test.ts @@ -1,3 +1,4 @@ +import { AgentHttpError } from '@forestadmin/agent-client'; import { z } from 'zod'; import registerToolWithLogging from '../../src/utils/tool-with-logging'; @@ -173,6 +174,46 @@ describe('registerToolWithLogging', () => { }); }); + it('should surface the agent error detail and HTTP status for an AgentHttpError', async () => { + const error = new AgentHttpError(422, { + errors: [{ name: 'ValidationError', detail: 'The value "reason" is required' }], + }); + const handler = jest.fn().mockRejectedValue(error); + + registerToolWithLogging(mockMcpServer as never, 'test-tool', toolConfig, handler, mockLogger); + + const result = await registeredHandler({ name: 'test', count: 42 }, {}); + expect(result).toEqual({ + content: [{ type: 'text', text: 'The value "reason" is required (HTTP 422)' }], + isError: true, + }); + }); + + it('should return the generic HTTP message without a doubled status when the body is not JSON:API', async () => { + const error = new AgentHttpError(500, 'Internal Server Error'); + const handler = jest.fn().mockRejectedValue(error); + + registerToolWithLogging(mockMcpServer as never, 'test-tool', toolConfig, handler, mockLogger); + + const result = await registeredHandler({ name: 'test', count: 42 }, {}); + expect(result).toEqual({ + content: [{ type: 'text', text: 'Agent responded with HTTP 500' }], + isError: true, + }); + }); + + it('should use the message of a thrown non-Error object that has one', async () => { + const handler = jest.fn().mockRejectedValue({ message: 'boom', code: 'X' }); + + registerToolWithLogging(mockMcpServer as never, 'test-tool', toolConfig, handler, mockLogger); + + const result = await registeredHandler({ name: 'test', count: 42 }, {}); + expect(result).toEqual({ + content: [{ type: 'text', text: 'boom' }], + isError: true, + }); + }); + it('should call handler even when validation fails', async () => { const handler = jest.fn().mockResolvedValue({ content: [{ type: 'text', text: 'ok' }] }); diff --git a/packages/mcp-server/test/utils/with-activity-log.test.ts b/packages/mcp-server/test/utils/with-activity-log.test.ts index 0a0b29cdd0..338325ce12 100644 --- a/packages/mcp-server/test/utils/with-activity-log.test.ts +++ b/packages/mcp-server/test/utils/with-activity-log.test.ts @@ -298,7 +298,7 @@ describe('withActivityLog', () => { }), ).rejects.toThrow('Context: Invalid field value'); - expect(errorEnhancer).toHaveBeenCalledWith('Invalid field value', agentError); + expect(errorEnhancer).toHaveBeenCalledWith('Invalid field value (HTTP 400)', agentError); }); }); });