Skip to content

fix: consistent fetch --max-amount cap error message#54

Draft
reneaaron wants to merge 1 commit into
masterfrom
fix/fetch-max-amount-error
Draft

fix: consistent fetch --max-amount cap error message#54
reneaaron wants to merge 1 commit into
masterfrom
fix/fetch-max-amount-error

Conversation

@reneaaron

Copy link
Copy Markdown
Member

What

fetch --max-amount validates its denomination through the shared amount classifier (src/amount.ts), which speaks the whole CLI's vocabulary — fiat codes, chain networks, and a <BTC|USD|EUR|USDC|…> hint. For an invalid cap, those raw classifier errors leaked through, e.g.:

$ fetch <url> --max-amount 100 --currency USD --unit sats
{ "error": "An amount requires --network <name>. Use \"lightning\" ... Use a chain name (e.g. arbitrum) ..." }

$ fetch <url> --max-amount 100
{ "error": "An amount requires --currency <BTC|USD|EUR|USDC|…> so the denomination is never guessed" }

Both advertise rails fetch's cap doesn't accept (fiat, arbitrum, USDC) and contradict the command's own help, which says the cap requires --currency BTC --unit sats --network lightning.

Fix

Wrap the classifyRail call in fetch so any rejection — or any non-BTC/sats rail it classifies — collapses into fetch's single supported-rail message:

{ "error": "fetch's --max-amount spend cap currently supports only --currency BTC --unit sats --network lightning" }

Valid caps behave exactly as before; only the error text for invalid ones changed. The shared classifier and other commands (pay, etc.) are untouched.

Test

  • yarn test green (110 pass, 2 skipped).
  • New table test in fetch-max-amount.test.ts asserts the four previously-leaking combos (no denomination / fiat+unit / BTC without network / token on a chain) all surface fetch's own message and no longer leak arbitrum or the BTC|USD|EUR|USDC vocabulary.

Context

Found while consumer-testing the CLI for #49. Separate from that PR since it's a pre-existing issue independent of the discover/bridge work.

Not included: the -v/--verbose flag being quiet during fetch — that turned out to be by-design (verbose is a connection-flow flag, same as pay), not a bug.

…del's

fetch's --max-amount runs through the shared amount classifier, which speaks
the whole CLI's vocabulary - fiat codes, chain names like "arbitrum", and a
'<BTC|USD|EUR|USDC|…>' hint. For invalid caps those raw errors leaked
through, advertising rails fetch can't pay a cap in and contradicting the
command's own help (which says the cap requires --currency BTC --unit sats
--network lightning).

Wrap the classifier call so any rejection - or any non-BTC/sats rail it
classifies - collapses into fetch's single supported-rail message. Behavior
is unchanged for valid caps; only the error text is now consistent.
@coderabbitai

coderabbitai Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9f20ebc9-722d-42b6-9ba6-0e6ba750aad4

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/fetch-max-amount-error

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.

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