From ef209c5e0b68331cde3714f9d7ac2cd4c9e7253b Mon Sep 17 00:00:00 2001
From: Agniva De Sarker <agnivade@yahoo.co.in>
Date: Mon, 6 Apr 2020 21:07:30 +0530
Subject: [PATCH] MM-23620: Handle error from GetUser (#14204)

* MM-23620: Handle error from GetUser

In case of high DB load, the DB will start to throw errors.
Unless we handle the error appropriately, the server will crash.

* Removing unnecessary lines
---
 app/channel.go      |  6 +++++-
 app/channel_test.go | 28 ++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/app/channel.go b/app/channel.go
index 903ae45c29..ad2305b019 100644
--- a/app/channel.go
+++ b/app/channel.go
@@ -2113,7 +2113,11 @@ func (a *App) MarkChannelsAsViewed(channelIds []string, userId string, currentSe
 
 			notify := member.NotifyProps[model.PUSH_NOTIFY_PROP]
 			if notify == model.CHANNEL_NOTIFY_DEFAULT {
-				user, _ := a.GetUser(userId)
+				user, err := a.GetUser(userId)
+				if err != nil {
+					mlog.Warn("Failed to get user", mlog.String("user_id", userId), mlog.Err(err))
+					continue
+				}
 				notify = user.NotifyProps[model.PUSH_NOTIFY_PROP]
 			}
 			if notify == model.USER_NOTIFY_ALL {
diff --git a/app/channel_test.go b/app/channel_test.go
index e0e4ef3905..23025ed6c0 100644
--- a/app/channel_test.go
+++ b/app/channel_test.go
@@ -5,6 +5,7 @@ package app
 
 import (
 	"fmt"
+	"net/http"
 	"sort"
 	"strings"
 	"testing"
@@ -13,6 +14,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/mattermost/mattermost-server/v5/model"
+	"github.com/mattermost/mattermost-server/v5/store/storetest/mocks"
 )
 
 func TestPermanentDeleteChannel(t *testing.T) {
@@ -1648,3 +1650,29 @@ func TestPatchChannelModerationsForChannel(t *testing.T) {
 		})
 	}
 }
+
+// TestMarkChannelsAsViewedPanic verifies that returning an error from a.GetUser
+// does not cause a panic.
+func TestMarkChannelsAsViewedPanic(t *testing.T) {
+	th := SetupWithStoreMock(t)
+	defer th.TearDown()
+
+	mockStore := th.App.Srv().Store.(*mocks.Store)
+	mockUserStore := mocks.UserStore{}
+	mockUserStore.On("Get", "userID").Return(nil, model.NewAppError("SqlUserStore.Get", "store.sql_user.get.app_error", nil, "user_id=userID", http.StatusInternalServerError))
+	mockChannelStore := mocks.ChannelStore{}
+	mockChannelStore.On("Get", "channelID", true).Return(&model.Channel{}, nil)
+	mockChannelStore.On("GetMember", "channelID", "userID").Return(&model.ChannelMember{
+		NotifyProps: model.StringMap{
+			model.PUSH_NOTIFY_PROP: model.CHANNEL_NOTIFY_DEFAULT,
+		}}, nil)
+	times := map[string]int64{
+		"userID": 1,
+	}
+	mockChannelStore.On("UpdateLastViewedAt", []string{"channelID"}, "userID").Return(times, nil)
+	mockStore.On("User").Return(&mockUserStore)
+	mockStore.On("Channel").Return(&mockChannelStore)
+
+	_, err := th.App.MarkChannelsAsViewed([]string{"channelID"}, "userID", th.App.Session().Id)
+	require.Nil(t, err)
+}