From 2771bb2d743fb09746aec09b18e16f81f7dbc8a5 Mon Sep 17 00:00:00 2001 From: johnxie Date: Sat, 27 Jun 2026 02:56:57 -0700 Subject: [PATCH] fix(runtime): surface API errors instead of returning them as successful results MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The runtime returned `await response.json()` with no status check, so a non-2xx response (401/403/422/5xx) was handed back as a successful tool result — the model saw an error payload as if the call succeeded (worst for promptAgent, whose normalizer says "this is the agent's reply"). Now: gate on `response.ok`, throw on non-2xx, and have executeToolCall return an `isError` CallToolResult (also covering network failures). - Fix in the shared packages/openapi-codegen/src/runtime.ts → regenerated into both tools.generated.ts and tools.v2.generated.ts (runtime block only; 0 per-tool changes). - The 2xx path and all 15 normalizeResponse handlers are unchanged (they only run on 2xx). - Adds runtime tests for 2xx / 401 / 500 / network-failure (17 tests pass). Benefits all 62 tools. --- .changeset/surface-api-errors.md | 10 +++ packages/openapi-codegen/src/runtime.test.ts | 66 +++++++++++++++++++- packages/openapi-codegen/src/runtime.ts | 26 +++++++- packages/server/src/tools.generated.ts | 26 +++++++- packages/server/src/tools.v2.generated.ts | 26 +++++++- 5 files changed, 150 insertions(+), 4 deletions(-) create mode 100644 .changeset/surface-api-errors.md diff --git a/.changeset/surface-api-errors.md b/.changeset/surface-api-errors.md new file mode 100644 index 0000000..4dbd18b --- /dev/null +++ b/.changeset/surface-api-errors.md @@ -0,0 +1,10 @@ +--- +'@taskade/mcp-server': patch +'@taskade/mcp-openapi-codegen': patch +--- + +Surface API errors instead of returning them as successful tool results. The runtime now +checks `response.ok`: a non-2xx response (401/403/422/5xx) or a network failure comes back +as an `isError` tool result carrying the status and body, rather than the error payload +being handed to the model as if the call had succeeded. Applies to all generated tools +(v1 + v2); the 2xx path and the `normalizeResponse` handlers are unchanged. diff --git a/packages/openapi-codegen/src/runtime.test.ts b/packages/openapi-codegen/src/runtime.test.ts index 53e694a..b76d743 100644 --- a/packages/openapi-codegen/src/runtime.test.ts +++ b/packages/openapi-codegen/src/runtime.test.ts @@ -1,6 +1,10 @@ import { describe, expect, it } from 'vitest'; -import { prepareToolCallOperation } from './runtime'; +import { + OpenAPIToolRuntimeConfig, + prepareToolCallOperation, + ToolCallOpenApiOperation, +} from './runtime'; describe('prepareToolCallOperation', () => { it('splits input into path params, query params, and JSON body', () => { @@ -34,3 +38,63 @@ describe('prepareToolCallOperation', () => { expect(result.headers['Content-Type']).toBeUndefined(); }); }); + +const op: ToolCallOpenApiOperation = { + name: 'thing', + path: '/thing', + method: 'POST', + input: {}, +}; + +const fakeFetch = (status: number, body: unknown) => async () => ({ + ok: status >= 200 && status < 300, + status, + statusText: `Status ${status}`, + json: async () => body, + text: async () => (typeof body === 'string' ? body : JSON.stringify(body)), +}); + +describe('OpenAPIToolRuntimeConfig.executeToolCall', () => { + it('returns the response body normally on a 2xx', async () => { + const config = new OpenAPIToolRuntimeConfig({ + url: 'https://example.com', + fetch: fakeFetch(200, { hello: 'world' }), + }); + const result = await config.executeToolCall(op); + expect(result.isError).toBeFalsy(); + expect(JSON.stringify(result.content)).toContain('hello'); + }); + + it('surfaces a 401 as isError instead of a fake success', async () => { + const config = new OpenAPIToolRuntimeConfig({ + url: 'https://example.com', + fetch: fakeFetch(401, { error: 'Unauthorized' }), + }); + const result = await config.executeToolCall(op); + expect(result.isError).toBe(true); + expect(JSON.stringify(result.content)).toContain('401'); + expect(JSON.stringify(result.content)).toContain('Unauthorized'); + }); + + it('surfaces a 500 as isError', async () => { + const config = new OpenAPIToolRuntimeConfig({ + url: 'https://example.com', + fetch: fakeFetch(500, 'Internal Server Error'), + }); + const result = await config.executeToolCall(op); + expect(result.isError).toBe(true); + expect(JSON.stringify(result.content)).toContain('500'); + }); + + it('surfaces a network/transport failure as isError', async () => { + const config = new OpenAPIToolRuntimeConfig({ + url: 'https://example.com', + fetch: async () => { + throw new Error('ECONNREFUSED'); + }, + }); + const result = await config.executeToolCall(op); + expect(result.isError).toBe(true); + expect(JSON.stringify(result.content)).toContain('ECONNREFUSED'); + }); +}); diff --git a/packages/openapi-codegen/src/runtime.ts b/packages/openapi-codegen/src/runtime.ts index c21e4d6..6031966 100644 --- a/packages/openapi-codegen/src/runtime.ts +++ b/packages/openapi-codegen/src/runtime.ts @@ -126,6 +126,17 @@ export class OpenAPIToolRuntimeConfig { }, }); + if (!response.ok) { + // Surface API errors instead of handing the error body back as a successful + // tool result. Read the body as text (works for JSON or non-JSON errors) and + // throw — executeToolCall turns this into an `isError` CallToolResult so the + // model sees a real failure rather than a 401/422/500 payload that looks like success. + const body = await response.text().catch(() => ''); + throw new Error( + `Taskade API request failed: ${response.status} ${response.statusText}${body ? ` — ${body}` : ''}`, + ); + } + return await response.json(); } @@ -137,8 +148,21 @@ export class OpenAPIToolRuntimeConfig { this.defaultExecuteToolCall(payload)); return this.normaliseResponse(operation, response); } catch (error) { + // Return a proper error result (instead of rethrowing) so the model gets a + // clear, actionable failure message. Covers HTTP errors (thrown above) and + // network/transport failures alike. console.error('OPENAPI_TOOL_CALL_ERROR', error); - throw error; + return { + isError: true, + content: [ + { + type: 'text', + text: `Tool "${operation.name}" failed: ${ + error instanceof Error ? error.message : String(error) + }`, + }, + ], + }; } } diff --git a/packages/server/src/tools.generated.ts b/packages/server/src/tools.generated.ts index be02955..4c57093 100644 --- a/packages/server/src/tools.generated.ts +++ b/packages/server/src/tools.generated.ts @@ -133,6 +133,17 @@ export class OpenAPIToolRuntimeConfig { }, }); + if (!response.ok) { + // Surface API errors instead of handing the error body back as a successful + // tool result. Read the body as text (works for JSON or non-JSON errors) and + // throw — executeToolCall turns this into an `isError` CallToolResult so the + // model sees a real failure rather than a 401/422/500 payload that looks like success. + const body = await response.text().catch(() => ''); + throw new Error( + `Taskade API request failed: ${response.status} ${response.statusText}${body ? ` — ${body}` : ''}`, + ); + } + return await response.json(); } @@ -144,8 +155,21 @@ export class OpenAPIToolRuntimeConfig { this.defaultExecuteToolCall(payload)); return this.normaliseResponse(operation, response); } catch (error) { + // Return a proper error result (instead of rethrowing) so the model gets a + // clear, actionable failure message. Covers HTTP errors (thrown above) and + // network/transport failures alike. console.error('OPENAPI_TOOL_CALL_ERROR', error); - throw error; + return { + isError: true, + content: [ + { + type: 'text', + text: `Tool "${operation.name}" failed: ${ + error instanceof Error ? error.message : String(error) + }`, + }, + ], + }; } } diff --git a/packages/server/src/tools.v2.generated.ts b/packages/server/src/tools.v2.generated.ts index 59820a0..f406083 100644 --- a/packages/server/src/tools.v2.generated.ts +++ b/packages/server/src/tools.v2.generated.ts @@ -133,6 +133,17 @@ export class OpenAPIToolRuntimeConfig { }, }); + if (!response.ok) { + // Surface API errors instead of handing the error body back as a successful + // tool result. Read the body as text (works for JSON or non-JSON errors) and + // throw — executeToolCall turns this into an `isError` CallToolResult so the + // model sees a real failure rather than a 401/422/500 payload that looks like success. + const body = await response.text().catch(() => ''); + throw new Error( + `Taskade API request failed: ${response.status} ${response.statusText}${body ? ` — ${body}` : ''}`, + ); + } + return await response.json(); } @@ -144,8 +155,21 @@ export class OpenAPIToolRuntimeConfig { this.defaultExecuteToolCall(payload)); return this.normaliseResponse(operation, response); } catch (error) { + // Return a proper error result (instead of rethrowing) so the model gets a + // clear, actionable failure message. Covers HTTP errors (thrown above) and + // network/transport failures alike. console.error('OPENAPI_TOOL_CALL_ERROR', error); - throw error; + return { + isError: true, + content: [ + { + type: 'text', + text: `Tool "${operation.name}" failed: ${ + error instanceof Error ? error.message : String(error) + }`, + }, + ], + }; } }