Skip to content

Conversation

@pdrobnjak
Copy link
Contributor

@pdrobnjak pdrobnjak commented Sep 30, 2025

Describe your changes and provide context

Currently we are defaulting to latest if an error happens, by doing this we are implicitly introducing side effects into user's query by returning latest instead of queried on error, a much better approach is to fail the request.

As this change introduces an API break to RPCContextProvider I've done an analysis of its usage across the codebase - it's only used in RPC routes (both http and ws) and in instantiations of EVM servers (both http and ws) therefore the newly introduced panic can either fail a request with

{
    "jsonrpc": "2.0",
    "id": 1,
    "error": {
        "code": -32603,
        "message": "method handler crashed"
    }
}

or panic during server instantiation which will lead to failure to boot.

Testing performed to validate your change

Tested locally.

@pdrobnjak pdrobnjak self-assigned this Sep 30, 2025
@pdrobnjak pdrobnjak changed the title Do not resolve latest upon error Do not resolve latest upon error Sep 30, 2025
@pdrobnjak pdrobnjak enabled auto-merge (squash) October 3, 2025 13:52
@codecov
Copy link

codecov bot commented Oct 3, 2025

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 27.11%. Comparing base (5a15103) to head (ef1b212).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
app/app.go 0.00% 1 Missing ⚠️

❌ Your patch status has failed because the patch coverage (0.00%) is below the target coverage (70.00%). You can increase the patch coverage or adjust the target coverage.
❌ Your project status has failed because the head coverage (27.11%) is below the target coverage (60.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           main    #2414       +/-   ##
=========================================
+ Coverage      0   27.11%   +27.11%     
=========================================
  Files         0     1464     +1464     
  Lines         0   144400   +144400     
=========================================
+ Hits          0    39159    +39159     
- Misses        0   101885   +101885     
- Partials      0     3356     +3356     
Files with missing lines Coverage Δ
app/app.go 72.33% <0.00%> (ø)

... and 1463 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@pdrobnjak pdrobnjak merged commit e7ff452 into main Oct 3, 2025
88 of 91 checks passed
@pdrobnjak pdrobnjak deleted the pd/do-not-resolve-latest-upon-error branch October 3, 2025 14:06
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.

4 participants