Skip to content

fix(framework): clean up AgentDeliveryError message to avoid logging raw response bodies#10907

Merged
ChmaraX merged 1 commit intonextfrom
fix/framework-agent-delivery-error-message
Apr 28, 2026
Merged

fix(framework): clean up AgentDeliveryError message to avoid logging raw response bodies#10907
ChmaraX merged 1 commit intonextfrom
fix/framework-agent-delivery-error-message

Conversation

@ChmaraX
Copy link
Copy Markdown
Contributor

@ChmaraX ChmaraX commented Apr 28, 2026

Summary

  • AgentDeliveryError now always produces a short, human-readable message (e.g. "Delivery failed: Unauthorized") instead of using the raw upstream response body, which could be a full Cloudflare HTML error page.
  • The raw body is preserved on a new responseBody property for users who catch the error and need to debug.
  • All non-ok responses from _post now throw AgentDeliveryError uniformly — no more special-casing 502 vs other status codes, no body parsing.
  • The handler catch in handler.ts logs a one-liner for delivery errors while still logging the full object for unexpected user-code errors.

Before:

[agent:monday-test] Handler error: Error [AgentDeliveryError]: <!DOCTYPE html>
<!--[if lt IE 7]> <html class="no-js ie6 oldie" lang="en-US"> <![endif]-->
... (hundreds of lines of Cloudflare HTML)

After:

[agent:monday-test] Delivery failed: Bad Gateway

Test plan

  • Parameterized test covering 6 upstream error shapes: HTML page (502), JSON credentials error (401), plain text (403), rate limit JSON (429), empty body (500), unknown status code (599)
  • All assert the same contract: clean message, correct statusCode, raw body in responseBody
  • Logging test verifies the handler console output is the one-liner and does not leak the response body
  • All 41 tests pass

Made with Cursor

What changed

The PR refactors AgentDeliveryError to provide concise, standardized error messages instead of exposing raw upstream response bodies in logs. Error messages now follow the pattern "Delivery failed: " (derived from HTTP status text), while the raw response body is preserved on a responseBody property for debugging. Handler logging is updated to distinguish delivery errors and log only the clean message, preventing multi-line HTML or error page dumps from appearing in console output.

Affected areas

framework: Updated AgentDeliveryError to standardize message formatting and store raw response bodies separately; simplified the _post method in agent context to always throw AgentDeliveryError with actual response status and text; modified handler logging to check for AgentDeliveryError and output only the error message instead of the full error object.

Key technical decisions

  • AgentDeliveryError constructor signature changed from (statusCode, message) to (statusCode, responseBody), with message auto-generated from HTTP status text lookup (with fallback to numeric status code).

Testing

Parameterized test covers six upstream error scenarios (HTML 502, JSON 401, plain text 403, JSON 429, empty 500, unknown 599) to verify normalized message format, correct status code, and raw body preservation. Additional test verifies handler logging output is concise without leaking response body content. All 41 tests pass.

…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
@netlify
Copy link
Copy Markdown

netlify Bot commented Apr 28, 2026

Deploy Preview for dashboard-v2-novu-staging canceled.

Name Link
🔨 Latest commit 4c1ab4d
🔍 Latest deploy log https://app.netlify.com/projects/dashboard-v2-novu-staging/deploys/69f07c961f68cb0008572770

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 28, 2026

Hey there and thank you for opening this pull request! 👋

We require pull request titles to follow specific formatting rules and it looks like your proposed title needs to be adjusted.

Your PR title is: fix(framework): clean up AgentDeliveryError message to avoid logging raw response bodies

Requirements:

  1. Follow the Conventional Commits specification
  2. As a team member, include Linear ticket ID at the end: fixes TICKET-ID or include it in your branch name

Expected format: feat(scope): Add fancy new feature fixes NOV-123

Details:

PR title must end with 'fixes TICKET-ID' (e.g., 'fixes NOV-123') or include ticket ID in branch name

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 28, 2026

📝 Walkthrough

Walkthrough

The PR refactors agent delivery error handling by consolidating error throwing logic to always use AgentDeliveryError, carrying raw response bodies instead of preformatted messages, and implementing standardized error message formatting with HTTP status text lookup. Handler logging is updated to distinguish delivery failures from other errors.

Changes

Cohort / File(s) Summary
Error Class Refactoring
packages/framework/src/resources/agent/agent.errors.ts
AgentDeliveryError now carries raw responseBody instead of preformatted message; message standardized to Delivery failed: <reason> using new HTTP_STATUS_TEXT lookup with fallback to numeric status code.
Error Handling Simplification
packages/framework/src/resources/agent/agent.context.ts
_post method no longer treats HTTP 502 specially or attempts JSON parsing; always throws AgentDeliveryError with actual response status and raw response text.
Logging Enhancement
packages/framework/src/handler.ts
Added runtime check to distinguish AgentDeliveryError from other errors; logs delivery error messages directly instead of full error objects.
Delivery Error Test Coverage
packages/framework/src/resources/agent/agent.test.ts
Comprehensive tests validating AgentDeliveryError construction with various HTTP status codes and response bodies, plus assertion that delivery errors log with cleaned messages.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • scopsy
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed Title follows Conventional Commits format with valid type 'fix', valid scope 'framework', and imperative description that accurately describes the main change.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/framework/src/resources/agent/agent.errors.ts`:
- Around line 40-50: The constructor signature of AgentDeliveryError was changed
from (statusCode, message) to (statusCode, responseBody) which is a breaking API
change; restore backward-compatibility by adding an overload or runtime
compatibility: update the AgentDeliveryError constructor to accept either
(statusCode: number, message: string) or (statusCode: number, responseBody:
string|object) (detect by type or add a second optional param), preserve the
existing .message semantics (pass the human message to Error via super) and
still set .responseBody when provided (or assign the incoming message to
responseBody if caller used the old form), keep the fields statusCode and
responseBody and set this.name = 'AgentDeliveryError' so existing consumers
continue to work without requiring a major version bump.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: ce59409b-a341-441e-ae9e-cda39b5f2600

📥 Commits

Reviewing files that changed from the base of the PR and between 3c14915 and 4c1ab4d.

📒 Files selected for processing (4)
  • packages/framework/src/handler.ts
  • packages/framework/src/resources/agent/agent.context.ts
  • packages/framework/src/resources/agent/agent.errors.ts
  • packages/framework/src/resources/agent/agent.test.ts

Comment thread packages/framework/src/resources/agent/agent.errors.ts
@ChmaraX ChmaraX merged commit 1bd3632 into next Apr 28, 2026
37 checks passed
@ChmaraX ChmaraX deleted the fix/framework-agent-delivery-error-message branch April 28, 2026 10:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant