MM-40880: Sentry crash: MarkChannelAsUnread (#19370)
Use the correct error variable to prevent crash The errors.As check has been removed because ThreadStore.Get returns a nil error if no thread is found and that's how the application logic is written. So the check was redundant. ```release-note NONE ``` Co-authored-by: Mattermod <mattermod@users.noreply.github.com>
Этот коммит содержится в:
коммит произвёл
GitHub
родитель
0b46264426
Коммит
3207d861ca
@@ -2552,8 +2552,8 @@ func (a *App) MarkChannelAsUnreadFromPost(postID string, userID string, collapse
|
|||||||
return nil, model.NewAppError("MarkChannelAsUnreadFromPost", "app.channel.update_last_viewed_at_post.app_error", nil, storeErr.Error(), http.StatusInternalServerError)
|
return nil, model.NewAppError("MarkChannelAsUnreadFromPost", "app.channel.update_last_viewed_at_post.app_error", nil, storeErr.Error(), http.StatusInternalServerError)
|
||||||
}
|
}
|
||||||
threadData, storeErr2 := a.Srv().Store.Thread().Get(threadId)
|
threadData, storeErr2 := a.Srv().Store.Thread().Get(threadId)
|
||||||
if storeErr2 != nil && !errors.As(storeErr2, &nfErr) {
|
if storeErr2 != nil {
|
||||||
return nil, model.NewAppError("MarkChannelAsUnreadFromPost", "app.channel.update_last_viewed_at_post.app_error", nil, storeErr.Error(), http.StatusInternalServerError)
|
return nil, model.NewAppError("MarkChannelAsUnreadFromPost", "app.channel.update_last_viewed_at_post.app_error", nil, storeErr2.Error(), http.StatusInternalServerError)
|
||||||
}
|
}
|
||||||
if threadData != nil && threadMembership != nil && threadMembership.Following {
|
if threadData != nil && threadMembership != nil && threadMembership.Following {
|
||||||
channel, nErr := a.Srv().Store.Channel().Get(post.ChannelId, true)
|
channel, nErr := a.Srv().Store.Channel().Get(post.ChannelId, true)
|
||||||
|
|||||||
@@ -5,6 +5,7 @@ package app
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
"net/http"
|
"net/http"
|
||||||
"os"
|
"os"
|
||||||
@@ -1992,6 +1993,78 @@ func TestMarkChannelsAsViewedPanic(t *testing.T) {
|
|||||||
require.Nil(t, appErr)
|
require.Nil(t, appErr)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestMarkChannelAsUnreadFromPostPanic(t *testing.T) {
|
||||||
|
th := SetupWithStoreMock(t)
|
||||||
|
defer th.TearDown()
|
||||||
|
|
||||||
|
mockStore := th.App.Srv().Store.(*mocks.Store)
|
||||||
|
mockUserStore := mocks.UserStore{}
|
||||||
|
mockUserStore.On("Get", context.Background(), "userID").Return(&model.User{Id: "userID"}, nil)
|
||||||
|
mockUserStore.On("Count", mock.Anything).Return(int64(10), nil)
|
||||||
|
|
||||||
|
mockChannelStore := mocks.ChannelStore{}
|
||||||
|
mockChannelStore.On("Get", "channelID", true).Return(&model.Channel{Id: "channelID"}, nil)
|
||||||
|
mockChannelStore.On("GetMember", context.Background(), "channelID", "userID").Return(&model.ChannelMember{
|
||||||
|
NotifyProps: model.StringMap{
|
||||||
|
model.PushNotifyProp: model.ChannelNotifyDefault,
|
||||||
|
}}, nil)
|
||||||
|
|
||||||
|
mockSessionStore := mocks.SessionStore{}
|
||||||
|
mockOAuthStore := mocks.OAuthStore{}
|
||||||
|
var err error
|
||||||
|
th.App.ch.srv.userService, err = users.New(users.ServiceConfig{
|
||||||
|
UserStore: &mockUserStore,
|
||||||
|
SessionStore: &mockSessionStore,
|
||||||
|
OAuthStore: &mockOAuthStore,
|
||||||
|
ConfigFn: th.App.ch.srv.Config,
|
||||||
|
LicenseFn: th.App.ch.srv.License,
|
||||||
|
})
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
mockPreferenceStore := mocks.PreferenceStore{}
|
||||||
|
mockPreferenceStore.On("Get", "userID", model.PreferenceCategoryDisplaySettings, model.PreferenceNameCollapsedThreadsEnabled).Return(&model.Preference{Value: "on"}, nil)
|
||||||
|
|
||||||
|
mockThreadStore := mocks.ThreadStore{}
|
||||||
|
mockThreadStore.On("MarkAllAsReadInChannels", "userID", []string{"channelID"}).Return(nil)
|
||||||
|
mockThreadStore.On("GetMembershipForUser", "userID", "rootID").Return(nil, nil)
|
||||||
|
mockThreadStore.On("MaintainMembership", "userID", "rootID", mock.AnythingOfType("store.ThreadMembershipOpts")).Return(&model.ThreadMembership{}, nil)
|
||||||
|
mockThreadStore.On("Get", "rootID").Return(nil, errors.New("bad error")) // Returning an error from here causes the panic
|
||||||
|
|
||||||
|
mockPostStore := mocks.PostStore{}
|
||||||
|
mockPostStore.On("GetMaxPostSize").Return(65535, nil)
|
||||||
|
mockPostStore.On("Get", context.Background(), "postID", false, false, false, "userID").Return(&model.PostList{}, nil)
|
||||||
|
mockPostStore.On("GetPostsAfter", mock.AnythingOfType("model.GetPostsOptions")).Return(&model.PostList{}, nil)
|
||||||
|
mockPostStore.On("GetSingle", "postID", false).Return(&model.Post{
|
||||||
|
Id: "postID",
|
||||||
|
RootId: "rootID",
|
||||||
|
ChannelId: "channelID",
|
||||||
|
}, nil)
|
||||||
|
|
||||||
|
mockSystemStore := mocks.SystemStore{}
|
||||||
|
mockSystemStore.On("GetByName", "UpgradedFromTE").Return(&model.System{Name: "UpgradedFromTE", Value: "false"}, nil)
|
||||||
|
mockSystemStore.On("GetByName", "InstallationDate").Return(&model.System{Name: "InstallationDate", Value: "10"}, nil)
|
||||||
|
mockSystemStore.On("GetByName", "FirstServerRunTimestamp").Return(&model.System{Name: "FirstServerRunTimestamp", Value: "10"}, nil)
|
||||||
|
mockLicenseStore := mocks.LicenseStore{}
|
||||||
|
mockLicenseStore.On("Get", "").Return(&model.LicenseRecord{}, nil)
|
||||||
|
|
||||||
|
mockStore.On("Channel").Return(&mockChannelStore)
|
||||||
|
mockStore.On("Preference").Return(&mockPreferenceStore)
|
||||||
|
mockStore.On("Thread").Return(&mockThreadStore)
|
||||||
|
mockStore.On("Post").Return(&mockPostStore)
|
||||||
|
mockStore.On("User").Return(&mockUserStore)
|
||||||
|
mockStore.On("System").Return(&mockSystemStore)
|
||||||
|
mockStore.On("License").Return(&mockLicenseStore)
|
||||||
|
|
||||||
|
th.App.UpdateConfig(func(cfg *model.Config) {
|
||||||
|
*cfg.ServiceSettings.ThreadAutoFollow = true
|
||||||
|
*cfg.ServiceSettings.CollapsedThreads = model.CollapsedThreadsDefaultOn
|
||||||
|
})
|
||||||
|
|
||||||
|
require.NotPanics(t, func() {
|
||||||
|
th.App.MarkChannelAsUnreadFromPost("postID", "userID", true, true)
|
||||||
|
}, "unexpected panic from MarkChannelAsUnreadFromPost")
|
||||||
|
}
|
||||||
|
|
||||||
func TestClearChannelMembersCache(t *testing.T) {
|
func TestClearChannelMembersCache(t *testing.T) {
|
||||||
th := SetupWithStoreMock(t)
|
th := SetupWithStoreMock(t)
|
||||||
defer th.TearDown()
|
defer th.TearDown()
|
||||||
|
|||||||
Ссылка в новой задаче
Block a user