feat: include amountSat and feesPaidMsat on interrupted payment error#334
Conversation
- Rename Fetch402PaymentError to Fetch402InterruptedError: the name wrongly implied the payment failed. The error means the fetch402 flow was interrupted without a response: with paid:true the payment succeeded but the follow-up request failed; with paid:false the payment outcome is unknown (it may still settle). - Add amountSat, decoded from the invoice in the constructor exactly like paymentHash, so it is always present. - Add feesPaidMsat from the wallet's fees_paid, present when paid - so a caller surfacing the payment info (e.g. the Alby CLI's paymentRecovery error output) no longer loses the routing fee. - Bump version to 9.0.1. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe 402 payment failure error is renamed to Changes402 interruption error update
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@package.json`:
- Line 3: Preserve backward compatibility for the public Fetch402PaymentError
export and its serialized name discriminator by adding a legacy alias or
deserialization handler for existing consumers; otherwise update the package
version from 9.0.1 to 10.0.0 to mark the breaking change.
🪄 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
Run ID: dc053904-b1b7-4918-b032-bf610172b346
📒 Files selected for processing (6)
README.mdpackage.jsonsrc/402/l402/l402.test.tssrc/402/mpp/mpp.test.tssrc/402/utils.tssrc/402/x402/x402.test.ts
| { | ||
| "name": "@getalby/lightning-tools", | ||
| "version": "9.0.0", | ||
| "version": "9.0.1", |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## package.json version\n'
rg -n '"version"\s*:' package.json || true
printf '\n## Fetch402 names\n'
rg -n 'Fetch402(Payment|Interrupted)Error|name\s*=\s*[\'"]Fetch402' . || true
printf '\n## exports / public API surfaces\n'
rg -n 'export .*Fetch402|Fetch402.*export|from .*Fetch402' . || trueRepository: getAlby/js-lightning-tools
Length of output: 292
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo 'package.json:'
cat package.json | sed -n '1,40p'
echo
echo 'matches for Fetch402PaymentError / Fetch402InterruptedError:'
rg -n 'Fetch402PaymentError|Fetch402InterruptedError' .Repository: getAlby/js-lightning-tools
Length of output: 3046
🏁 Script executed:
#!/bin/bash
set -euo pipefail
pwd
lsRepository: getAlby/js-lightning-tools
Length of output: 381
Keep Fetch402PaymentError compatibility, or bump to 10.0.0. If this change replaces the public export and serialized name discriminator, it breaks existing imports and JSON-based recovery; add a legacy alias/handler or ship it as a major release.
🤖 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 `@package.json` at line 3, Preserve backward compatibility for the public
Fetch402PaymentError export and its serialized name discriminator by adding a
legacy alias or deserialization handler for existing consumers; otherwise update
the package version from 9.0.1 to 10.0.0 to mark the breaking change.
There was a problem hiding this comment.
v9.0.0 was released today, it's a quick follow-up fix.
There was a problem hiding this comment.
@rolznz, understood. Since 9.0.0 is already published, consumers can still receive 9.0.1 as a patch update and have existing Fetch402PaymentError imports break. I’d retain a deprecated compatibility alias (and accept the legacy name where relevant) for this release, or publish the rename as 10.0.0.
1 participant
Follow-up to #333, found while consuming the error in getAlby/cli#59: the thrown error lost the payment's cost details, so the CLI could not surface complete payment info to the agent.
Changes
Fetch402PaymentError→Fetch402InterruptedError. The old name implied the payment failed, but the error actually means the fetch402 flow was interrupted without a response: withpaid: truethe payment succeeded and only the follow-up request failed; withpaid: falsethe outcome is unknown (it may still settle). Thenamediscriminator is updated to match.amountSat— decoded from the invoice in the constructor exactly likepaymentHash, so it is always present (0 when the invoice cannot be decoded).feesPaidMsat— from the wallet'sfees_paid, present on thepaid: truethrow. PreviouslyinvResp.fees_paidwas in scope at the throw site but dropped, so a caller rebuildingPaymentInfofrom the error had no routing fee.Note: the rename changes a public export and the documented
name === "..."match, which is technically breaking - but 9.0.0 shipped only hours before this, so real-world impact should be nil.Tests
166 passed. The L402 post-payment-failure test now pays with
fees_paid: 3000and assertsamountSat: 402/feesPaidMsat: 3000; the payInvoice-failure test assertsamountSatis set whilefeesPaidMsatis not.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Documentation
Chores