MM-54580: Fix panic in jobs/base_workers.go (#24631)

If GetJob fails, we would return a nil job and then try to log
with that. To prevent this, we keep a reference to the old job
and log with that in the error case.

https://mattermost.atlassian.net/browse/MM-54580

```release-note
Fix a panic where a simple worker would crash if if failed to get a job.
```

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 {

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)
+	})
+}

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