From 02bab903cfd95d4448a9955929e11a32b25f8f38 Mon Sep 17 00:00:00 2001 From: DiscoBard <223926914+DiscoBard@users.noreply.github.com> Date: Tue, 5 May 2026 10:39:34 -0400 Subject: [PATCH] fix(app): preserve session.json on transient ListenFatalError OnDisconnect was wired to fire for any libgm ListenFatalError -- including transient network or RPC failures -- and unconditionally removed session.json. On long-running headless deployments this caused a full re-pair to be required after every transient blip, since the session file would not be present at the next start. Issue #1's intent was for session.json to be removed only when the session itself is invalid (user unpaired from phone, cookies revoked). This forwards the underlying error to the OnDisconnect callback and gates removal on a 401 / SESSION_COOKIE_INVALID classification. Other fatal errors now leave the session intact so the next reconnect can succeed without re-pairing. - internal/client/events.go: OnDisconnect signature now takes an error - internal/app/app.go: classify error via isSessionInvalidated; only delete session.json on confirmed invalidation, otherwise preserve for retry - internal/app/app_test.go: TestIsSessionInvalidated covers nil, 401-status, SESSION_COOKIE_INVALID, wrapped 401, and several transient-error variants Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/app/app.go | 40 ++++++++++++++++++++++++++++++++++----- internal/app/app_test.go | 30 +++++++++++++++++++++++++++++ internal/client/events.go | 9 ++++++--- 3 files changed, 71 insertions(+), 8 deletions(-) diff --git a/internal/app/app.go b/internal/app/app.go index 746d707..67b8f44 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -330,15 +330,26 @@ func (a *App) LoadAndConnect() error { OnRealtimeGapRecovered: func(reason string) { a.StartRecentReconcile(reason) }, - OnDisconnect: func() { + OnDisconnect: func(disconnectErr error) { a.Connected.Store(false) a.setClient(nil) - if err := os.Remove(a.SessionPath); err != nil && !os.IsNotExist(err) { - a.Logger.Warn().Err(err).Msg("Failed to remove invalidated Google Messages session") + // Distinguish a true session invalidation (e.g. user unpaired + // from their phone, cookies revoked) from a transient fatal + // error (network blip, server-side RPC failure). Issue #1 + // specifies session.json should only be removed when the + // session itself is invalid; deleting on every fatal forces a + // full re-pair after every transient disconnect, which on + // long-running deployments effectively makes pairing fragile. + if isSessionInvalidated(disconnectErr) { + if err := os.Remove(a.SessionPath); err != nil && !os.IsNotExist(err) { + a.Logger.Warn().Err(err).Msg("Failed to remove invalidated Google Messages session") + } + a.setGoogleLastError("Google Messages session invalidated; pair again") + } else { + a.setGoogleLastError("Disconnected from Google Messages; will retry with existing session") } - a.setGoogleLastError("Google Messages session invalidated; pair again") a.emitStatusChange(false) - a.Logger.Warn().Msg("Disconnected from Google Messages") + a.Logger.Warn().Err(disconnectErr).Msg("Disconnected from Google Messages") }, } cli.GM.SetEventHandler(a.EventHandler.Handle) @@ -354,6 +365,25 @@ func (a *App) LoadAndConnect() error { return nil } +// isSessionInvalidated reports whether a disconnect error indicates the +// Google Messages session itself is no longer valid (the user unpaired from +// their phone, or the auth cookies were revoked) versus a transient fatal +// error from libgm (network failure, RPC error). Only the former should +// trigger session.json removal -- the latter should preserve the session so +// reconnection can succeed without a full re-pair. +// +// libgm surfaces auth invalidation as an HTTP 401 with the +// SESSION_COOKIE_INVALID error code in the response body. Both signals are +// matched here for resilience against minor wording changes. +func isSessionInvalidated(err error) bool { + if err == nil { + return false + } + msg := err.Error() + return strings.Contains(msg, "401") || + strings.Contains(msg, "SESSION_COOKIE_INVALID") +} + // Unpair deletes the session file so the app can re-pair. func (a *App) Unpair() error { a.Connected.Store(false) diff --git a/internal/app/app_test.go b/internal/app/app_test.go index 2b9afdb..d6208a9 100644 --- a/internal/app/app_test.go +++ b/internal/app/app_test.go @@ -1,6 +1,8 @@ package app import ( + "errors" + "fmt" "os" "path/filepath" "testing" @@ -56,6 +58,34 @@ func TestNewDemoUsesIsolatedTempDataDir(t *testing.T) { } } +func TestIsSessionInvalidated(t *testing.T) { + cases := []struct { + name string + err error + want bool + }{ + {"nil error", nil, false}, + {"401 status code", errors.New("http 401 while polling"), true}, + {"SESSION_COOKIE_INVALID code", errors.New("RPC failed: SESSION_COOKIE_INVALID"), true}, + {"401 in wrapped libgm error", + fmt.Errorf("listen: %w", errors.New("server returned status 401")), true}, + {"transient network failure", + errors.New("dial tcp: i/o timeout"), false}, + {"unrelated 500 error", + errors.New("http 500 internal server error"), false}, + {"transient RPC error", + errors.New("context deadline exceeded"), false}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := isSessionInvalidated(tc.err); got != tc.want { + t.Fatalf("isSessionInvalidated(%v) = %v, want %v", tc.err, got, tc.want) + } + }) + } +} + func TestDemoModeEnvParsing(t *testing.T) { t.Run("disabled when empty", func(t *testing.T) { t.Setenv("OPENMESSAGES_DEMO", "") diff --git a/internal/client/events.go b/internal/client/events.go index ac4f32e..f6ac373 100644 --- a/internal/client/events.go +++ b/internal/client/events.go @@ -13,8 +13,11 @@ import ( "github.com/maxghenis/openmessage/internal/db" ) -// OnDisconnect is called when the client fatally disconnects (e.g. unpaired). -type OnDisconnect func() +// OnDisconnect is called when the client fatally disconnects. +// The error describes the underlying cause, which the handler can inspect to +// decide whether the session itself is invalid (caller should re-pair) or the +// disconnect was transient (caller should reconnect with the existing session). +type OnDisconnect func(err error) type EventHandler struct { Store *db.Store @@ -45,7 +48,7 @@ func (h *EventHandler) Handle(rawEvt any) { case *events.ListenFatalError: h.Logger.Error().Err(evt.Error).Msg("Listen fatal error") if h.OnDisconnect != nil { - h.OnDisconnect() + h.OnDisconnect(evt.Error) } case *events.ListenTemporaryError: h.Logger.Warn().Err(evt.Error).Msg("Listen temporary error")