diff --git a/cmd/meshtender/main.go b/cmd/meshtender/main.go index 69cf49d..953aec3 100644 --- a/cmd/meshtender/main.go +++ b/cmd/meshtender/main.go @@ -150,9 +150,10 @@ func run(logger *slog.Logger) error { } // Shutdown doesn't close hijacked/WebSocket connections, so close the active - // console/confirm sockets and give their handlers a moment to finish (they do - // DB work) before we stop the flusher and close the pool. - wsCtx, wsCancel := context.WithTimeout(context.Background(), 5*time.Second) + // console/confirm sockets and give their handlers time to unwind and stamp + // their session's ended_at (see core.WSDrainTimeout) before we stop the flusher + // and close the pool. Too short and drained sessions stay "in progress" forever. + wsCtx, wsCancel := context.WithTimeout(context.Background(), core.WSDrainTimeout) if !srv.DrainWebSockets(wsCtx) { logger.Warn("shutdown: some WebSocket handlers did not finish before the deadline") } diff --git a/internal/core/console.go b/internal/core/console.go index 2bbea72..cc3e9f7 100644 --- a/internal/core/console.go +++ b/internal/core/console.go @@ -22,6 +22,13 @@ import ( const ( consoleIdleTimeout = 5 * time.Minute + // consoleEndTimeout bounds the deferred EndConsoleSession stamp that runs when + // a console handler returns (including on shutdown drain). It uses a fresh + // background context — the session context is already cancelled by then — so + // the stamp still lands. WSDrainTimeout MUST exceed this (plus unwind slack) or + // the drain gives up before the stamp completes and the session is orphaned as + // "in progress" forever (see TestDrainWebSockets). + consoleEndTimeout = 5 * time.Second // maxCommandLen bounds a single CLI command; MeshCore commands are short and // a LoRa frame is tiny, so anything longer is malformed/abusive. maxCommandLen = 200 @@ -227,7 +234,7 @@ func (s *Handlers) wsConsole(w http.ResponseWriter, r *http.Request) { return } defer func() { - endCtx, c := context.WithTimeout(context.Background(), 5*time.Second) + endCtx, c := context.WithTimeout(context.Background(), consoleEndTimeout) defer c() _ = s.Store.EndConsoleSession(endCtx, sessionID) }() diff --git a/internal/core/web.go b/internal/core/web.go index 8320a4b..84ed09f 100644 --- a/internal/core/web.go +++ b/internal/core/web.go @@ -9,6 +9,7 @@ import ( "net" "net/http" "sync" + "time" "github.com/go-chi/chi/v5" @@ -47,6 +48,15 @@ type Server struct { // Handler returns the root HTTP handler. func (s *Server) Handler() http.Handler { return s.handler } +// WSDrainTimeout is the recommended deadline for DrainWebSockets on shutdown. It +// must comfortably exceed a single handler's consoleEndTimeout (the deferred +// EndConsoleSession stamp) plus the time a handler needs to unwind after its +// context is cancelled — otherwise the drain gives up mid-stamp and leaves the +// session's ended_at NULL, so it shows "in progress" forever. It must also fit +// inside the deployment's process stop grace (Kubernetes default 30s), alongside +// the preceding HTTP drain. TestDrainTimeoutExceedsStamp enforces the lower bound. +const WSDrainTimeout = 15 * time.Second + // DrainWebSockets closes active WebSocket connections and waits for their handlers // to return, up to ctx's deadline. Call it after http.Server.Shutdown, which does // not close hijacked/WebSocket connections. Reports whether all handlers finished diff --git a/internal/core/ws_drain_test.go b/internal/core/ws_drain_test.go index 45339e5..3e1c512 100644 --- a/internal/core/ws_drain_test.go +++ b/internal/core/ws_drain_test.go @@ -108,4 +108,28 @@ func TestDrainWebSockets(t *testing.T) { } break // socket closed } + + // The handler's deferred EndConsoleSession must have stamped ended_at during the + // drain. If it hasn't, the session shows "in progress" forever — the exact bug + // the drain budget (WSDrainTimeout > consoleEndTimeout) exists to prevent. + var open int + if err := st.Pool().QueryRow(ctx, + `SELECT count(*) FROM console_sessions WHERE repeater_id = $1 AND ended_at IS NULL`, + rep.ID).Scan(&open); err != nil { + t.Fatalf("query sessions: %v", err) + } + if open != 0 { + t.Fatalf("drain left %d console session(s) unstamped (ended_at IS NULL) — would show 'in progress' forever", open) + } +} + +// TestDrainTimeoutExceedsStamp guards the invariant the drain budget depends on: +// the shutdown drain deadline must comfortably exceed a single handler's +// EndConsoleSession stamp timeout, or the drain gives up mid-stamp and orphans the +// session as "in progress". +func TestDrainTimeoutExceedsStamp(t *testing.T) { + t.Parallel() + if WSDrainTimeout <= consoleEndTimeout { + t.Fatalf("WSDrainTimeout (%s) must exceed consoleEndTimeout (%s) so the drain can complete the ended_at stamp", WSDrainTimeout, consoleEndTimeout) + } }