-
Notifications
You must be signed in to change notification settings - Fork 11
Add retry to EVM RPC #201
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add retry to EVM RPC #201
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds a generic retry utility and integrates it into the EVM RPC stream configuration to wrap several RPC calls (log and block header fetches) with retry logic; also adds a prerelease changelog entry documenting the retry behavior. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Retry as RetryUtil
participant RPC as EVM_RPC_Client
participant Oracle as BlockRangeOracle
Client->>Retry: call fetchLogsForRangeWithRetry(fn)
Retry->>RPC: attempt RPC request (fetch logs / block)
RPC-->>Retry: success OR error
alt success
Retry-->>Client: return result
else error & attempts < max
Retry-->>Retry: wait delay
Retry->>RPC: retry attempt
else error & attempts >= max
Retry-->>Oracle: notify via handleError(...)
Oracle-->>Client: handle error path
Retry-->>Client: throw error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
7fd76d6 to
ff399d5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@packages/evm-rpc/src/retry.ts`:
- Around line 1-20: The retry function may loop indefinitely if maxAttempts or
delay are non-finite/NaN; update retry to validate and clamp input parameters by
ensuring maxAttempts is a finite integer >=1 (fallback to 3) and delay is a
finite non-negative number (fallback to 100), coerce/round maxAttempts (e.g.
Math.floor) and guard attempts against exceeding the clamped maxAttempts before
retrying; locate the retry function and adjust its parameter handling and the
attempts check to use these validated values (references: retry, maxAttempts,
delay, attempts, fn).
In `@packages/evm-rpc/src/stream-config.ts`:
- Around line 275-289: The retry block currently calls
this.blockRangeOracle.handleError(error) inside the retried fn, causing
currentSize to be halved on every attempt; change this by removing handleError
from inside the async fn passed to retry and instead wrap the entire await
retry(...) in a try/catch where you call
this.blockRangeOracle.handleError(error) once before rethrowing; locate the
retry invocation that calls fetchLogsForRange (the function using this.client,
fromBlock, toBlock, filter, mergeGetLogs) and move the handleError call to the
outer catch so state mutation happens only once per error event.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/evm-rpc/src/stream-config.ts`:
- Around line 275-290: The code contains an extra closing token "});" after the
try/catch that breaks parsing; remove the stray "});" so the try { return await
retry({...}) } catch (error) { this.blockRangeOracle.handleError(error); throw
error; } block closes correctly, ensuring the call to retry and the error
handling around fetchLogsForRange remain intact (look for the retry invocation
and this.blockRangeOracle.handleError references to locate the exact spot).
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
006679c to
58e6f7f
Compare
No description provided.