From ddb4c4360c12f1fc5f68075b35a077932d8ac8cd Mon Sep 17 00:00:00 2001 From: kasyap dharanikota <76957823+kasyap1234@users.noreply.github.com> Date: Mon, 5 May 2025 15:48:47 +0530 Subject: [PATCH] fix errcheck in server/channels/app/platform/session.go (#30595) Co-authored-by: Ben Schumacher --- server/.golangci.yml | 1 - server/channels/app/oauth.go | 4 ++- .../channels/app/platform/cluster_handlers.go | 8 +++-- server/channels/app/platform/session.go | 35 +++++++++++++------ server/channels/app/platform/session_test.go | 4 +-- server/channels/app/session.go | 16 ++++++--- 6 files changed, 47 insertions(+), 21 deletions(-) diff --git a/server/.golangci.yml b/server/.golangci.yml index 3d03fc0460..412d706948 100644 --- a/server/.golangci.yml +++ b/server/.golangci.yml @@ -96,7 +96,6 @@ issues: channels/app/permissions_test.go|\ channels/app/platform/helper_test.go|\ channels/app/platform/license.go|\ - channels/app/platform/session.go|\ channels/app/platform/status.go|\ channels/app/slashcommands/command_test.go|\ channels/app/slashcommands/helper_test.go|\ diff --git a/server/channels/app/oauth.go b/server/channels/app/oauth.go index 8b6885852e..e4914dcc6c 100644 --- a/server/channels/app/oauth.go +++ b/server/channels/app/oauth.go @@ -403,7 +403,9 @@ func (a *App) newSession(c request.CTX, app *model.OAuthApp, user *model.User) ( return nil, model.NewAppError("newSession", "api.oauth.get_access_token.internal_session.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } - a.ch.srv.platform.AddSessionToCache(session) + if err := a.ch.srv.platform.AddSessionToCache(session); err != nil { + c.Logger().Warn("Failed to add session to cache", mlog.Err(err)) + } return session, nil } diff --git a/server/channels/app/platform/cluster_handlers.go b/server/channels/app/platform/cluster_handlers.go index e5ced2b9b5..2fa5e2b70c 100644 --- a/server/channels/app/platform/cluster_handlers.go +++ b/server/channels/app/platform/cluster_handlers.go @@ -68,7 +68,9 @@ func (ps *PlatformService) ClearSessionCacheForUserSkipClusterSend(userID string func (ps *PlatformService) ClearSessionCacheForAllUsersSkipClusterSend() { ps.logger.Info("Purging sessions cache") - ps.ClearAllUsersSessionCacheLocal() + if err := ps.ClearAllUsersSessionCacheLocal(); err != nil { + ps.logger.Error("Failed to purge session cache", mlog.Err(err)) + } } func (ps *PlatformService) clusterClearSessionCacheForUserHandler(msg *model.ClusterMessage) { @@ -102,7 +104,9 @@ func (ps *PlatformService) invalidateWebConnSessionCacheForUserSkipClusterSend(u func (ps *PlatformService) InvalidateAllCachesSkipSend() *model.AppError { ps.logger.Info("Purging all caches") - ps.ClearAllUsersSessionCacheLocal() + if err := ps.ClearAllUsersSessionCacheLocal(); err != nil { + ps.logger.Error("Failed to purge session cache", mlog.Err(err)) + } if err := ps.statusCache.Purge(); err != nil { ps.logger.Warn("Failed to clear the status cache", mlog.Err(err)) } diff --git a/server/channels/app/platform/session.go b/server/channels/app/platform/session.go index 1847a0d7ba..bfb440083f 100644 --- a/server/channels/app/platform/session.go +++ b/server/channels/app/platform/session.go @@ -22,7 +22,9 @@ func (ps *PlatformService) CreateSession(c request.CTX, session *model.Session) return nil, err } - ps.AddSessionToCache(session) + if err := ps.AddSessionToCache(session); err != nil { + ps.Logger().Error("Failed to add session to cache", mlog.Err(err)) + } return session, nil } @@ -39,8 +41,8 @@ func (ps *PlatformService) GetLRUSessions(c request.CTX, userID string, limit ui return ps.Store.Session().GetLRUSessions(c, userID, limit, offset) } -func (ps *PlatformService) AddSessionToCache(session *model.Session) { - ps.sessionCache.SetWithExpiry(session.Token, session, time.Duration(int64(*ps.Config().ServiceSettings.SessionCacheInMinutes))*time.Minute) +func (ps *PlatformService) AddSessionToCache(session *model.Session) error { + return ps.sessionCache.SetWithExpiry(session.Token, session, time.Duration(int64(*ps.Config().ServiceSettings.SessionCacheInMinutes))*time.Minute) } func (ps *PlatformService) ClearUserSessionCacheLocal(userID string) { @@ -90,8 +92,8 @@ func (ps *PlatformService) ClearUserSessionCacheLocal(userID string) { } } -func (ps *PlatformService) ClearAllUsersSessionCacheLocal() { - ps.sessionCache.Purge() +func (ps *PlatformService) ClearAllUsersSessionCacheLocal() error { + return ps.sessionCache.Purge() } func (ps *PlatformService) ClearUserSessionCache(userID string) { @@ -107,8 +109,10 @@ func (ps *PlatformService) ClearUserSessionCache(userID string) { } } -func (ps *PlatformService) ClearAllUsersSessionCache() { - ps.ClearAllUsersSessionCacheLocal() +func (ps *PlatformService) ClearAllUsersSessionCache() error { + if err := ps.ClearAllUsersSessionCacheLocal(); err != nil { + return err + } if ps.clusterIFace != nil { msg := &model.ClusterMessage{ @@ -117,6 +121,7 @@ func (ps *PlatformService) ClearAllUsersSessionCache() { } ps.clusterIFace.SendClusterMessage(msg) } + return nil } func (ps *PlatformService) GetSession(c request.CTX, token string) (*model.Session, error) { @@ -153,7 +158,9 @@ func (ps *PlatformService) RevokeSessionsFromAllUsers() error { return err } - ps.ClearAllUsersSessionCache() + if err := ps.ClearAllUsersSessionCache(); err != nil { + ps.logger.Error("Failed to clear session cache", mlog.Err(err)) + } return nil } @@ -238,7 +245,9 @@ func (ps *PlatformService) ExtendSessionExpiry(session *model.Session, newExpiry // ensures each node will get an extended expiry within the next 10 minutes. // Worst case is another node may generate a redundant expiry update. session.ExpiresAt = newExpiry - ps.AddSessionToCache(session) + if err := ps.AddSessionToCache(session); err != nil { + ps.Logger().Error("Failed to update session cache", mlog.Err(err)) + } return nil } @@ -261,7 +270,9 @@ func (ps *PlatformService) UpdateSessionsIsGuest(c request.CTX, user *model.User c.Logger().Warn("Unable to update isGuest session", mlog.Err(err)) continue } - ps.AddSessionToCache(session) + if err := ps.AddSessionToCache(session); err != nil { + ps.Logger().Error("Failed to update session cache", mlog.Err(err)) + } } return nil } @@ -273,7 +284,9 @@ func (ps *PlatformService) RevokeAllSessions(c request.CTX, userID string) error } for _, session := range sessions { if session.IsOAuth { - ps.RevokeAccessToken(c, session.Token) + if err := ps.RevokeAccessToken(c, session.Token); err != nil { + return err + } } else { if err := ps.Store.Session().Remove(session.Id); err != nil { return fmt.Errorf("%s: %w", err.Error(), DeleteSessionError) diff --git a/server/channels/app/platform/session_test.go b/server/channels/app/platform/session_test.go index f353a6e62b..bca401db72 100644 --- a/server/channels/app/platform/session_test.go +++ b/server/channels/app/platform/session_test.go @@ -60,8 +60,8 @@ func TestCache(t *testing.T) { clear(rkeys) rkeys = []string{} - th.Service.ClearAllUsersSessionCache() - + err = th.Service.ClearAllUsersSessionCache() + require.NoError(t, err) err = th.Service.sessionCache.Scan(func(in []string) error { rkeys = append(rkeys, in...) return nil diff --git a/server/channels/app/session.go b/server/channels/app/session.go index 2ccc00dccb..30bd18ea45 100644 --- a/server/channels/app/session.go +++ b/server/channels/app/session.go @@ -96,7 +96,9 @@ func (a *App) GetSession(token string) (*model.Session, *model.AppError) { } if !session.IsExpired() { - a.ch.srv.platform.AddSessionToCache(session) + if err := a.ch.srv.platform.AddSessionToCache(session); err != nil { + c.Logger().Error("Failed to add session to cache", mlog.Err(err)) + } } } @@ -205,7 +207,9 @@ func (a *App) RevokeAllSessions(c request.CTX, userID string) *model.AppError { } func (a *App) AddSessionToCache(session *model.Session) { - a.ch.srv.platform.AddSessionToCache(session) + if err := a.ch.srv.platform.AddSessionToCache(session); err != nil { + a.Srv().Platform().Log().Error("Failed to add session to cache", mlog.String("session_id", session.Id), mlog.String("user_id", session.UserId), mlog.Err(err)) + } } // RevokeSessionsFromAllUsers will go through all the sessions active @@ -228,7 +232,9 @@ func (a *App) ClearSessionCacheForUser(userID string) { } func (a *App) ClearSessionCacheForAllUsers() { - a.ch.srv.platform.ClearAllUsersSessionCache() + if err := a.ch.srv.platform.ClearAllUsersSessionCache(); err != nil { + a.Srv().Platform().Log().Error("Failed to clear session cache for all users", mlog.Err(err)) + } } func (a *App) ClearSessionCacheForUserSkipClusterSend(userID string) { @@ -490,7 +496,9 @@ func (a *App) createSessionForUserAccessToken(c request.CTX, tokenString string) } } - a.ch.srv.platform.AddSessionToCache(session) + if err := a.ch.srv.platform.AddSessionToCache(session); err != nil { + a.ch.srv.Log().Error("Failed to add session to cache", mlog.String("session_id", session.Id), mlog.Err(err)) + } return session, nil }