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")