Skip to content

Conversation

@DiogoSantoss
Copy link
Contributor

@DiogoSantoss DiogoSantoss commented Dec 3, 2025

The previous PR #4042 added eth2wrap logic (multi,lazy,etc) to proxied requests but broke SSE requests.
This PR fixes this issue by using proxy.ServeHTTP(proxyResponseWriter{w.(writeFlusher)}, clonedReq) to maintain a long lived HTTP connection between BN and VC.

category: bug
ticket: none

@DiogoSantoss DiogoSantoss added the do not merge Indicate to bulldozer bot that this PR should not be merged label Dec 3, 2025
@codecov
Copy link

codecov bot commented Dec 3, 2025

Codecov Report

❌ Patch coverage is 8.00000% with 46 lines in your changes missing coverage. Please review.
✅ Project coverage is 56.36%. Comparing base (1cf3d3a) to head (6dcbc31).
⚠️ Report is 15 commits behind head on main.

Files with missing lines Patch % Lines
core/validatorapi/router.go 11.42% 30 Missing and 1 partial ⚠️
app/eth2wrap/lazy.go 0.00% 5 Missing ⚠️
app/eth2wrap/multi.go 0.00% 4 Missing ⚠️
core/validatorapi/validatorapi.go 0.00% 4 Missing ⚠️
app/eth2wrap/httpwrap.go 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4140      +/-   ##
==========================================
- Coverage   56.43%   56.36%   -0.08%     
==========================================
  Files         245      245              
  Lines       31175    31223      +48     
==========================================
+ Hits        17595    17598       +3     
- Misses      11272    11320      +48     
+ Partials     2308     2305       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@KaloyanTanev KaloyanTanev removed the do not merge Indicate to bulldozer bot that this PR should not be merged label Dec 3, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds support for proxying Server-Sent Events (SSE) from beacon nodes by implementing a dedicated reverse proxy handler for the /eth/v1/events endpoint. The implementation extends the existing validator API infrastructure to handle streaming responses that require persistent connections.

Key changes:

  • Added Address() and Headers() methods to client interfaces to support direct beacon node proxying
  • Implemented eventsHandler using Go's httputil.ReverseProxy for SSE streaming
  • Created proxyResponseWriter wrapper for error metrics instrumentation

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
core/validatorapi/router.go Added SSE events handler with reverse proxy implementation and proxyResponseWriter wrapper
core/validatorapi/validatorapi.go Implemented Address() and Headers() methods for Component
core/validatorapi/router_internal_test.go Added Address() and Headers() methods to testHandler mock
core/validatorapi/mocks/handler.go Generated mock implementations for new interface methods
app/eth2wrap/eth2wrap_gen.go Added Address() and Headers() to Client interface definition
app/eth2wrap/httpwrap.go Implemented Headers() method to return custom headers
app/eth2wrap/lazy.go Added lazy wrapper implementation for Headers() method
app/eth2wrap/multi.go Implemented Address() and Headers() methods for multi-client wrapper
app/eth2wrap/mocks/client.go Generated mock implementation for Headers() method
testutil/beaconmock/beaconmock.go Added Headers() method to Mock returning nil

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

}

// Start a proxy server that will proxy to the target server.
ctx, cancel := context.WithCancel(context.Background())
Copy link
Collaborator

Choose a reason for hiding this comment

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

Try using t.Context() wherever you can in tests.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 8, 2025

@DiogoSantoss DiogoSantoss added the merge when ready Indicates bulldozer bot may merge when all checks pass label Dec 8, 2025
@obol-bulldozer obol-bulldozer bot merged commit 99d5f26 into main Dec 8, 2025
11 of 12 checks passed
@obol-bulldozer obol-bulldozer bot deleted the diogo/support-sse-proxy branch December 8, 2025 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge when ready Indicates bulldozer bot may merge when all checks pass

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants