From 2055c49dfe33d12daa681b27a80dc48f04f99e2f Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Fri, 21 Aug 2020 21:07:07 +0530 Subject: [PATCH] MM-27916: Improve logging when session is not found (#15292) * MM-27916: Improve logging when session is not found The error handling in this code is pretty bad and the same error message happens for multiple conditions, making it difficult to diagnose the real issue. Most of the times, we get a log like: ``` "Invalid session {"error": "GetSession: Invalid session token=jodb6sau47rnugaqj1fy7khmpr, err=, "}" ``` And it could have happened from multiple places. So this log turns out to be not that useful. We improve this by populating the Error field to fix the "" issue and also add a separate detailed error field for each log line to uniquely identify each error. * fix Co-authored-by: Mattermod --- app/session.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/app/session.go b/app/session.go index c9c8eff9a5..d3e9d3139c 100644 --- a/app/session.go +++ b/app/session.go @@ -54,7 +54,7 @@ func (a *App) GetSession(token string) (*model.Session, *model.AppError) { if session, nErr = a.Srv().Store.Session().Get(token); nErr == nil { if session != nil { if session.Token != token { - return nil, model.NewAppError("GetSession", "api.context.invalid_token.error", map[string]interface{}{"Token": token, "Error": ""}, "", http.StatusUnauthorized) + return nil, model.NewAppError("GetSession", "api.context.invalid_token.error", map[string]interface{}{"Token": token, "Error": ""}, "session token is different from the one in DB", http.StatusUnauthorized) } if !session.IsExpired() { @@ -74,13 +74,15 @@ func (a *App) GetSession(token string) (*model.Session, *model.AppError) { if err.Id != "app.user_access_token.invalid_or_missing" { detailedError = err.Error() statusCode = err.StatusCode + } else { + mlog.Warn("Error while creating session for user access token", mlog.Err(err)) } - return nil, model.NewAppError("GetSession", "api.context.invalid_token.error", map[string]interface{}{"Token": token}, detailedError, statusCode) + return nil, model.NewAppError("GetSession", "api.context.invalid_token.error", map[string]interface{}{"Token": token, "Error": detailedError}, "", statusCode) } } if session == nil || session.IsExpired() { - return nil, model.NewAppError("GetSession", "api.context.invalid_token.error", map[string]interface{}{"Token": token}, "", http.StatusUnauthorized) + return nil, model.NewAppError("GetSession", "api.context.invalid_token.error", map[string]interface{}{"Token": token, "Error": ""}, "session is either nil or expired", http.StatusUnauthorized) } if *a.Config().ServiceSettings.SessionIdleTimeoutInMinutes > 0 && @@ -103,7 +105,7 @@ func (a *App) GetSession(token string) (*model.Session, *model.AppError) { mlog.Warn("Error while revoking session", mlog.Err(err)) } }) - return nil, model.NewAppError("GetSession", "api.context.invalid_token.error", map[string]interface{}{"Token": token}, "idle timeout", http.StatusUnauthorized) + return nil, model.NewAppError("GetSession", "api.context.invalid_token.error", map[string]interface{}{"Token": token, "Error": ""}, "idle timeout", http.StatusUnauthorized) } }