fix: consistent fetch --max-amount cap error message#54
Conversation
…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.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 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 |
|
I don't think this is an issue. The problem is your agent didn't pass all required parameters. Closing for now - if you disagree please let me know. |
What
fetch --max-amountvalidates 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.: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
classifyRailcall infetchso any rejection — or any non-BTC/sats rail it classifies — collapses into fetch's single supported-rail message: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 testgreen (110 pass, 2 skipped).fetch-max-amount.test.tsasserts 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 leakarbitrumor theBTC|USD|EUR|USDCvocabulary.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/--verboseflag being quiet duringfetch— that turned out to be by-design (verbose is a connection-flow flag, same aspay), not a bug.