Skip to content

Commit 43b1676

Browse files
ParkreinerParkreiner
committed
fix: apply feedback
1 parent 68da7af commit 43b1676

File tree

4 files changed

+19
-23
lines changed

4 files changed

+19
-23
lines changed

‎coderd/httpapi/httpapi.go

+7-11
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,7 @@ func WebsocketCloseSprintf(format string, vars ...any) string {
285285
return msg
286286
}
287287

288-
type InitializeConnectionCallback func(rw http.ResponseWriter, r *http.Request) (
288+
type EventSender func(rw http.ResponseWriter, r *http.Request) (
289289
sendEvent func(sse codersdk.ServerSentEvent) error,
290290
done <-chan struct{},
291291
err error,
@@ -410,8 +410,8 @@ func ServerSentEventSender(rw http.ResponseWriter, r *http.Request) (
410410
return sendEvent, closed, nil
411411
}
412412

413-
// OneWayWebSocket establishes a new WebSocket connection that enforces one-way
414-
// communication from the server to the client.
413+
// WebSocketEventSender establishes a new WebSocket connection that enforces
414+
// one-way communication from the server to the client.
415415
//
416416
// The function returned allows you to send a single message to the client,
417417
// while the channel lets you listen for when the connection closes.
@@ -422,7 +422,7 @@ func ServerSentEventSender(rw http.ResponseWriter, r *http.Request) (
422422
// open a workspace in multiple tabs, the entire UI can start to lock up.
423423
// WebSockets have no such limitation, no matter what HTTP protocol was used to
424424
// establish the connection.
425-
func OneWayWebSocket(rw http.ResponseWriter, r *http.Request) (
425+
func WebSocketEventSender(rw http.ResponseWriter, r *http.Request) (
426426
func(event codersdk.ServerSentEvent) error,
427427
<-chan struct{},
428428
error,
@@ -436,12 +436,8 @@ func OneWayWebSocket(rw http.ResponseWriter, r *http.Request) (
436436
}
437437
go Heartbeat(ctx, socket)
438438

439-
type SocketError struct {
440-
Code websocket.StatusCode
441-
Reason string
442-
}
443439
eventC := make(chan codersdk.ServerSentEvent)
444-
socketErrC := make(chan SocketError, 1)
440+
socketErrC := make(chan websocket.CloseError, 1)
445441
closed := make(chan struct{})
446442
go func() {
447443
defer cancel()
@@ -477,13 +473,13 @@ func OneWayWebSocket(rw http.ResponseWriter, r *http.Request) (
477473
return
478474
}
479475
if err != nil {
480-
socketErrC <- SocketError{
476+
socketErrC <- websocket.CloseError{
481477
Code: websocket.StatusInternalError,
482478
Reason: "Unable to process invalid message from client",
483479
}
484480
return
485481
}
486-
socketErrC <- SocketError{
482+
socketErrC <- websocket.CloseError{
487483
Code: websocket.StatusProtocolError,
488484
Reason: "Clients cannot send messages for one-way WebSockets",
489485
}

‎coderd/httpapi/httpapi_test.go

+8-8
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ func TestWebsocketCloseMsg(t *testing.T) {
162162
}
163163

164164
// Our WebSocket library accepts any arbitrary ResponseWriter at the type level,
165-
// but it must also implement http.Hijack
165+
// but the writer must also implement http.Hijacker for long-lived connections
166166
type mockWsResponseWriter struct {
167167
serverRecorder *httptest.ResponseRecorder
168168
serverConn net.Conn
@@ -196,7 +196,7 @@ func (w mockWsWrite) Write(b []byte) (int, error) {
196196
return w(b)
197197
}
198198

199-
func TestOneWayWebSocket(t *testing.T) {
199+
func TestWebSocketEventSender(t *testing.T) {
200200
t.Parallel()
201201

202202
newBaseRequest := func(ctx context.Context) *http.Request {
@@ -256,7 +256,7 @@ func TestOneWayWebSocket(t *testing.T) {
256256
req.Proto = p.proto
257257

258258
writer := newWebsocketWriter()
259-
_, _, err := httpapi.OneWayWebSocket(writer, req)
259+
_, _, err := httpapi.WebSocketEventSender(writer, req)
260260
require.ErrorContains(t, err, p.proto)
261261
}
262262
})
@@ -267,7 +267,7 @@ func TestOneWayWebSocket(t *testing.T) {
267267
ctx := testutil.Context(t, testutil.WaitShort)
268268
req := newBaseRequest(ctx)
269269
writer := newWebsocketWriter()
270-
send, _, err := httpapi.OneWayWebSocket(writer, req)
270+
send, _, err := httpapi.WebSocketEventSender(writer, req)
271271
require.NoError(t, err)
272272

273273
serverPayload := codersdk.ServerSentEvent{
@@ -293,7 +293,7 @@ func TestOneWayWebSocket(t *testing.T) {
293293
ctx, cancel := context.WithCancel(testutil.Context(t, testutil.WaitShort))
294294
req := newBaseRequest(ctx)
295295
writer := newWebsocketWriter()
296-
_, done, err := httpapi.OneWayWebSocket(writer, req)
296+
_, done, err := httpapi.WebSocketEventSender(writer, req)
297297
require.NoError(t, err)
298298

299299
successC := make(chan bool)
@@ -317,7 +317,7 @@ func TestOneWayWebSocket(t *testing.T) {
317317
ctx := testutil.Context(t, testutil.WaitShort)
318318
req := newBaseRequest(ctx)
319319
writer := newWebsocketWriter()
320-
_, done, err := httpapi.OneWayWebSocket(writer, req)
320+
_, done, err := httpapi.WebSocketEventSender(writer, req)
321321
require.NoError(t, err)
322322

323323
successC := make(chan bool)
@@ -347,7 +347,7 @@ func TestOneWayWebSocket(t *testing.T) {
347347
ctx, cancel := context.WithCancel(testutil.Context(t, testutil.WaitShort))
348348
req := newBaseRequest(ctx)
349349
writer := newWebsocketWriter()
350-
send, done, err := httpapi.OneWayWebSocket(writer, req)
350+
send, done, err := httpapi.WebSocketEventSender(writer, req)
351351
require.NoError(t, err)
352352

353353
successC := make(chan bool)
@@ -388,7 +388,7 @@ func TestOneWayWebSocket(t *testing.T) {
388388
ctx := testutil.Context(t, timeout)
389389
req := newBaseRequest(ctx)
390390
writer := newWebsocketWriter()
391-
_, _, err := httpapi.OneWayWebSocket(writer, req)
391+
_, _, err := httpapi.WebSocketEventSender(writer, req)
392392
require.NoError(t, err)
393393

394394
type Result struct {

‎coderd/workspaceagents.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -1109,13 +1109,13 @@ func (api *API) watchWorkspaceAgentMetadataSSE(rw http.ResponseWriter, r *http.R
11091109
// @Router /workspaceagents/{workspaceagent}/watch-metadata-ws [get]
11101110
// @x-apidocgen {"skip": true}
11111111
func (api *API) watchWorkspaceAgentMetadataWS(rw http.ResponseWriter, r *http.Request) {
1112-
api.watchWorkspaceAgentMetadata(rw, r, httpapi.OneWayWebSocket)
1112+
api.watchWorkspaceAgentMetadata(rw, r, httpapi.WebSocketEventSender)
11131113
}
11141114

11151115
func (api *API) watchWorkspaceAgentMetadata(
11161116
rw http.ResponseWriter,
11171117
r *http.Request,
1118-
connect httpapi.InitializeConnectionCallback,
1118+
connect httpapi.EventSender,
11191119
) {
11201120
// Allow us to interrupt watch via cancel.
11211121
ctx, cancel := context.WithCancel(r.Context())

‎coderd/workspaces.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -1732,13 +1732,13 @@ func (api *API) watchWorkspaceSSE(rw http.ResponseWriter, r *http.Request) {
17321732
// @Success 200 {object} codersdk.ServerSentEvent
17331733
// @Router /workspaces/{workspace}/watch-ws [get]
17341734
func (api *API) watchWorkspaceWS(rw http.ResponseWriter, r *http.Request) {
1735-
api.watchWorkspace(rw, r, httpapi.OneWayWebSocket)
1735+
api.watchWorkspace(rw, r, httpapi.WebSocketEventSender)
17361736
}
17371737

17381738
func (api *API) watchWorkspace(
17391739
rw http.ResponseWriter,
17401740
r *http.Request,
1741-
connect httpapi.InitializeConnectionCallback,
1741+
connect httpapi.EventSender,
17421742
) {
17431743
ctx := r.Context()
17441744
workspace := httpmw.WorkspaceParam(r)

0 commit comments

Comments
 (0)