Skip to content

Conversation

@dvovk
Copy link
Contributor

@dvovk dvovk commented Apr 5, 2025

This pull request includes several updates to the WebSocket handling in the api/websocket_handler.go file and a minor dependency update in the go.mod file. The most important changes focus on improving the WebSocket connection management and handling.

Improvements to WebSocket handling:

  • api/websocket_handler.go: Added a closed field to the WebsocketHandler struct to track the connection state and prevent multiple closures.
  • api/websocket_handler.go: Updated the closeConnection method to use a mutex lock for thread-safe operation and to check if the connection is already closed before proceeding.
  • api/websocket_handler.go: Modified the HandleWebSocket method to close the connection gracefully when the request context is done or the channel is closed.
  • api/websocket_handler.go: Added error handling for normal and unexpected WebSocket closures in the message reading loop.

Dependency update:

  • go.mod: Updated the erigontech/erigonwatch dependency from version v0.1.24 to v0.1.26.

@dvovk dvovk requested a review from Copilot April 5, 2025 08:00
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.

Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.

Files not reviewed (1)
  • go.mod: Language not supported
Comments suppressed due to low confidence (1)

api/websocket_handler.go:174

  • [nitpick] Instead of simply printing a message and breaking out of the loop when a normal or unexpected closure occurs, consider invoking closeConnection() to update the handler’s state and gracefully clean up resources.
if websocket.IsCloseError(err, websocket.CloseNormalClosure) {

@dvovk dvovk merged commit bef5299 into main Apr 5, 2025
1 check passed
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