Skip to content

fix: add query exceeds max block range to ParseMaxRangeFromError#1589

Draft
joanestebanr wants to merge 1 commit intorelease/0.10from
fix/query_exceeds_block_range
Draft

fix: add query exceeds max block range to ParseMaxRangeFromError#1589
joanestebanr wants to merge 1 commit intorelease/0.10from
fix/query_exceeds_block_range

Conversation

@joanestebanr
Copy link
Copy Markdown
Collaborator

@joanestebanr joanestebanr commented Apr 20, 2026

🔄 Changes Summary

  • Added a new regex to ParseMaxRangeFromError (common/errors.go) to match the error format query exceeds max block range <N>, which was not covered by any of the existing patterns.
  • Without this fix, the ClaimSyncer's RPC fallback path failed to detect this as a block-range limit error and returned immediately instead of retrying with chunked requests, causing:
    claimsync: FilterLogs error for globalIndex X [0, 963183]: query exceeds max block range 100000
    
  • Added two unit tests for the new pattern in common/errors_test.go.

⚠️ Breaking Changes

None.

📋 Config Updates

None.

✅ Testing

  • 🤖 Automatic: go test ./common/... -run TestParseMaxRangeFromError

🐞 Issues

  • No linked issue.

🔗 Related PRs

📝 Notes

  • The existing regexes cover: block range too large, max range: X, exceeded maximum block range: X, and eth_getLogs is limited to a X range. The new format query exceeds max block range X is emitted by some RPC providers (observed in production with a 100000-block limit).

@claude
Copy link
Copy Markdown

claude Bot commented Apr 20, 2026

Claude finished @joanestebanr's task in 1m 2s —— View job


Code Review Complete ✅

Todo List

  • Read repository guidelines (CLAUDE.md)
  • Read modified files (common/errors.go and common/errors_test.go)
  • Analyze the changes using git diff
  • Review code quality and best practices
  • Check test coverage and quality
  • Identify potential bugs or security issues
  • Provide prioritized feedback
  • Fill out PR summary template

🚀 What's New

This PR adds support for parsing a new error format: "query exceeds max block range [number]". The ParseMaxRangeFromError function now recognizes four different error message formats that indicate block range limitations from RPC providers.

🐛 Bug Fixes

None - this is a feature enhancement to improve error parsing coverage.

📋 Config Updates

None.

⚠️ Breaking Changes

None - this change is fully backward compatible.


Review Feedback

🟢 Overall Assessment

This is a clean, well-tested addition that follows the project's established patterns. The change is straightforward and maintains consistency with existing code.

✅ Strengths

  1. Good test coverage: Two comprehensive test cases added covering both standalone and embedded error message formats (common/errors_test.go:109-119)
  2. Consistent pattern: Follows the existing approach for handling multiple error formats using regex patterns
  3. Documentation updated: Function comment could be updated but the regex comment is clear (common/errors.go:17-18)
  4. Proper regex pattern: The pattern query exceeds max block range\s+(\d+) correctly captures the numeric value with flexible whitespace handling

🟡 Suggestions

  1. Update function documentation (common/errors.go:21-25)

    • The doc comment lists expected formats but doesn't include the new "query exceeds max block range" format
    • Suggestion: Add the new format to the documentation:
    // ParseMaxRangeFromError extracts the max range value from error message
    // Expected formats:
    //   - "block range too large, max range: 1000"
    //   - "exceeded maximum block range: 5000"
    //   - "eth_getLogs is limited to a 10,000 range"
    //   - "query exceeds max block range 100000"
  2. Test assertion consistency (common/errors_test.go:129)

    • The test at line 129 checks require.Equal(t, tt.expectedMaxBlock, result) even when expectedIsMaxRange is false, which could be confusing
    • For clarity, this assertion should probably only be checked when the parsing succeeds
    • However, this is pre-existing code pattern, so maintaining consistency is acceptable

Code Quality Checklist

  • ✅ Follows Go formatting standards (gofmt, goimports)
  • ✅ Uses testify with require (not assert) as per CLAUDE.md guidelines
  • ✅ Regex pattern is properly compiled at package level
  • ✅ Error handling is appropriate
  • ✅ Test cases cover edge cases (embedded in longer messages)
  • ✅ No security concerns identified
  • ✅ No performance issues (regex is pre-compiled)
  • ✅ Variable naming follows conventions
  • ⚠️ Function documentation slightly outdated (minor)

Security & Performance

  • 🟢 Security: No concerns. Regex patterns are bounded and don't introduce ReDoS vulnerabilities
  • 🟢 Performance: No concerns. Regex is pre-compiled as a package-level variable, so no performance impact

Final Verdict

LGTM with minor documentation suggestion. The code is production-ready and the documentation update is optional but recommended for completeness.


@sonarqubecloud
Copy link
Copy Markdown

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