fix: surface non-OK response bodies instead of discarding them by rolznz · Pull Request #64 · getAlby/cli

rolznz · 2026-07-24T08:16:12Z

Fixes #63

Non-OK responses discarded their body, which is often the only diagnostic - e.g. the l402.space gateway's 502 explaining exactly why it refused ("upstream price $4.78 exceeds this gateway's per-request cap") was reduced to {"status": 502, "payment_required": false}, forcing a drop to raw curl to see the reason.

Changes

Both fetch paths now surface the error body, structurally and capped at 4096 chars (with content_truncated: true when cut off), via a shared readErrorBody helper:

--dry-run: a non-2xx, non-402 response now includes content alongside status/payment_required (empty bodies are omitted; 2xx stays body-less - a dry run is about the price, not the content)
normal fetch: the non-OK error output now carries content as a structured field next to paymentRecovery (when a payment was made), instead of embedding the unbounded body inside the error message string

Review follow-ups

error bodies are now read through a bounded stream reader that stops at the 4096-char cap, instead of buffering the whole body with text() before slicing - an arbitrarily large error body is never held in memory. A failed read mid-body surfaces what was read so far instead of throwing, so the paymentRecovery details next to it are never lost
--dry-run no longer regex-scans the 402 response body for a BOLT11 invoice. The pay path in lightning-tools only ever reads invoices from headers (WWW-Authenticate for L402/MPP, Payment-Required for x402), so a body-only invoice was a price preview the fetch could never actually pay. Those responses now fall through to the l402.space bridge suggestion, and dropping the fallback lets the dry-run body read be bounded too

Verification

New tests: non-2xx body surfaced, 4096-char truncation flagged, empty body omitted, and the non-OK-without-payment error now asserts its structured content; full suite passes (133 tests). Verified live against a bridged 404: both --dry-run and normal fetch now show the upstream error page that was previously discarded.

🤖 Generated with Claude Code

Summary by CodeRabbit

Bug Fixes
- Improved error reporting for failed payment recovery requests by preserving diagnostic response details.
- Dry-run requests now expose structured error content when responses fail.
- Large error responses are safely limited to 4,096 characters and flagged when truncated.
- Empty error responses no longer produce misleading content.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

coderabbitai · 2026-07-24T08:16:29Z

📝 Walkthrough

Walkthrough

Non-OK fetch responses now preserve capped response-body diagnostics. fetch402 attaches structured details to thrown errors, while dryRun402 exposes optional content and truncation metadata. Tests cover successful, populated, oversized, empty, and payment-recovery responses.

Changes

Error diagnostics

Layer / File(s)	Summary
Diagnostic response contract and body reader `src/tools/lightning/fetch.ts`	`DryRun402Result` adds optional diagnostic fields, response bodies are streamed and capped at 4096 characters, and invoice extraction no longer reads the response body.
Non-OK response integration and validation `src/tools/lightning/fetch.ts`, `src/test/fetch-402-recovery.test.ts`, `src/test/fetch-dry-run.test.ts`	`fetch402` attaches structured response details to errors, while non-OK dry-run responses expose capped content; tests cover successful, populated, oversized, empty, and payment-recovery cases.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

getAlby/cli#56: Both modify dryRun402 behavior and its result diagnostics.
getAlby/cli#59: Both update fetch402 structured error details and recovery tests.
getAlby/cli#62: Both change how non-OK fetch402 response bodies are attached to errors.

Suggested reviewers: reneaaron

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The changes expose capped error bodies for non-2xx responses in both fetch paths, matching issue `#63`.
Out of Scope Changes check	✅ Passed	The added tests and helper updates all support the same error-body surfacing fix, with no obvious unrelated scope.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: surfacing non-OK response bodies instead of discarding them.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/surface-error-bodies

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.}

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/test/fetch-402-recovery.test.ts (1)
153-155: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Also assert content alongside payment recovery.

This verifies the unpaid branch only. Add an assertion in the paid non-OK case that error.details.content is present together with paymentRecovery, ensuring both spreads remain intact.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/test/fetch-402-recovery.test.ts` around lines 153 - 155, Extend the paid
non-OK test case in fetch-402-recovery.test.ts to assert that error.details
includes both paymentRecovery and the preserved content field. Keep the existing
unpaid-branch assertion unchanged and verify content alongside the recovery
data.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/tools/lightning/fetch.ts`:
- Around line 119-124: Update the non-OK response handling in the fetch flow to
read result.body through a bounded stream reader before passing the content to
errorBody, rather than calling result.text(). Enforce the established
response-size cap, stop reading once it is reached, and preserve the
content_truncated indicator in the resulting DetailedError metadata while
retaining paidRecoveryDetails handling.

---

Nitpick comments:
In `@src/test/fetch-402-recovery.test.ts`:
- Around line 153-155: Extend the paid non-OK test case in
fetch-402-recovery.test.ts to assert that error.details includes both
paymentRecovery and the preserved content field. Keep the existing unpaid-branch
assertion unchanged and verify content alongside the recovery data.

🪄 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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 365a34e6-7658-4096-866a-6007bdf36ffe

📥 Commits

Reviewing files that changed from the base of the PR and between ce434db and 36ff644.

📒 Files selected for processing (3)

src/test/fetch-402-recovery.test.ts
src/test/fetch-dry-run.test.ts
src/tools/lightning/fetch.ts

…2 headers Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

fix: surface non-OK response bodies instead of discarding them (#63)

36ff644

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

coderabbitai Bot reviewed Jul 24, 2026

View reviewed changes

Comment thread src/tools/lightning/fetch.ts

fix: bound error-body reads at the cap and take invoices only from 40…

47e72a3

…2 headers Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

rolznz merged commit ae789db into master Jul 25, 2026
1 of 2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: surface non-OK response bodies instead of discarding them#64

fix: surface non-OK response bodies instead of discarding them#64
rolznz merged 2 commits into
masterfrom
fix/surface-error-bodies

rolznz commented Jul 24, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Jul 24, 2026 •

edited

Loading

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

rolznz commented Jul 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Review follow-ups

Verification

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

rolznz commented Jul 24, 2026 •

edited

Loading

coderabbitai Bot commented Jul 24, 2026 •

edited

Loading