-
Notifications
You must be signed in to change notification settings - Fork 196
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
fix(cli): add retry to getLogs when getting resource ID's #2709
Conversation
🦋 Changeset detectedLatest commit: 9e5a05a The changes in this PR will be included in the next version bump. This PR includes changesets to release 24 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
onFailedAttempt: async (error) => { | ||
const delay = error.attemptNumber * 500; | ||
debug(`failed to get logs, retrying in ${delay}ms...`); | ||
await wait(delay); |
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.
no need for this here, I believe pRetry
has its own back off internally
(I suspect this was copied from other spots where we also don't need to do this)
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.
Oh, yes just copied other examples :D
}), | ||
{ | ||
retries: 3, | ||
onFailedAttempt: async (error) => { |
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.
we could check the error to see if it contains "block is out of range" and only retry those? but I am happy just retrying all here since its a relatively safe operation
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.
Done, I also like that it makes the retry logic more self-explanatory
Co-authored-by: Kevin Ingersoll <[email protected]>
Co-authored-by: Kevin Ingersoll <[email protected]>
Attempting to fetch invalid logs from an RPC can error with a 400 Bad Request and
"block is out of range"
. In this case, Viem'sgetLogs
does not retry the query.However, when using a request router like proxyd, it is possible that some RPC's are slightly behind and do not have the newer logs. In this case we do actually want to retry the query until the RPC has caught up.
This PR adds a
pretry
to thegetLogs
call in thegetResourceIds
function which has consistently failed in deployment.