backend: prevent panic in WebSocket multiplexer #2731
Merged
Conversation
This file contains 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
This fixes a race condition in the WebSocket multiplexer where sending a COMPLETE message could panic if the connection was closed. Added proper connection state checking and error handling to safely handle closed connections. - Add connection state check before writing messages - Return early if connection is already closed - Add error logging for failed writes - Properly handle WriteJSON errors Signed-off-by: Kautilya Tripathi <[email protected]>
dosubot
bot
added
the
size:S
7 tasks
|
Backend Code coverage changed from 63.4% to 63.6%. Change: .2% 😃. Coverage report |
|
@knrt10 got the same issues again with this patch applied. // ./github.com/farodin91/headlamp/backend/cmd/multiplexer.go:60 I hope the multiplexer patch another bug. i have but no. |
|
@farodin91 Can you please provide more info? It would be great if you please help me reproducing this issue. Can you please help with your setup and what led to this error. Anything will help. Thanks |
|
@knrt10 Could we call later? |
|
Pinged you on k8s slack |
dosubot
bot
added
size:M
|
Backend Code coverage changed from 63.4% to 63.6%. Change: .2% 😃. Coverage report |
Previously, WebSocket messages were being filtered out after receiving a COMPLETE message, which prevented the UI from updating when resources changed. This fix removes the filtering logic to ensure all updates are processed, keeping the UI in sync with the cluster state. Signed-off-by: Kautilya Tripathi <[email protected]>
knrt10
force-pushed
the
fix-websocket-race-condition
branch
from
372deba to
dd7080c
Compare
|
Backend Code coverage changed from 63.4% to 63.7%. Change: .3% 😃. Coverage report |
knrt10
force-pushed
the
fix-websocket-race-condition
branch
from
dd7080c to
75ddf6e
Compare
dosubot
bot
added
size:L
COMPLETE messages are not critical for functionality since the client will continue to receive updates even if the COMPLETE message fails. Changed the log level to reduce noise in error logs while maintaining visibility through info logs. Signed-off-by: Kautilya Tripathi <[email protected]>
knrt10
force-pushed
the
fix-websocket-race-condition
branch
from
75ddf6e to
9c0d977
Compare
|
Backend Code coverage changed from 63.4% to 63.7%. Change: .3% 😃. Coverage report |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This fixes a race condition in the WebSocket multiplexer where sending a COMPLETE message could panic if the connection was closed. Added proper connection state checking and error handling to safely handle closed connections.
Ref: #2563 (comment)