feat: add exit code 3 for rate limiting (DIS-146)#36
Conversation
When the OpenSea API returns HTTP 429, the CLI now exits with code 3 and labels the error as 'Rate Limited' instead of the generic 'API Error'. This allows agents to programmatically detect rate limits and implement wait-and-retry behavior. Exit codes: 0=success, 1=API error, 2=auth error, 3=rate limited. Co-Authored-By: Chris K <ckorhonen@gmail.com>
Original prompt from Chris K |
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
ckorhonen
left a comment
There was a problem hiding this comment.
Code Review
Overall: Approve with suggestions — All 154 tests pass. The implementation is correct — the conditional is properly nested and JSON output includes all diagnostic fields.
Suggestions
-
Clarify README exit codes — Update
1 - API errorto1 - API error (non-429)to make clear that 429s use code 3. -
Extract exit code constants — With three exit codes (1, 2, 3) as magic numbers, consider defining named constants:
const EXIT_API_ERROR = 1 const EXIT_AUTH_ERROR = 2 const EXIT_RATE_LIMITED = 3
-
Test fragility note — The 3 separate test files rely on Vitest's module isolation because
main()auto-executes on import. Ifisolate: falseis ever set, all 3 break. Future improvement: refactorcli.tsto exportmain()without auto-executing. -
Interaction with PR #35 — The health command catches errors internally and always exits with code 1. A rate-limited health check won't use exit code 3. Worth aligning if both PRs merge.
I've pushed a commit with fixes 1 and 2.
Address reviewer feedback: - Extract EXIT_API_ERROR, EXIT_AUTH_ERROR, EXIT_RATE_LIMITED constants - Clarify README exit code 1 as 'API error (non-429)' Co-Authored-By: Chris K <ckorhonen@gmail.com>
|
Addressed suggestions 1 and 2 in bd01192:
Re: suggestion 3 (test fragility) — agreed, exporting Re: suggestion 4 (PR #35 interaction) — good call. If both merge, the health command's internal catch should be updated to use |
feat: add dedicated exit code 3 for rate limiting (DIS-146)
Summary
When the OpenSea API returns HTTP 429, the CLI now exits with code 3 and labels the error as
"Rate Limited"instead of the generic"API Error". Previously, 429s were indistinguishable from other API errors (both exited with code 1), making it impossible for agents to programmatically detect rate limits and implement wait-and-retry.Exit codes after this change:
0success ·1API error (non-429) ·2auth error ·3rate limitedFiles changed:
src/cli.ts— conditional check onstatusCode === 429in the error handler; all exit codes use named constants (EXIT_API_ERROR,EXIT_AUTH_ERROR,EXIT_RATE_LIMITED)README.md/.agents/rules.md— updated exit code documentationUpdates since last revision
Addressed reviewer feedback from @ckorhonen:
EXIT_API_ERROR = 1,EXIT_AUTH_ERROR = 2,EXIT_RATE_LIMITED = 3— replacing all magic numbers across the threeprocess.exit()call sites1asAPI error (non-429)to make the 429/non-429 split explicitReview & Testing Checklist for Human
isRateLimitedconditional insrc/cli.tscorrectly branches onstatusCode === 429only — no other 4xx codes should trigger exit code 3"error": "Rate Limited"with exit code 3Notes
cli.tsexecutesmain()at import time, requiring separate module scopes per error scenario