diff --git a/server/channels/jobs/base_workers.go b/server/channels/jobs/base_workers.go index dc4096df007..e16b0317016 100644 --- a/server/channels/jobs/base_workers.go +++ b/server/channels/jobs/base_workers.go @@ -82,13 +82,14 @@ func (worker *SimpleWorker) DoJob(job *model.Job) { c := request.EmptyContext(worker.logger) - var appErr *model.AppError // We get the job again because ClaimJob changes the job status. - job, appErr = worker.jobServer.GetJob(c, job.Id) + newJob, appErr := worker.jobServer.GetJob(c, job.Id) if appErr != nil { job.Logger.Error("SimpleWorker: job execution error", mlog.Err(appErr)) worker.setJobError(job, appErr) + return } + job = newJob err := worker.execute(job) if err != nil { diff --git a/server/channels/jobs/base_workers_test.go b/server/channels/jobs/base_workers_test.go new file mode 100644 index 00000000000..56fc4cef595 --- /dev/null +++ b/server/channels/jobs/base_workers_test.go @@ -0,0 +1,43 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package jobs + +import ( + "errors" + "testing" + + "github.com/mattermost/mattermost/server/public/model" + "github.com/mattermost/mattermost/server/public/shared/mlog" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +func TestSimpleWorkerPanic(t *testing.T) { + jobServer, mockStore, mockMetrics := makeJobServer(t) + + job := &model.Job{ + Id: "job_id", + Type: "job_type", + Logger: jobServer.logger.(*mlog.Logger), + } + + exec := func(_ *model.Job) error { + return nil + } + + isEnabled := func(_ *model.Config) bool { + return true + } + + mockStore.JobStore.On("UpdateStatusOptimistically", "job_id", model.JobStatusPending, model.JobStatusInProgress).Return(true, nil) + mockStore.JobStore.On("UpdateOptimistically", mock.AnythingOfType("*model.Job"), model.JobStatusInProgress).Return(true, nil) + mockStore.JobStore.On("Get", mock.AnythingOfType("*request.Context"), "job_id").Return(nil, errors.New("test")) + mockMetrics.On("IncrementJobActive", "job_type") + mockMetrics.On("DecrementJobActive", "job_type") + sWorker := NewSimpleWorker("test", jobServer, exec, isEnabled) + + require.NotPanics(t, func() { + sWorker.DoJob(job) + }) +} diff --git a/server/channels/jobs/jobs_test.go b/server/channels/jobs/jobs_test.go index 964c0a31ac5..8bc4621ddea 100644 --- a/server/channels/jobs/jobs_test.go +++ b/server/channels/jobs/jobs_test.go @@ -38,6 +38,7 @@ func makeJobServer(t *testing.T) (*JobServer, *storetest.Store, *mocks.MetricsIn ConfigService: configService, Store: mockStore, metrics: mockMetrics, + logger: mlog.CreateConsoleTestLogger(t), } return jobServer, mockStore, mockMetrics