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

[rolznz]

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.

[coderabbitai]

📝 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

• X
• Mastodon
• Reddit
• LinkedIn

_{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