Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
akundaz
previously approved these changes
Dec 1, 2025
Contributor
|
please fix ci issues |
0x00101010
reviewed
Dec 1, 2025
Comment on lines
+85
to
+94
| } else { | ||
| // connect_and_handle should never return Ok(()) | ||
| tracing::error!("Builder websocket connection has stopped. Invariant is broken."); |
Contributor
There was a problem hiding this comment.
Suggest to remove else break part, and just do
match self.connect_and_handle(&mut backoff, timeout).await {
Err(err) => { ... },
Ok(err) => {
// log error, but allow retry
},
}
We want it to always retry anyway, no point of keeping the break
Contributor
There was a problem hiding this comment.
My observation is that from our side, the retry is not happening, it seems to be going into the else break part here somehow, however I cannot exactly pinpoint the problem from the code (they look correct). And it's one of the reasons for this suggestion
avalonche
reviewed
Dec 2, 2025
Collaborator
avalonche
left a comment
avalonche
left a comment
There was a problem hiding this comment.
I think these lines shouldn't be needed as the write will be dropped when Ok(()) is returned
if let Err(e) = write.close().await {
tracing::warn!("Failed to close builder ws connection: {}", e);
}
avalonche
approved these changes
Dec 4, 2025
theochap
pushed a commit
to ethereum-optimism/optimism
that referenced
this pull request
Feb 11, 2026
* Add timout to connect_async * Add test * fmt * Review comm
theochap
pushed a commit
to ethereum-optimism/optimism
that referenced
this pull request
Feb 13, 2026
* Add timout to connect_async * Add test * fmt * Review comm
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
connect_async does not have any timeouts built in, if there is problems with websocket handshake it will hang.