From 4c1ab4d4a8be4301c917abb16ea6a764c352c441 Mon Sep 17 00:00:00 2001 From: Adam Chmara Date: Tue, 28 Apr 2026 11:23:04 +0200 Subject: [PATCH] fix(framework): clean up AgentDeliveryError message to avoid logging raw response bodies Previously, non-ok upstream responses (e.g. Cloudflare 502 HTML pages, provider JSON errors) were used verbatim as the error message, flooding the console with unhelpful output. Now the message is always a short human-readable summary like "Delivery failed: Bad Gateway" and the raw body is preserved on a separate `responseBody` property for debugging. Made-with: Cursor --- packages/framework/src/handler.ts | 8 +- .../src/resources/agent/agent.context.ts | 14 +-- .../src/resources/agent/agent.errors.ts | 27 +++++- .../src/resources/agent/agent.test.ts | 90 +++++++++++++++++++ 4 files changed, 120 insertions(+), 19 deletions(-) diff --git a/packages/framework/src/handler.ts b/packages/framework/src/handler.ts index 8d6e7de92e0..92d92ec8b84 100644 --- a/packages/framework/src/handler.ts +++ b/packages/framework/src/handler.ts @@ -22,7 +22,7 @@ import { } from './errors'; import { isPlatformError } from './errors/guard.errors'; import type { Agent, AgentBridgeRequest, MessageContent } from './resources/agent'; -import { AgentContextImpl, AgentEventEnum } from './resources/agent'; +import { AgentContextImpl, AgentDeliveryError, AgentEventEnum } from './resources/agent'; import type { Awaitable, EventTriggerParams, Workflow } from './types'; import { createHmacSubtle, initApiClient } from './utils'; @@ -226,7 +226,11 @@ export class NovuRequestHandler { const ctx = new AgentContextImpl(body as AgentBridgeRequest, this.client.secretKey); const handlerPromise = this.runAgentHandler(registeredAgent, agentEvent, ctx).catch((err) => { - console.error(`[agent:${agentId}] Handler error:`, err); + if (err instanceof AgentDeliveryError) { + console.error(`[agent:${agentId}] ${err.message}`); + } else { + console.error(`[agent:${agentId}] Handler error:`, err); + } }); if (waitUntil) { diff --git a/packages/framework/src/resources/agent/agent.context.ts b/packages/framework/src/resources/agent/agent.context.ts index 1c52ea1f2d8..4bc7c82d178 100644 --- a/packages/framework/src/resources/agent/agent.context.ts +++ b/packages/framework/src/resources/agent/agent.context.ts @@ -226,19 +226,7 @@ export class AgentContextImpl implements AgentContext { if (!response.ok) { const text = await response.text().catch(() => ''); - - if (response.status === 502) { - let message = text; - try { - const parsed = JSON.parse(text) as { message?: string }; - if (parsed.message) message = parsed.message; - } catch { - // use raw text if JSON parsing fails - } - throw new AgentDeliveryError(502, message); - } - - throw new Error(`Agent reply failed (${response.status}): ${text}`); + throw new AgentDeliveryError(response.status, text); } const raw = await response.text().catch(() => ''); diff --git a/packages/framework/src/resources/agent/agent.errors.ts b/packages/framework/src/resources/agent/agent.errors.ts index 349955594b1..c2ad1b1fae4 100644 --- a/packages/framework/src/resources/agent/agent.errors.ts +++ b/packages/framework/src/resources/agent/agent.errors.ts @@ -1,9 +1,26 @@ +const HTTP_STATUS_TEXT: Record = { + 400: 'Bad Request', + 401: 'Unauthorized', + 402: 'Payment Required', + 403: 'Forbidden', + 404: 'Not Found', + 408: 'Request Timeout', + 409: 'Conflict', + 422: 'Unprocessable Entity', + 429: 'Too Many Requests', + 500: 'Internal Server Error', + 502: 'Bad Gateway', + 503: 'Service Unavailable', + 504: 'Gateway Timeout', +}; + /** * Thrown by `ctx.reply()` and `handle.edit()` when the upstream message delivery * fails — e.g. the configured email provider returns 401, Slack rejects the token, * or Teams rejects the request. * - * The `message` property contains the original provider error text + * `message` is always a short, human-readable summary. + * `responseBody` preserves the raw upstream body for debugging. * * @example * ```ts @@ -13,7 +30,6 @@ * await ctx.reply('Hello!'); * } catch (err) { * if (err instanceof AgentDeliveryError) { - * // Delivery failed (misconfigured provider, rate limit, etc.) * console.error('Delivery failed:', err.message); * return; * } @@ -23,10 +39,13 @@ */ export class AgentDeliveryError extends Error { readonly statusCode: number; + readonly responseBody: string; - constructor(statusCode: number, message: string) { - super(message); + constructor(statusCode: number, responseBody: string) { + const reason = HTTP_STATUS_TEXT[statusCode] ?? statusCode; + super(`Delivery failed: ${reason}`); this.name = 'AgentDeliveryError'; this.statusCode = statusCode; + this.responseBody = responseBody; } } diff --git a/packages/framework/src/resources/agent/agent.test.ts b/packages/framework/src/resources/agent/agent.test.ts index 6cca4536342..5a8b3665f45 100644 --- a/packages/framework/src/resources/agent/agent.test.ts +++ b/packages/framework/src/resources/agent/agent.test.ts @@ -4,6 +4,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { Client } from '../../client'; import { PostActionEnum } from '../../constants'; import { NovuRequestHandler } from '../../handler'; +import { AgentDeliveryError } from './agent.errors'; import { agent } from './agent.resource'; import type { AgentBridgeRequest } from './agent.types'; import { Button, Card, CardText } from './index'; @@ -1148,6 +1149,95 @@ describe('agent dispatch via NovuRequestHandler', () => { expect(replyBody.reply.markdown).toBe("Sorry that wasn't helpful!"); }); + it.each([ + { status: 502, body: '

502 Bad Gateway

', label: 'gateway HTML page', reason: 'Bad Gateway' }, + { status: 401, body: '{"message":"Invalid API key"}', label: 'JSON credentials error', reason: 'Unauthorized' }, + { status: 403, body: 'Forbidden', label: 'plain text forbidden', reason: 'Forbidden' }, + { status: 429, body: '{"statusCode":429,"message":"Rate limit exceeded"}', label: 'rate limit', reason: 'Too Many Requests' }, + { status: 500, body: '', label: 'empty body', reason: 'Internal Server Error' }, + { status: 599, body: 'weird', label: 'unknown status code', reason: '599' }, + ])('should throw AgentDeliveryError with clean message for $label ($status)', async ({ status, body, reason }) => { + fetchMock.mockResolvedValueOnce({ ok: false, status, text: () => Promise.resolve(body) }); + + let caughtError: unknown; + const testBot = agent('test-bot', { + onMessage: async (ctx) => { + try { + await ctx.reply('Hello'); + } catch (err) { + caughtError = err; + throw err; + } + }, + }); + + const handler = new NovuRequestHandler({ + frameworkName: 'test', + agents: [testBot], + client, + handler: () => { + const body = createMockBridgeRequest(); + const url = new URL(`http://localhost?action=${PostActionEnum.AGENT_EVENT}&agentId=test-bot&event=onMessage`); + + return { + body: () => body, + headers: () => null, + method: () => 'POST', + url: () => url, + transformResponse: (res: any) => res, + }; + }, + }); + + await handler.createHandler()(); + await vi.waitFor(() => expect(caughtError).toBeDefined()); + + expect(caughtError).toBeInstanceOf(AgentDeliveryError); + const err = caughtError as AgentDeliveryError; + expect(err.message).toBe(`Delivery failed: ${reason}`); + expect(err.statusCode).toBe(status); + expect(err.responseBody).toBe(body); + }); + + it('should log delivery errors without leaking the response body', async () => { + const longBody = '' + '

error

'.repeat(500); + fetchMock.mockResolvedValueOnce({ ok: false, status: 502, text: () => Promise.resolve(longBody) }); + + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + + const testBot = agent('test-bot', { + onMessage: async (ctx) => { + await ctx.reply('Hello'); + }, + }); + + const handler = new NovuRequestHandler({ + frameworkName: 'test', + agents: [testBot], + client, + handler: () => { + const body = createMockBridgeRequest(); + const url = new URL(`http://localhost?action=${PostActionEnum.AGENT_EVENT}&agentId=test-bot&event=onMessage`); + + return { + body: () => body, + headers: () => null, + method: () => 'POST', + url: () => url, + transformResponse: (res: any) => res, + }; + }, + }); + + await handler.createHandler()(); + await vi.waitFor(() => expect(errorSpy).toHaveBeenCalled()); + + const logged = errorSpy.mock.calls[0].join(' '); + expect(logged).toBe('[agent:test-bot] Delivery failed: Bad Gateway'); + + errorSpy.mockRestore(); + }); + it('should not send a reply when onReaction returns nothing (reaction removed)', async () => { const testBot = agent('test-bot', { onMessage: async (ctx) => { await ctx.reply('noop'); },