[PR #387] [MERGED] Detect dead/half-open control websocket (read deadline + protocol ping) #8584

New Issue

GiteaMirror · 2026-07-13T14:54:59-05:00

GiteaMirror commented

2026-07-13 14:54:59 -05:00

📋 Pull Request Information

Original PR: https://github.com/fosrl/newt/pull/387
Author: @DanivosYoun
Created: 6/22/2026
Status: ✅ Merged
Merged: 6/23/2026
Merged by: @oschwartz10612

Base: main ← Head: fix/ws-half-open-detection

📝 Commits (1)

855b2b2 fix(websocket): detect dead/half-open control connection (read deadline + protocol ping)

📊 Changes

1 file changed (+50 additions, -7 deletions)

View changed files

📝 websocket/client.go (+50 -7)

📄 Description

Problem

When the backend API server has an outage and then recovers, newt does not reconnect on its own — it stays stuck and has to be restarted by hand before it works again. This reproduces reliably when newt connects through a cloud load balancer / reverse proxy in front of the API server.

Root cause

The control websocket (websocket/client.go) has no read deadline, and the keepalive is an application-level newt/ping JSON message, not a protocol PING frame:

readPumpWithDisconnectDetection calls conn.ReadMessage() with no deadline, so on a half-open connection it blocks forever.
sendPing does WriteJSON({type:"newt/ping"}). On a half-open socket (e.g. an LB holding the front-end TCP open after the backend died/restarted) these writes are buffered and keep succeeding, so no write error is ever raised.
The existing SetPongHandler is effectively dead code, because newt never sends a protocol PING, so no PONG is ever received.

Net effect: newt believes it is still connected to a peer that is gone, never tears the connection down, and never reconnects — until the process is restarted.

Fix

Standard gorilla websocket keepalive, plus two related cleanups:

Read deadline: arm SetReadDeadline(pongWait) on connect, where pongWait = 2 × pingInterval (min 20s), and refresh it on every pong and on every inbound message (covers servers that answer newt/ping with a message rather than a protocol pong).
Protocol PING: send a websocket.PingMessage alongside the existing app ping; a compliant server auto-replies with a PONG that refreshes the deadline. If the peer is gone, no pong arrives, the read deadline expires, ReadMessage errors, and the read pump reconnects.
Single reconnect path: on ping-write failure, close the connection and let the read pump perform the one reconnect, instead of sendPing and the read-pump defer both calling reconnect() (which could race two connections).
No ping-monitor leak: each connection gets a lifecycle channel so its ping monitor stops when the connection ends (previously one ping-monitor goroutine leaked per reconnect).

No server-side changes required — protocol PING/PONG is handled automatically by any standards-compliant websocket server.

Testing

go build ./... (all non-example packages), go vet ./websocket/, and go test ./websocket/ pass.
Behaviour: with the API server behind a proxy, dropping/restarting the backend now causes newt to detect the dead connection within ~pongWait and reconnect automatically — no manual restart.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/fosrl/newt/pull/387 **Author:** [@DanivosYoun](https://github.com/DanivosYoun) **Created:** 6/22/2026 **Status:** ✅ Merged **Merged:** 6/23/2026 **Merged by:** [@oschwartz10612](https://github.com/oschwartz10612) **Base:** `main` ← **Head:** `fix/ws-half-open-detection` --- ### 📝 Commits (1) - [`855b2b2`](https://github.com/fosrl/newt/commit/855b2b272fa5b74ac23e95bf07ea87b1c302ec32) fix(websocket): detect dead/half-open control connection (read deadline + protocol ping) ### 📊 Changes **1 file changed** (+50 additions, -7 deletions) <details> <summary>View changed files</summary> 📝 `websocket/client.go` (+50 -7) </details> ### 📄 Description ## Problem When the backend API server has an outage and then recovers, newt does **not** reconnect on its own — it stays stuck and has to be **restarted by hand** before it works again. This reproduces reliably when newt connects through a cloud load balancer / reverse proxy in front of the API server. ## Root cause The control websocket (`websocket/client.go`) has **no read deadline**, and the keepalive is an **application-level `newt/ping` JSON message**, not a protocol PING frame: - `readPumpWithDisconnectDetection` calls `conn.ReadMessage()` with no deadline, so on a half-open connection it blocks **forever**. - `sendPing` does `WriteJSON({type:"newt/ping"})`. On a half-open socket (e.g. an LB holding the front-end TCP open after the backend died/restarted) these writes are buffered and keep **succeeding**, so no write error is ever raised. - The existing `SetPongHandler` is effectively dead code, because newt never sends a protocol PING, so no PONG is ever received. Net effect: newt believes it is still connected to a peer that is gone, never tears the connection down, and never reconnects — until the process is restarted. ## Fix Standard gorilla websocket keepalive, plus two related cleanups: - **Read deadline**: arm `SetReadDeadline(pongWait)` on connect, where `pongWait = 2 × pingInterval` (min 20s), and refresh it on every pong **and** on every inbound message (covers servers that answer `newt/ping` with a message rather than a protocol pong). - **Protocol PING**: send a `websocket.PingMessage` alongside the existing app ping; a compliant server auto-replies with a PONG that refreshes the deadline. If the peer is gone, no pong arrives, the read deadline expires, `ReadMessage` errors, and the read pump reconnects. - **Single reconnect path**: on ping-write failure, close the connection and let the read pump perform the one reconnect, instead of `sendPing` and the read-pump defer both calling `reconnect()` (which could race two connections). - **No ping-monitor leak**: each connection gets a lifecycle channel so its ping monitor stops when the connection ends (previously one ping-monitor goroutine leaked per reconnect). No server-side changes required — protocol PING/PONG is handled automatically by any standards-compliant websocket server. ## Testing - `go build ./...` (all non-example packages), `go vet ./websocket/`, and `go test ./websocket/` pass. - Behaviour: with the API server behind a proxy, dropping/restarting the backend now causes newt to detect the dead connection within ~`pongWait` and reconnect automatically — no manual restart. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

GiteaMirror added the pull-request label 2026-07-13 14:54:59 -05:00

GiteaMirror closed this issue

2026-07-13 14:55:00 -05:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: github-starred/newt#8584