Skip to content

fix: additional error handling, more logs, more retries#80

Merged
abs2023 merged 2 commits into
devfrom
fix/web3-retries
Feb 8, 2024
Merged

fix: additional error handling, more logs, more retries#80
abs2023 merged 2 commits into
devfrom
fix/web3-retries

Conversation

@alex-sandrk
Copy link
Copy Markdown
Contributor

No description provided.

Comment thread src/plugins/eth/web3Http.js Outdated
Comment on lines +12 to +13
if (payload?.method === 'eth_call' && response.error?.message?.includes('execution reverted') || response.error?.code === -32000) {
return true
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logical OR operator in the condition payload?.method === 'eth_call' && response.error?.message?.includes('execution reverted') || response.error?.code === -32000 is not grouped properly with parentheses, which could lead to unexpected behavior. The condition is meant to check if the method is 'eth_call' and the message includes 'execution reverted', or if the error code is -32000. However, without proper grouping, it might evaluate the 'eth_call' and 'execution reverted' check together with the error code check in an unintended way. To fix this, add parentheses around the conditions that should be evaluated together to ensure the correct order of operations.

Comment thread src/plugins/eth/web3Http.js
Comment on lines +5 to 18
const isRateLimitError = (response, payload) => {
const { result, ...data } = response
const code = response.error?.code
if (code === 429 || code === -32029 || code === -32097) {
return true
}

if (payload?.method === 'eth_call' && (response.error?.message?.includes('execution reverted') || response.error?.code === -32000)) {
return true
}

const message = response.error?.message?.toLowerCase()
if (!message) {
return false
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function isRateLimitError is misleadingly named as it checks for more than just rate limit errors. It also checks for execution reversion and generic errors, which are not related to rate limiting. This can lead to confusion and incorrect error handling.

Recommended solution: Rename the function to accurately reflect the types of errors it checks for, or refactor the function to only check for rate limit errors and handle other error types separately.

@abs2023 abs2023 merged commit 00e96ce into dev Feb 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants