diff --git a/api4/group_test.go b/api4/group_test.go index ca1f457fb4..64d6b9a6f4 100644 --- a/api4/group_test.go +++ b/api4/group_test.go @@ -714,7 +714,7 @@ func TestGetGroupsByChannel(t *testing.T) { require.True(t, *groups[0].SchemeAdmin) groups, _, response = client.GetGroupsByChannel(model.NewId(), opts) - assert.Equal(t, "store.sql_channel.get.existing.app_error", response.Error.Id) + assert.Equal(t, "app.channel.get.existing.app_error", response.Error.Id) assert.Empty(t, groups) }) } diff --git a/app/channel.go b/app/channel.go index 11d8faf8c4..21b2615671 100644 --- a/app/channel.go +++ b/app/channel.go @@ -1471,14 +1471,15 @@ func (a *App) PostUpdateChannelDisplayNameMessage(userId string, channel *model. } func (a *App) GetChannel(channelId string) (*model.Channel, *model.AppError) { - channel, errCh := a.Srv().Store.Channel().Get(channelId, true) - if errCh != nil { - if errCh.Id == "store.sql_channel.get.existing.app_error" { - errCh.StatusCode = http.StatusNotFound - return nil, errCh + channel, err := a.Srv().Store.Channel().Get(channelId, true) + if err != nil { + var nfErr *store.ErrNotFound + switch { + case errors.As(err, &nfErr): + return nil, model.NewAppError("GetChannel", "app.channel.get.existing.app_error", nil, nfErr.Error(), http.StatusNotFound) + default: + return nil, model.NewAppError("GetChannel", "app.channel.get.find.app_error", nil, err.Error(), http.StatusInternalServerError) } - errCh.StatusCode = http.StatusBadRequest - return nil, errCh } return channel, nil } @@ -1772,7 +1773,7 @@ func (a *App) LeaveChannel(channelId string, userId string) *model.AppError { sc := make(chan store.StoreResult, 1) go func() { channel, err := a.Srv().Store.Channel().Get(channelId, true) - sc <- store.StoreResult{Data: channel, Err: err} + sc <- store.StoreResult{Data: channel, NErr: err} close(sc) }() @@ -1795,8 +1796,14 @@ func (a *App) LeaveChannel(channelId string, userId string) *model.AppError { return cresult.Err } uresult := <-uc - if uresult.Err != nil { - return cresult.Err + if uresult.NErr != nil { + var nfErr *store.ErrNotFound + switch { + case errors.As(uresult.NErr, &nfErr): + return model.NewAppError("LeaveChannel", "app.channel.get.existing.app_error", nil, nfErr.Error(), http.StatusNotFound) + default: + return model.NewAppError("LeaveChannel", "app.channel.get.find.app_error", nil, uresult.NErr.Error(), http.StatusInternalServerError) + } } ccresult := <-mcc if ccresult.Err != nil { diff --git a/app/command.go b/app/command.go index a44f2161ff..1b6b4d066a 100644 --- a/app/command.go +++ b/app/command.go @@ -4,6 +4,7 @@ package app import ( + "errors" "io" "io/ioutil" "net/http" @@ -344,7 +345,7 @@ func (a *App) tryExecuteCustomCommand(args *model.CommandArgs, trigger string, m chanChan := make(chan store.StoreResult, 1) go func() { channel, err := a.Srv().Store.Channel().Get(args.ChannelId, true) - chanChan <- store.StoreResult{Data: channel, Err: err} + chanChan <- store.StoreResult{Data: channel, NErr: err} close(chanChan) }() @@ -380,8 +381,14 @@ func (a *App) tryExecuteCustomCommand(args *model.CommandArgs, trigger string, m user := ur.Data.(*model.User) cr := <-chanChan - if cr.Err != nil { - return nil, nil, cr.Err + if cr.NErr != nil { + var nfErr *store.ErrNotFound + switch { + case errors.As(cr.NErr, &nfErr): + return nil, nil, model.NewAppError("tryExecuteCustomCommand", "app.channel.get.existing.app_error", nil, nfErr.Error(), http.StatusNotFound) + default: + return nil, nil, model.NewAppError("tryExecuteCustomCommand", "app.channel.get.find.app_error", nil, cr.NErr.Error(), http.StatusInternalServerError) + } } channel := cr.Data.(*model.Channel) diff --git a/app/group.go b/app/group.go index 29bbe0d4ee..9ffd1b5727 100644 --- a/app/group.go +++ b/app/group.go @@ -4,9 +4,11 @@ package app import ( + "errors" "net/http" "github.com/mattermost/mattermost-server/v5/model" + "github.com/mattermost/mattermost-server/v5/store" ) func (a *App) GetGroup(id string) (*model.Group, *model.AppError) { @@ -90,10 +92,15 @@ func (a *App) UpsertGroupSyncable(groupSyncable *model.GroupSyncable) (*model.Gr // reject the syncable creation if the group isn't already associated to the parent team if groupSyncable.Type == model.GroupSyncableTypeChannel { - var channel *model.Channel - channel, err = a.Srv().Store.Channel().Get(groupSyncable.SyncableId, true) - if err != nil { - return nil, err + channel, nErr := a.Srv().Store.Channel().Get(groupSyncable.SyncableId, true) + if nErr != nil { + var nfErr *store.ErrNotFound + switch { + case errors.As(nErr, &nfErr): + return nil, model.NewAppError("UpsertGroupSyncable", "app.channel.get.existing.app_error", nil, nfErr.Error(), http.StatusNotFound) + default: + return nil, model.NewAppError("UpsertGroupSyncable", "app.channel.get.find.app_error", nil, nErr.Error(), http.StatusInternalServerError) + } } var team *model.Team diff --git a/app/integration_action.go b/app/integration_action.go index bb3eb0db8d..2be132bda0 100644 --- a/app/integration_action.go +++ b/app/integration_action.go @@ -20,6 +20,7 @@ package app import ( "bytes" "encoding/json" + "errors" "fmt" "io/ioutil" "net/http" @@ -101,7 +102,13 @@ func (a *App) DoPostActionWithCookie(postId, actionId, userId, selectedOption st channel, err := a.Srv().Store.Channel().Get(cookie.ChannelId, true) if err != nil { - return "", err + var nfErr *store.ErrNotFound + switch { + case errors.As(err, &nfErr): + return "", model.NewAppError("DoPostActionWithCookie", "app.channel.get.existing.app_error", nil, nfErr.Error(), http.StatusNotFound) + default: + return "", model.NewAppError("DoPostActionWithCookie", "app.channel.get.find.app_error", nil, err.Error(), http.StatusInternalServerError) + } } upstreamRequest.ChannelId = cookie.ChannelId diff --git a/app/post.go b/app/post.go index 847470f966..2ec73840a0 100644 --- a/app/post.go +++ b/app/post.go @@ -5,6 +5,7 @@ package app import ( "encoding/json" + "errors" "fmt" "net/http" "strings" @@ -94,7 +95,13 @@ func (a *App) CreatePostAsUser(post *model.Post, currentSessionId string, setOnl func (a *App) CreatePostMissingChannel(post *model.Post, triggerWebhooks bool) (*model.Post, *model.AppError) { channel, err := a.Srv().Store.Channel().Get(post.ChannelId, true) if err != nil { - return nil, err + var nfErr *store.ErrNotFound + switch { + case errors.As(err, &nfErr): + return nil, model.NewAppError("CreatePostMissingChannel", "app.channel.get.existing.app_error", nil, nfErr.Error(), http.StatusNotFound) + default: + return nil, model.NewAppError("CreatePostMissingChannel", "app.channel.get.find.app_error", nil, err.Error(), http.StatusInternalServerError) + } } return a.CreatePost(post, channel, triggerWebhooks, true) diff --git a/app/webhook.go b/app/webhook.go index 87c650f765..9731d8177d 100644 --- a/app/webhook.go +++ b/app/webhook.go @@ -4,6 +4,7 @@ package app import ( + "errors" "io" "net/http" "regexp" @@ -412,7 +413,13 @@ func (a *App) CreateOutgoingWebhook(hook *model.OutgoingWebhook) (*model.Outgoin if len(hook.ChannelId) != 0 { channel, errCh := a.Srv().Store.Channel().Get(hook.ChannelId, true) if errCh != nil { - return nil, errCh + var nfErr *store.ErrNotFound + switch { + case errors.As(errCh, &nfErr): + return nil, model.NewAppError("CreateOutgoingWebhook", "app.channel.get.existing.app_error", nil, nfErr.Error(), http.StatusNotFound) + default: + return nil, model.NewAppError("CreateOutgoingWebhook", "app.channel.get.find.app_error", nil, errCh.Error(), http.StatusInternalServerError) + } } if channel.Type != model.CHANNEL_OPEN { @@ -633,10 +640,16 @@ func (a *App) HandleIncomingWebhook(hookId string, req *model.IncomingWebhookReq }() } } else { - var err *model.AppError + var err error channel, err = a.Srv().Store.Channel().Get(hook.ChannelId, true) if err != nil { - return model.NewAppError("HandleIncomingWebhook", "web.incoming_webhook.channel.app_error", nil, "err="+err.Message, err.StatusCode) + var nfErr *store.ErrNotFound + switch { + case errors.As(err, &nfErr): + return model.NewAppError("HandleIncomingWebhook", "app.channel.get.existing.app_error", nil, nfErr.Error(), http.StatusNotFound) + default: + return model.NewAppError("HandleIncomingWebhook", "app.channel.get.find.app_error", nil, err.Error(), http.StatusInternalServerError) + } } } diff --git a/i18n/en.json b/i18n/en.json index 8a341d2db4..cdc44676d6 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -2974,6 +2974,14 @@ "id": "app.channel.create_direct_channel.internal_error", "translation": "Unable to save direct channel." }, + { + "id": "app.channel.get.existing.app_error", + "translation": "Unable to find the existing channel." + }, + { + "id": "app.channel.get.find.app_error", + "translation": "We encountered an error finding the channel." + }, { "id": "app.channel.move_channel.members_do_not_match.error", "translation": "Unable to move a channel unless all its members are already members of the destination team." diff --git a/store/localcachelayer/channel_layer.go b/store/localcachelayer/channel_layer.go index b7b4e9fd4b..d97a6d7a2a 100644 --- a/store/localcachelayer/channel_layer.go +++ b/store/localcachelayer/channel_layer.go @@ -150,7 +150,7 @@ func (s LocalCacheChannelStore) GetPinnedPostCount(channelId string, allowFromCa return count, nil } -func (s LocalCacheChannelStore) Get(id string, allowFromCache bool) (*model.Channel, *model.AppError) { +func (s LocalCacheChannelStore) Get(id string, allowFromCache bool) (*model.Channel, error) { if allowFromCache { if cacheItem := s.rootStore.doStandardReadCache(s.rootStore.channelByIdCache, id); cacheItem != nil { diff --git a/store/opentracing_layer.go b/store/opentracing_layer.go index 1bde5af9a8..3d7f6e0667 100644 --- a/store/opentracing_layer.go +++ b/store/opentracing_layer.go @@ -630,7 +630,7 @@ func (s *OpenTracingLayerChannelStore) Delete(channelId string, time int64) *mod return resultVar0 } -func (s *OpenTracingLayerChannelStore) Get(id string, allowFromCache bool) (*model.Channel, *model.AppError) { +func (s *OpenTracingLayerChannelStore) Get(id string, allowFromCache bool) (*model.Channel, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "ChannelStore.Get") s.Root.Store.SetContext(newCtx) @@ -1026,7 +1026,7 @@ func (s *OpenTracingLayerChannelStore) GetForPost(postId string) (*model.Channel return resultVar0, resultVar1 } -func (s *OpenTracingLayerChannelStore) GetFromMaster(id string) (*model.Channel, *model.AppError) { +func (s *OpenTracingLayerChannelStore) GetFromMaster(id string) (*model.Channel, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "ChannelStore.GetFromMaster") s.Root.Store.SetContext(newCtx) diff --git a/store/sqlstore/channel_store.go b/store/sqlstore/channel_store.go index 97c0a8ea9a..5afc74dd30 100644 --- a/store/sqlstore/channel_store.go +++ b/store/sqlstore/channel_store.go @@ -725,7 +725,7 @@ func (s SqlChannelStore) InvalidateChannelByName(teamId, name string) { } } -func (s SqlChannelStore) Get(id string, allowFromCache bool) (*model.Channel, *model.AppError) { +func (s SqlChannelStore) Get(id string, allowFromCache bool) (*model.Channel, error) { return s.get(id, false, allowFromCache) } @@ -743,11 +743,11 @@ func (s SqlChannelStore) GetPinnedPosts(channelId string) (*model.PostList, *mod return pl, nil } -func (s SqlChannelStore) GetFromMaster(id string) (*model.Channel, *model.AppError) { +func (s SqlChannelStore) GetFromMaster(id string) (*model.Channel, error) { return s.get(id, true, false) } -func (s SqlChannelStore) get(id string, master bool, allowFromCache bool) (*model.Channel, *model.AppError) { +func (s SqlChannelStore) get(id string, master bool, allowFromCache bool) (*model.Channel, error) { var db *gorp.DbMap if master { @@ -758,11 +758,11 @@ func (s SqlChannelStore) get(id string, master bool, allowFromCache bool) (*mode obj, err := db.Get(model.Channel{}, id) if err != nil { - return nil, model.NewAppError("SqlChannelStore.Get", "store.sql_channel.get.find.app_error", nil, "id="+id+", "+err.Error(), http.StatusInternalServerError) + return nil, errors.Wrapf(err, "failed to find channel with id = %s", id) } if obj == nil { - return nil, model.NewAppError("SqlChannelStore.Get", "store.sql_channel.get.existing.app_error", nil, "id="+id, http.StatusNotFound) + return nil, store.NewErrNotFound("Channel", id) } ch := obj.(*model.Channel) diff --git a/store/sqlstore/group_store.go b/store/sqlstore/group_store.go index 331d3c2e99..5abb2b4c27 100644 --- a/store/sqlstore/group_store.go +++ b/store/sqlstore/group_store.go @@ -10,6 +10,7 @@ import ( "strings" sq "github.com/Masterminds/squirrel" + "github.com/pkg/errors" "github.com/mattermost/mattermost-server/v5/model" "github.com/mattermost/mattermost-server/v5/store" @@ -480,7 +481,13 @@ func (s *SqlGroupStore) CreateGroupSyncable(groupSyncable *model.GroupSyncable) insertErr = s.GetMaster().Insert(groupSyncableToGroupTeam(groupSyncable)) case model.GroupSyncableTypeChannel: if _, err := s.Channel().Get(groupSyncable.SyncableId, false); err != nil { - return nil, err + var nfErr *store.ErrNotFound + switch { + case errors.As(err, &nfErr): + return nil, model.NewAppError("CreateGroupSyncable", "store.sql_channel.get.existing.app_error", nil, nfErr.Error(), http.StatusNotFound) + default: + return nil, model.NewAppError("CreateGroupSyncable", "store.sql_channel.get.find.app_error", nil, err.Error(), http.StatusInternalServerError) + } } insertErr = s.GetMaster().Insert(groupSyncableToGroupChannel(groupSyncable)) diff --git a/store/store.go b/store/store.go index 1c08c145a1..54eb2e72b0 100644 --- a/store/store.go +++ b/store/store.go @@ -15,6 +15,9 @@ import ( type StoreResult struct { Data interface{} Err *model.AppError + + // NErr a temporary field used by the new code for the AppError migration. This will later become Err when the entire store is migrated. + NErr error } type Store interface { @@ -131,10 +134,10 @@ type ChannelStore interface { CreateDirectChannel(userId *model.User, otherUserId *model.User) (*model.Channel, error) SaveDirectChannel(channel *model.Channel, member1 *model.ChannelMember, member2 *model.ChannelMember) (*model.Channel, error) Update(channel *model.Channel) (*model.Channel, error) - Get(id string, allowFromCache bool) (*model.Channel, *model.AppError) + Get(id string, allowFromCache bool) (*model.Channel, error) InvalidateChannel(id string) InvalidateChannelByName(teamId, name string) - GetFromMaster(id string) (*model.Channel, *model.AppError) + GetFromMaster(id string) (*model.Channel, error) Delete(channelId string, time int64) *model.AppError Restore(channelId string, time int64) *model.AppError SetDeleteAt(channelId string, deleteAt int64, updateAt int64) *model.AppError diff --git a/store/storetest/mocks/ChannelStore.go b/store/storetest/mocks/ChannelStore.go index 9551ecbf32..f468b7e12f 100644 --- a/store/storetest/mocks/ChannelStore.go +++ b/store/storetest/mocks/ChannelStore.go @@ -195,7 +195,7 @@ func (_m *ChannelStore) Delete(channelId string, time int64) *model.AppError { } // Get provides a mock function with given fields: id, allowFromCache -func (_m *ChannelStore) Get(id string, allowFromCache bool) (*model.Channel, *model.AppError) { +func (_m *ChannelStore) Get(id string, allowFromCache bool) (*model.Channel, error) { ret := _m.Called(id, allowFromCache) var r0 *model.Channel @@ -207,13 +207,11 @@ func (_m *ChannelStore) Get(id string, allowFromCache bool) (*model.Channel, *mo } } - var r1 *model.AppError - if rf, ok := ret.Get(1).(func(string, bool) *model.AppError); ok { + var r1 error + if rf, ok := ret.Get(1).(func(string, bool) error); ok { r1 = rf(id, allowFromCache) } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } + r1 = ret.Error(1) } return r0, r1 @@ -743,7 +741,7 @@ func (_m *ChannelStore) GetForPost(postId string) (*model.Channel, *model.AppErr } // GetFromMaster provides a mock function with given fields: id -func (_m *ChannelStore) GetFromMaster(id string) (*model.Channel, *model.AppError) { +func (_m *ChannelStore) GetFromMaster(id string) (*model.Channel, error) { ret := _m.Called(id) var r0 *model.Channel @@ -755,13 +753,11 @@ func (_m *ChannelStore) GetFromMaster(id string) (*model.Channel, *model.AppErro } } - var r1 *model.AppError - if rf, ok := ret.Get(1).(func(string) *model.AppError); ok { + var r1 error + if rf, ok := ret.Get(1).(func(string) error); ok { r1 = rf(id) } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } + r1 = ret.Error(1) } return r0, r1 diff --git a/store/storetest/scheme_store.go b/store/storetest/scheme_store.go index 4273283424..41cd53043e 100644 --- a/store/storetest/scheme_store.go +++ b/store/storetest/scheme_store.go @@ -450,8 +450,8 @@ func testSchemeStoreDelete(t *testing.T, ss store.Store) { _, err = ss.Scheme().Delete(d5.Id) assert.Nil(t, err) - c6, err := ss.Channel().Get(c5.Id, true) - assert.Nil(t, err) + c6, nErr := ss.Channel().Get(c5.Id, true) + assert.Nil(t, nErr) assert.Equal(t, "", *c6.SchemeId) } diff --git a/store/timer_layer.go b/store/timer_layer.go index dfd6349c35..2232bd5a35 100644 --- a/store/timer_layer.go +++ b/store/timer_layer.go @@ -600,7 +600,7 @@ func (s *TimerLayerChannelStore) Delete(channelId string, time int64) *model.App return resultVar0 } -func (s *TimerLayerChannelStore) Get(id string, allowFromCache bool) (*model.Channel, *model.AppError) { +func (s *TimerLayerChannelStore) Get(id string, allowFromCache bool) (*model.Channel, error) { start := timemodule.Now() resultVar0, resultVar1 := s.ChannelStore.Get(id, allowFromCache) @@ -952,7 +952,7 @@ func (s *TimerLayerChannelStore) GetForPost(postId string) (*model.Channel, *mod return resultVar0, resultVar1 } -func (s *TimerLayerChannelStore) GetFromMaster(id string) (*model.Channel, *model.AppError) { +func (s *TimerLayerChannelStore) GetFromMaster(id string) (*model.Channel, error) { start := timemodule.Now() resultVar0, resultVar1 := s.ChannelStore.GetFromMaster(id)