chore: add support for one-way websockets to backend #16853
Merged
Conversation
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
|
@Emyrk Just pinging you in case you didn't see this. Let me know if you're busy, and I can find someone else to review this |
|
@f0ssel Steven is really heads-down on Mango stuff right now. Would you be able to take over review for this? |
f0ssel
reviewed
Mar 18, 2025
coderd/httpapi/httpapi.go
Outdated
| @@ -282,7 +285,25 @@ func WebsocketCloseSprintf(format string, vars ...any) string { | |||
| return msg | |||
| } | |||
|
|
|||
| func ServerSentEventSender(rw http.ResponseWriter, r *http.Request) (sendEvent func(ctx context.Context, sse codersdk.ServerSentEvent) error, closed chan struct{}, err error) { | |||
| type InitializeConnectionCallback func(rw http.ResponseWriter, r *http.Request) ( | |||
There was a problem hiding this comment.
Suggested change
| type InitializeConnectionCallback func(rw http.ResponseWriter, r *http.Request) ( | |
| type EventSender func(rw http.ResponseWriter, r *http.Request) ( |
coderd/httpapi/httpapi.go
Outdated
| // open a workspace in multiple tabs, the entire UI can start to lock up. | ||
| // WebSockets have no such limitation, no matter what HTTP protocol was used to | ||
| // establish the connection. | ||
| func OneWayWebSocket(rw http.ResponseWriter, r *http.Request) ( |
There was a problem hiding this comment.
Suggested change
| func OneWayWebSocket(rw http.ResponseWriter, r *http.Request) ( | |
| func WebSocketEventSender(rw http.ResponseWriter, r *http.Request) ( |
coderd/httpapi/httpapi.go
Outdated
Comment on lines
439
to
442
| type SocketError struct { | ||
| Code websocket.StatusCode | ||
| Reason string | ||
| } |
There was a problem hiding this comment.
This should be declared at the package level.
And just to double check, there isn't already a type like this in websocket is there?
coderd/httpapi/httpapi_test.go
Outdated
Comment on lines
164
to
171
| // Our WebSocket library accepts any arbitrary ResponseWriter at the type level, | ||
| // but it must also implement http.Hijack | ||
| type mockWsResponseWriter struct { | ||
| serverRecorder *httptest.ResponseRecorder | ||
| serverConn net.Conn | ||
| clientConn net.Conn | ||
| serverReadWriter *bufio.ReadWriter | ||
| } |
coderd/httpapi/httpapi_test.go
Outdated
| return w(b) | ||
| } | ||
|
|
||
| func TestOneWayWebSocket(t *testing.T) { |
There was a problem hiding this comment.
Because we also touch the SSE Sender, could we include some basic coverage for that if there isn't already?
|
@f0ssel Forgot to ping you yesterday, but I applied all your feedback and added tests for the SSE code |
f0ssel
approved these changes
Mar 25, 2025
Closes #16777 ## Changes made - Added `OneWayWebSocket` utility class to help enforce one-way communication from the server to the client - Updated all client client code to use the new WebSocket-based endpoints made to replace the current SSE-based endpoints - Updated WebSocket event handlers to be aware of new protocols - Refactored existing `useEffect` calls and removed some synchronization bugs - Removed dependencies and types for dealing with SSEs - Addressed some minor Biome warnings
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Closes #16775
Part 1/2, with #16855 handling the frontend changes
Changes made
OneWayWebSocketfunction that establishes WebSocket connections that don't allow client-to-server communication