From 409360c62ddd426659a393eb711b8546eb5bb5ce Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Mon, 26 Jan 2026 09:24:45 +0000 Subject: [PATCH] fix(coderd): ensure inbox WebSocket is closed when client disconnects (#21652) Relates to https://github.com/coder/coder/issues/19715 This is similar to https://github.com/coder/coder/pull/19711 This endpoint works by doing the following: - Subscribing to the database's with pubsub - Accepts a WebSocket upgrade - Starts a `httpapi.Heartbeat` - Creates a json encoder - **Infinitely loops waiting for notification until request context cancelled** The critical issue here is that `httpapi.Heartbeat` silently fails when the client has disconnected. This means we never cancel the request context, leaving the WebSocket alive until we receive a notification from the database and fail to write that down the pipe. By replacing usage of `httpapi.Heartbeat` with `httpapi.HeartbeatClose`, we cancel the context _when the heartbeat fails to write_ due to the client disconnecting. This allows us to cleanup without waiting for a notification to come through the pubsub channel. --- coderd/inboxnotifications.go | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/coderd/inboxnotifications.go b/coderd/inboxnotifications.go index 6c35e33dc7..454aefee79 100644 --- a/coderd/inboxnotifications.go +++ b/coderd/inboxnotifications.go @@ -20,7 +20,6 @@ import ( "github.com/coder/coder/v2/coderd/pubsub" markdown "github.com/coder/coder/v2/coderd/render" "github.com/coder/coder/v2/codersdk" - "github.com/coder/coder/v2/codersdk/wsjson" "github.com/coder/websocket" ) @@ -126,6 +125,7 @@ func (api *API) watchInboxNotifications(rw http.ResponseWriter, r *http.Request) templates = p.UUIDs(vals, []uuid.UUID{}, "templates") readStatus = p.String(vals, "all", "read_status") format = p.String(vals, notificationFormatMarkdown, "format") + logger = api.Logger.Named("inbox_notifications_watcher") ) p.ErrorExcessParams(vals) if len(p.Errors) > 0 { @@ -213,11 +213,17 @@ func (api *API) watchInboxNotifications(rw http.ResponseWriter, r *http.Request) return } - go httpapi.Heartbeat(ctx, conn) - defer conn.Close(websocket.StatusNormalClosure, "connection closed") + ctx, cancel := context.WithCancel(ctx) + defer cancel() - encoder := wsjson.NewEncoder[codersdk.GetInboxNotificationResponse](conn, websocket.MessageText) - defer encoder.Close(websocket.StatusNormalClosure) + _ = conn.CloseRead(context.Background()) + + ctx, wsNetConn := codersdk.WebsocketNetConn(ctx, conn, websocket.MessageText) + defer wsNetConn.Close() + + go httpapi.HeartbeatClose(ctx, logger, cancel, conn) + + encoder := json.NewEncoder(wsNetConn) // Log the request immediately instead of after it completes. if rl := loggermw.RequestLoggerFromContext(ctx); rl != nil { @@ -226,8 +232,12 @@ func (api *API) watchInboxNotifications(rw http.ResponseWriter, r *http.Request) for { select { + case <-api.ctx.Done(): + return + case <-ctx.Done(): return + case notif := <-notificationCh: unreadCount, err := api.Database.CountUnreadInboxNotificationsByUserID(ctx, apikey.UserID) if err != nil {