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) + }`, + }, + ], + }; } }