fix: reject non-numeric --*-min/--*-max alert flags instead of silently dropping them - #577
fix: reject non-numeric --*-min/--*-max alert flags instead of silently dropping them#577batuhankocyigit wants to merge 3 commits into
Conversation
Add tests for numeric range validation in alerts commands.
Fixes silent dropping of numeric range filters in alerts commands by rejecting non-numeric input with clear error messages.
pr-reviewer Summary for #3c214a1✅ No issues found The code review completed successfully with no findings. Review effort: 2/5 (Simple) SummaryThis PR correctly fixes a real silent-failure bug: No issues found. A few things worth noting were verified and confirmed clean:
Token usage: 17 input, 2,761 output, 348,787 cache read, 39,427 cache write | Usage Guide New pushes are reviewed automatically with a 10-minute cooldown between reviews. To request a review at any time, comment |
There was a problem hiding this comment.
Auto-approved
This PR was automatically approved because:
- Claude recommends approval
- Claude assessed this as a moderate effort change
- The effort level is within the auto-approval threshold of 2
- No high or critical issues were detected
If you have any concerns, please request a manual review.
What
nansen alerts create/updatenumeric range flags (--usd-min/max,--market-cap-min/max,--fdv-min/max,--token-amount-min/max,--token-age-min/max, and all--inflow/outflow/netflow-*-min/maxflags) went through plainNumber(val).A non-numeric value becomes
NaN, andJSON.stringify(NaN)silently serializes tonull:So a typo'd flag doesn't error — the alert is just created without the filter the user
asked for, and there's no signal anywhere that anything went wrong. This module had no
test coverage at all before this PR.
Fix
Added
parseFiniteNumber(raw, name)and wired it intobuildRange()and the standalonetoken-age fields. Non-numeric input now throws a clear
NansenError(Invalid --<flag> "<value>": must be a number) instead of silently becomingnull. Negative values arestill allowed where they're meaningful (e.g.
--netflow-1h-min -5000for a net-outflowfilter) — this only rejects genuinely non-numeric input, not the sign.
Testing
npm testoutput for the new file:Full suite: 2626 passed / 5 failed / 2 skipped. The 5 failures (all in
doctor.test.js,chmod/permission-based checks) reproduce identically on a clean
mainwith no changes —confirmed by stashing this PR's changes and re-running — so they're a pre-existing
CI/sandbox artifact (running as root skips permission enforcement), not something this
PR introduces.
npm run lint— clean.Checklist
npm testpasses for the new/changed code (full-suite failures pre-exist on main)npm run lintpassesconsole.log, no hardcoded secretssrc/schema.jsonchange needed (no new commands/options, just validation)