jobstore (#15250)

* Migration completed

* Modify test case

* Update en.json

* Fix layers

* Lint: remove unnecessary use of sprint

* trigger CI

* fix error

* Fixes

* fix test

Co-authored-by: Rodrigo Villablanca <villa061004@gmail.com>
Co-authored-by: Mattermod <mattermod@users.noreply.github.com>

app/job.go

@@ -4,11 +4,26 @@
 package app
 
 import (
+	"errors"
+	"net/http"
+
 	"github.com/mattermost/mattermost-server/v5/model"
+	"github.com/mattermost/mattermost-server/v5/store"
 )
 
 func (a *App) GetJob(id string) (*model.Job, *model.AppError) {
-	return a.Srv().Store.Job().Get(id)
+	job, err := a.Srv().Store.Job().Get(id)
+	if err != nil {
+		var nfErr *store.ErrNotFound
+		switch {
+		case errors.As(err, &nfErr):
+			return nil, model.NewAppError("GetJob", "app.job.get.app_error", nil, nfErr.Error(), http.StatusNotFound)
+		default:
+			return nil, model.NewAppError("GetJob", "app.job.get.app_error", nil, err.Error(), http.StatusInternalServerError)
+		}
+	}
+
+	return job, nil
 }
 
 func (a *App) GetJobsPage(page int, perPage int) ([]*model.Job, *model.AppError) {
@@ -16,7 +31,12 @@ func (a *App) GetJobsPage(page int, perPage int) ([]*model.Job, *model.AppError)
 }
 
 func (a *App) GetJobs(offset int, limit int) ([]*model.Job, *model.AppError) {
-	return a.Srv().Store.Job().GetAllPage(offset, limit)
+	jobs, err := a.Srv().Store.Job().GetAllPage(offset, limit)
+	if err != nil {
+		return nil, model.NewAppError("GetJobs", "app.job.get_all.app_error", nil, err.Error(), http.StatusInternalServerError)
+	}
+
+	return jobs, nil
 }
 
 func (a *App) GetJobsByTypePage(jobType string, page int, perPage int) ([]*model.Job, *model.AppError) {
@@ -24,7 +44,12 @@ func (a *App) GetJobsByTypePage(jobType string, page int, perPage int) ([]*model
 }
 
 func (a *App) GetJobsByType(jobType string, offset int, limit int) ([]*model.Job, *model.AppError) {
-	return a.Srv().Store.Job().GetAllByTypePage(jobType, offset, limit)
+	jobs, err := a.Srv().Store.Job().GetAllByTypePage(jobType, offset, limit)
+	if err != nil {
+		return nil, model.NewAppError("GetJobsByType", "app.job.get_all.app_error", nil, err.Error(), http.StatusInternalServerError)
+	}
+
+	return jobs, nil
 }
 
 func (a *App) CreateJob(job *model.Job) (*model.Job, *model.AppError) {

i18n/en.json

@@ -4022,6 +4022,30 @@
     "id": "app.job.download_export_results_not_enabled",
     "translation": "DownloadExportResults in config.json is false. Please set this to true to download the results of this job."
   },
+  {
+    "id": "app.job.get.app_error",
+    "translation": "Unable to get the job."
+  },
+  {
+    "id": "app.job.get_all.app_error",
+    "translation": "Unable to get the jobs."
+  },
+  {
+    "id": "app.job.get_count_by_status_and_type.app_error",
+    "translation": "Unable to get the job count by status and type."
+  },
+  {
+    "id": "app.job.get_newest_job_by_status_and_type.app_error",
+    "translation": "Unable to get the newest job by status and type."
+  },
+  {
+    "id": "app.job.save.app_error",
+    "translation": "Unable to save the job."
+  },
+  {
+    "id": "app.job.update.app_error",
+    "translation": "Unable to update the job."
+  },
   {
     "id": "app.notification.body.intro.direct.full",
     "translation": "You have a new Direct Message."
@@ -7254,34 +7278,6 @@
     "id": "store.sql_group.uniqueness_error",
     "translation": "group member already exists"
   },
-  {
-    "id": "store.sql_job.delete.app_error",
-    "translation": "Unable to delete the job."
-  },
-  {
-    "id": "store.sql_job.get.app_error",
-    "translation": "Unable to get the job."
-  },
-  {
-    "id": "store.sql_job.get_all.app_error",
-    "translation": "Unable to get the jobs."
-  },
-  {
-    "id": "store.sql_job.get_count_by_status_and_type.app_error",
-    "translation": "Unable to get the job count by status and type."
-  },
-  {
-    "id": "store.sql_job.get_newest_job_by_status_and_type.app_error",
-    "translation": "Unable to get the newest job by status and type."
-  },
-  {
-    "id": "store.sql_job.save.app_error",
-    "translation": "Unable to save the job."
-  },
-  {
-    "id": "store.sql_job.update.app_error",
-    "translation": "Unable to update the job."
-  },
   {
     "id": "store.sql_post.analytics_posts_count.app_error",
     "translation": "Unable to get post counts."

jobs/jobs.go

@@ -5,12 +5,13 @@ package jobs
 
 import (
 	"context"
-	"time"
+	"errors"
-
 	"net/http"
+	"time"
 
 	"github.com/mattermost/mattermost-server/v5/mlog"
 	"github.com/mattermost/mattermost-server/v5/model"
+	"github.com/mattermost/mattermost-server/v5/store"
 )
 
 const (
@@ -31,18 +32,34 @@ func (srv *JobServer) CreateJob(jobType string, jobData map[string]string) (*mod
 	}
 
 	if _, err := srv.Store.Job().Save(&job); err != nil {
-		return nil, err
+		return nil, model.NewAppError("CreateJob", "app.job.save.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}
 
 	return &job, nil
 }
 
 func (srv *JobServer) GetJob(id string) (*model.Job, *model.AppError) {
-	return srv.Store.Job().Get(id)
+	job, err := srv.Store.Job().Get(id)
+	if err != nil {
+		var nfErr *store.ErrNotFound
+		switch {
+		case errors.As(err, &nfErr):
+			return nil, model.NewAppError("GetJob", "app.job.get.app_error", nil, nfErr.Error(), http.StatusNotFound)
+		default:
+			return nil, model.NewAppError("GetJob", "app.job.get.app_error", nil, err.Error(), http.StatusInternalServerError)
+		}
+	}
+
+	return job, nil
 }
 
 func (srv *JobServer) ClaimJob(job *model.Job) (bool, *model.AppError) {
-	return srv.Store.Job().UpdateStatusOptimistically(job.Id, model.JOB_STATUS_PENDING, model.JOB_STATUS_IN_PROGRESS)
+	updated, err := srv.Store.Job().UpdateStatusOptimistically(job.Id, model.JOB_STATUS_PENDING, model.JOB_STATUS_IN_PROGRESS)
+	if err != nil {
+		return false, model.NewAppError("ClaimJob", "app.job.update.app_error", nil, err.Error(), http.StatusInternalServerError)
+	}
+
+	return updated, nil
 }
 
 func (srv *JobServer) SetJobProgress(job *model.Job, progress int64) *model.AppError {
@@ -50,21 +67,21 @@ func (srv *JobServer) SetJobProgress(job *model.Job, progress int64) *model.AppE
 	job.Progress = progress
 
 	if _, err := srv.Store.Job().UpdateOptimistically(job, model.JOB_STATUS_IN_PROGRESS); err != nil {
-		return err
+		return model.NewAppError("SetJobProgress", "app.job.update.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}
 	return nil
 }
 
 func (srv *JobServer) SetJobWarning(job *model.Job) *model.AppError {
 	if _, err := srv.Store.Job().UpdateStatus(job.Id, model.JOB_STATUS_WARNING); err != nil {
-		return err
+		return model.NewAppError("SetJobWarning", "app.job.update.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}
 	return nil
 }
 
 func (srv *JobServer) SetJobSuccess(job *model.Job) *model.AppError {
 	if _, err := srv.Store.Job().UpdateStatus(job.Id, model.JOB_STATUS_SUCCESS); err != nil {
-		return err
+		return model.NewAppError("SetJobSuccess", "app.job.update.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}
 	return nil
 }
@@ -72,7 +89,11 @@ func (srv *JobServer) SetJobSuccess(job *model.Job) *model.AppError {
 func (srv *JobServer) SetJobError(job *model.Job, jobError *model.AppError) *model.AppError {
 	if jobError == nil {
 		_, err := srv.Store.Job().UpdateStatus(job.Id, model.JOB_STATUS_ERROR)
-		return err
+		if err != nil {
+			return model.NewAppError("SetJobError", "app.job.update.app_error", nil, err.Error(), http.StatusInternalServerError)
+		}
+
+		return nil
 	}
 
 	job.Status = model.JOB_STATUS_ERROR
@@ -84,16 +105,16 @@ func (srv *JobServer) SetJobError(job *model.Job, jobError *model.AppError) *mod
 
 	updated, err := srv.Store.Job().UpdateOptimistically(job, model.JOB_STATUS_IN_PROGRESS)
 	if err != nil {
-		return err
+		return model.NewAppError("SetJobError", "app.job.update.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}
 
 	if !updated {
 		updated, err = srv.Store.Job().UpdateOptimistically(job, model.JOB_STATUS_CANCEL_REQUESTED)
 		if err != nil {
-			return err
+			return model.NewAppError("SetJobError", "app.job.update.app_error", nil, err.Error(), http.StatusInternalServerError)
 		}
 		if !updated {
-			return model.NewAppError("Jobs.SetJobError", "jobs.set_job_error.update.error", nil, "id="+job.Id, http.StatusInternalServerError)
+			return model.NewAppError("SetJobError", "jobs.set_job_error.update.error", nil, "id="+job.Id, http.StatusInternalServerError)
 		}
 	}
 
@@ -102,7 +123,7 @@ func (srv *JobServer) SetJobError(job *model.Job, jobError *model.AppError) *mod
 
 func (srv *JobServer) SetJobCanceled(job *model.Job) *model.AppError {
 	if _, err := srv.Store.Job().UpdateStatus(job.Id, model.JOB_STATUS_CANCELED); err != nil {
-		return err
+		return model.NewAppError("SetJobCanceled", "app.job.update.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}
 	return nil
 }
@@ -111,7 +132,7 @@ func (srv *JobServer) UpdateInProgressJobData(job *model.Job) *model.AppError {
 	job.Status = model.JOB_STATUS_IN_PROGRESS
 	job.LastActivityAt = model.GetMillis()
 	if _, err := srv.Store.Job().UpdateOptimistically(job, model.JOB_STATUS_IN_PROGRESS); err != nil {
-		return err
+		return model.NewAppError("UpdateInProgressJobData", "app.job.update.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}
 	return nil
 }
@@ -119,7 +140,7 @@ func (srv *JobServer) UpdateInProgressJobData(job *model.Job) *model.AppError {
 func (srv *JobServer) RequestCancellation(jobId string) *model.AppError {
 	updated, err := srv.Store.Job().UpdateStatusOptimistically(jobId, model.JOB_STATUS_PENDING, model.JOB_STATUS_CANCELED)
 	if err != nil {
-		return err
+		return model.NewAppError("RequestCancellation", "app.job.update.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}
 	if updated {
 		return nil
@@ -127,14 +148,14 @@ func (srv *JobServer) RequestCancellation(jobId string) *model.AppError {
 
 	updated, err = srv.Store.Job().UpdateStatusOptimistically(jobId, model.JOB_STATUS_IN_PROGRESS, model.JOB_STATUS_CANCEL_REQUESTED)
 	if err != nil {
-		return err
+		return model.NewAppError("RequestCancellation", "app.job.update.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}
 
 	if updated {
 		return nil
 	}
 
-	return model.NewAppError("Jobs.RequestCancellation", "jobs.request_cancellation.status.error", nil, "id="+jobId, http.StatusInternalServerError)
+	return model.NewAppError("RequestCancellation", "jobs.request_cancellation.status.error", nil, "id="+jobId, http.StatusInternalServerError)
 }
 
 func (srv *JobServer) CancellationWatcher(ctx context.Context, jobId string, cancelChan chan interface{}) {
@@ -168,7 +189,7 @@ func GenerateNextStartDateTime(now time.Time, nextStartTime time.Time) *time.Tim
 func (srv *JobServer) CheckForPendingJobsByType(jobType string) (bool, *model.AppError) {
 	count, err := srv.Store.Job().GetCountByStatusAndType(model.JOB_STATUS_PENDING, jobType)
 	if err != nil {
-		return false, err
+		return false, model.NewAppError("CheckForPendingJobsByType", "app.job.get_count_by_status_and_type.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}
 	return count > 0, nil
 }
@@ -178,5 +199,16 @@ func (srv *JobServer) GetLastSuccessfulJobByType(jobType string) (*model.Job, *m
 	if jobType == model.JOB_TYPE_MESSAGE_EXPORT {
 		statuses = []string{model.JOB_STATUS_WARNING, model.JOB_STATUS_SUCCESS}
 	}
-	return srv.Store.Job().GetNewestJobByStatusesAndType(statuses, jobType)
+	job, err := srv.Store.Job().GetNewestJobByStatusesAndType(statuses, jobType)
+	if err != nil {
+		var nfErr *store.ErrNotFound
+		switch {
+		case errors.As(err, &nfErr):
+			return nil, model.NewAppError("GetLastSuccessfulJobByType", "app.job.get_newest_job_by_status_and_type.app_error", nil, nfErr.Error(), http.StatusNotFound)
+		default:
+			return nil, model.NewAppError("GetLastSuccessfulJobByType", "app.job.get_newest_job_by_status_and_type.app_error", nil, err.Error(), http.StatusInternalServerError)
+		}
+	}
+
+	return job, nil
 }

migrations/migrations.go

@@ -4,6 +4,8 @@
 package migrations
 
 import (
+	"net/http"
+
 	"github.com/mattermost/mattermost-server/v5/app"
 	tjobs "github.com/mattermost/mattermost-server/v5/jobs/interfaces"
 	"github.com/mattermost/mattermost-server/v5/model"
@@ -42,7 +44,7 @@ func GetMigrationState(migration string, store store.Store) (string, *model.Job,
 
 	jobs, err := store.Job().GetAllByType(model.JOB_TYPE_MIGRATIONS)
 	if err != nil {
-		return "", nil, err
+		return "", nil, model.NewAppError("GetMigrationState", "app.job.get_all.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}
 
 	for _, job := range jobs {

migrations/migrations_test.go

@@ -56,8 +56,8 @@ func TestGetMigrationState(t *testing.T) {
 		Type:   model.JOB_TYPE_MIGRATIONS,
 	}
 
-	j1, err = th.App.Srv().Store.Job().Save(j1)
+	j1, nErr = th.App.Srv().Store.Job().Save(j1)
-	require.Nil(t, err)
+	require.Nil(t, nErr)
 
 	state, job, err = GetMigrationState(migrationKey, th.App.Srv().Store)
 	assert.Nil(t, err)
@@ -75,8 +75,8 @@ func TestGetMigrationState(t *testing.T) {
 		Type:   model.JOB_TYPE_MIGRATIONS,
 	}
 
-	j2, err = th.App.Srv().Store.Job().Save(j2)
+	j2, nErr = th.App.Srv().Store.Job().Save(j2)
-	require.Nil(t, err)
+	require.Nil(t, nErr)
 
 	state, job, err = GetMigrationState(migrationKey, th.App.Srv().Store)
 	assert.Nil(t, err)
@@ -94,8 +94,8 @@ func TestGetMigrationState(t *testing.T) {
 		Type:   model.JOB_TYPE_MIGRATIONS,
 	}
 
-	j3, err = th.App.Srv().Store.Job().Save(j3)
+	j3, nErr = th.App.Srv().Store.Job().Save(j3)
-	require.Nil(t, err)
+	require.Nil(t, nErr)
 
 	state, job, err = GetMigrationState(migrationKey, th.App.Srv().Store)
 	assert.Nil(t, err)

store/opentracinglayer/opentracinglayer.go

@@ -3883,7 +3883,7 @@ func (s *OpenTracingLayerGroupStore) UpsertMember(groupID string, userID string)
 	return result, err
 }
 
-func (s *OpenTracingLayerJobStore) Delete(id string) (string, *model.AppError) {
+func (s *OpenTracingLayerJobStore) Delete(id string) (string, error) {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "JobStore.Delete")
 	s.Root.Store.SetContext(newCtx)
@@ -3901,7 +3901,7 @@ func (s *OpenTracingLayerJobStore) Delete(id string) (string, *model.AppError) {
 	return result, err
 }
 
-func (s *OpenTracingLayerJobStore) Get(id string) (*model.Job, *model.AppError) {
+func (s *OpenTracingLayerJobStore) Get(id string) (*model.Job, error) {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "JobStore.Get")
 	s.Root.Store.SetContext(newCtx)
@@ -3919,7 +3919,7 @@ func (s *OpenTracingLayerJobStore) Get(id string) (*model.Job, *model.AppError)
 	return result, err
 }
 
-func (s *OpenTracingLayerJobStore) GetAllByStatus(status string) ([]*model.Job, *model.AppError) {
+func (s *OpenTracingLayerJobStore) GetAllByStatus(status string) ([]*model.Job, error) {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "JobStore.GetAllByStatus")
 	s.Root.Store.SetContext(newCtx)
@@ -3937,7 +3937,7 @@ func (s *OpenTracingLayerJobStore) GetAllByStatus(status string) ([]*model.Job,
 	return result, err
 }
 
-func (s *OpenTracingLayerJobStore) GetAllByType(jobType string) ([]*model.Job, *model.AppError) {
+func (s *OpenTracingLayerJobStore) GetAllByType(jobType string) ([]*model.Job, error) {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "JobStore.GetAllByType")
 	s.Root.Store.SetContext(newCtx)
@@ -3955,7 +3955,7 @@ func (s *OpenTracingLayerJobStore) GetAllByType(jobType string) ([]*model.Job, *
 	return result, err
 }
 
-func (s *OpenTracingLayerJobStore) GetAllByTypePage(jobType string, offset int, limit int) ([]*model.Job, *model.AppError) {
+func (s *OpenTracingLayerJobStore) GetAllByTypePage(jobType string, offset int, limit int) ([]*model.Job, error) {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "JobStore.GetAllByTypePage")
 	s.Root.Store.SetContext(newCtx)
@@ -3973,7 +3973,7 @@ func (s *OpenTracingLayerJobStore) GetAllByTypePage(jobType string, offset int,
 	return result, err
 }
 
-func (s *OpenTracingLayerJobStore) GetAllPage(offset int, limit int) ([]*model.Job, *model.AppError) {
+func (s *OpenTracingLayerJobStore) GetAllPage(offset int, limit int) ([]*model.Job, error) {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "JobStore.GetAllPage")
 	s.Root.Store.SetContext(newCtx)
@@ -3991,7 +3991,7 @@ func (s *OpenTracingLayerJobStore) GetAllPage(offset int, limit int) ([]*model.J
 	return result, err
 }
 
-func (s *OpenTracingLayerJobStore) GetCountByStatusAndType(status string, jobType string) (int64, *model.AppError) {
+func (s *OpenTracingLayerJobStore) GetCountByStatusAndType(status string, jobType string) (int64, error) {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "JobStore.GetCountByStatusAndType")
 	s.Root.Store.SetContext(newCtx)
@@ -4009,7 +4009,7 @@ func (s *OpenTracingLayerJobStore) GetCountByStatusAndType(status string, jobTyp
 	return result, err
 }
 
-func (s *OpenTracingLayerJobStore) GetNewestJobByStatusAndType(status string, jobType string) (*model.Job, *model.AppError) {
+func (s *OpenTracingLayerJobStore) GetNewestJobByStatusAndType(status string, jobType string) (*model.Job, error) {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "JobStore.GetNewestJobByStatusAndType")
 	s.Root.Store.SetContext(newCtx)
@@ -4027,7 +4027,7 @@ func (s *OpenTracingLayerJobStore) GetNewestJobByStatusAndType(status string, jo
 	return result, err
 }
 
-func (s *OpenTracingLayerJobStore) GetNewestJobByStatusesAndType(statuses []string, jobType string) (*model.Job, *model.AppError) {
+func (s *OpenTracingLayerJobStore) GetNewestJobByStatusesAndType(statuses []string, jobType string) (*model.Job, error) {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "JobStore.GetNewestJobByStatusesAndType")
 	s.Root.Store.SetContext(newCtx)
@@ -4045,7 +4045,7 @@ func (s *OpenTracingLayerJobStore) GetNewestJobByStatusesAndType(statuses []stri
 	return result, err
 }
 
-func (s *OpenTracingLayerJobStore) Save(job *model.Job) (*model.Job, *model.AppError) {
+func (s *OpenTracingLayerJobStore) Save(job *model.Job) (*model.Job, error) {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "JobStore.Save")
 	s.Root.Store.SetContext(newCtx)
@@ -4063,7 +4063,7 @@ func (s *OpenTracingLayerJobStore) Save(job *model.Job) (*model.Job, *model.AppE
 	return result, err
 }
 
-func (s *OpenTracingLayerJobStore) UpdateOptimistically(job *model.Job, currentStatus string) (bool, *model.AppError) {
+func (s *OpenTracingLayerJobStore) UpdateOptimistically(job *model.Job, currentStatus string) (bool, error) {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "JobStore.UpdateOptimistically")
 	s.Root.Store.SetContext(newCtx)
@@ -4081,7 +4081,7 @@ func (s *OpenTracingLayerJobStore) UpdateOptimistically(job *model.Job, currentS
 	return result, err
 }
 
-func (s *OpenTracingLayerJobStore) UpdateStatus(id string, status string) (*model.Job, *model.AppError) {
+func (s *OpenTracingLayerJobStore) UpdateStatus(id string, status string) (*model.Job, error) {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "JobStore.UpdateStatus")
 	s.Root.Store.SetContext(newCtx)
@@ -4099,7 +4099,7 @@ func (s *OpenTracingLayerJobStore) UpdateStatus(id string, status string) (*mode
 	return result, err
 }
 
-func (s *OpenTracingLayerJobStore) UpdateStatusOptimistically(id string, currentStatus string, newStatus string) (bool, *model.AppError) {
+func (s *OpenTracingLayerJobStore) UpdateStatusOptimistically(id string, currentStatus string, newStatus string) (bool, error) {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "JobStore.UpdateStatusOptimistically")
 	s.Root.Store.SetContext(newCtx)

store/retrylayer/retrylayer.go

@@ -2646,81 +2646,263 @@ func (s *RetryLayerGroupStore) UpsertMember(groupID string, userID string) (*mod
 
 }
 
-func (s *RetryLayerJobStore) Delete(id string) (string, *model.AppError) {
+func (s *RetryLayerJobStore) Delete(id string) (string, error) {
 
-	return s.JobStore.Delete(id)
+	tries := 0
+	for {
+		result, err := s.JobStore.Delete(id)
+		if err == nil {
+			return result, err
+		}
+		if !isRepeatableError(err) {
+			return result, err
+		}
+		tries++
+		if tries >= 3 {
+			err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures")
+			return result, err
+		}
+	}
 
 }
 
-func (s *RetryLayerJobStore) Get(id string) (*model.Job, *model.AppError) {
+func (s *RetryLayerJobStore) Get(id string) (*model.Job, error) {
 
-	return s.JobStore.Get(id)
+	tries := 0
+	for {
+		result, err := s.JobStore.Get(id)
+		if err == nil {
+			return result, err
+		}
+		if !isRepeatableError(err) {
+			return result, err
+		}
+		tries++
+		if tries >= 3 {
+			err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures")
+			return result, err
+		}
+	}
 
 }
 
-func (s *RetryLayerJobStore) GetAllByStatus(status string) ([]*model.Job, *model.AppError) {
+func (s *RetryLayerJobStore) GetAllByStatus(status string) ([]*model.Job, error) {
 
-	return s.JobStore.GetAllByStatus(status)
+	tries := 0
+	for {
+		result, err := s.JobStore.GetAllByStatus(status)
+		if err == nil {
+			return result, err
+		}
+		if !isRepeatableError(err) {
+			return result, err
+		}
+		tries++
+		if tries >= 3 {
+			err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures")
+			return result, err
+		}
+	}
 
 }
 
-func (s *RetryLayerJobStore) GetAllByType(jobType string) ([]*model.Job, *model.AppError) {
+func (s *RetryLayerJobStore) GetAllByType(jobType string) ([]*model.Job, error) {
 
-	return s.JobStore.GetAllByType(jobType)
+	tries := 0
+	for {
+		result, err := s.JobStore.GetAllByType(jobType)
+		if err == nil {
+			return result, err
+		}
+		if !isRepeatableError(err) {
+			return result, err
+		}
+		tries++
+		if tries >= 3 {
+			err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures")
+			return result, err
+		}
+	}
 
 }
 
-func (s *RetryLayerJobStore) GetAllByTypePage(jobType string, offset int, limit int) ([]*model.Job, *model.AppError) {
+func (s *RetryLayerJobStore) GetAllByTypePage(jobType string, offset int, limit int) ([]*model.Job, error) {
 
-	return s.JobStore.GetAllByTypePage(jobType, offset, limit)
+	tries := 0
+	for {
+		result, err := s.JobStore.GetAllByTypePage(jobType, offset, limit)
+		if err == nil {
+			return result, err
+		}
+		if !isRepeatableError(err) {
+			return result, err
+		}
+		tries++
+		if tries >= 3 {
+			err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures")
+			return result, err
+		}
+	}
 
 }
 
-func (s *RetryLayerJobStore) GetAllPage(offset int, limit int) ([]*model.Job, *model.AppError) {
+func (s *RetryLayerJobStore) GetAllPage(offset int, limit int) ([]*model.Job, error) {
 
-	return s.JobStore.GetAllPage(offset, limit)
+	tries := 0
+	for {
+		result, err := s.JobStore.GetAllPage(offset, limit)
+		if err == nil {
+			return result, err
+		}
+		if !isRepeatableError(err) {
+			return result, err
+		}
+		tries++
+		if tries >= 3 {
+			err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures")
+			return result, err
+		}
+	}
 
 }
 
-func (s *RetryLayerJobStore) GetCountByStatusAndType(status string, jobType string) (int64, *model.AppError) {
+func (s *RetryLayerJobStore) GetCountByStatusAndType(status string, jobType string) (int64, error) {
 
-	return s.JobStore.GetCountByStatusAndType(status, jobType)
+	tries := 0
+	for {
+		result, err := s.JobStore.GetCountByStatusAndType(status, jobType)
+		if err == nil {
+			return result, err
+		}
+		if !isRepeatableError(err) {
+			return result, err
+		}
+		tries++
+		if tries >= 3 {
+			err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures")
+			return result, err
+		}
+	}
 
 }
 
-func (s *RetryLayerJobStore) GetNewestJobByStatusAndType(status string, jobType string) (*model.Job, *model.AppError) {
+func (s *RetryLayerJobStore) GetNewestJobByStatusAndType(status string, jobType string) (*model.Job, error) {
 
-	return s.JobStore.GetNewestJobByStatusAndType(status, jobType)
+	tries := 0
+	for {
+		result, err := s.JobStore.GetNewestJobByStatusAndType(status, jobType)
+		if err == nil {
+			return result, err
+		}
+		if !isRepeatableError(err) {
+			return result, err
+		}
+		tries++
+		if tries >= 3 {
+			err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures")
+			return result, err
+		}
+	}
 
 }
 
-func (s *RetryLayerJobStore) GetNewestJobByStatusesAndType(statuses []string, jobType string) (*model.Job, *model.AppError) {
+func (s *RetryLayerJobStore) GetNewestJobByStatusesAndType(statuses []string, jobType string) (*model.Job, error) {
 
-	return s.JobStore.GetNewestJobByStatusesAndType(statuses, jobType)
+	tries := 0
+	for {
+		result, err := s.JobStore.GetNewestJobByStatusesAndType(statuses, jobType)
+		if err == nil {
+			return result, err
+		}
+		if !isRepeatableError(err) {
+			return result, err
+		}
+		tries++
+		if tries >= 3 {
+			err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures")
+			return result, err
+		}
+	}
 
 }
 
-func (s *RetryLayerJobStore) Save(job *model.Job) (*model.Job, *model.AppError) {
+func (s *RetryLayerJobStore) Save(job *model.Job) (*model.Job, error) {
 
-	return s.JobStore.Save(job)
+	tries := 0
+	for {
+		result, err := s.JobStore.Save(job)
+		if err == nil {
+			return result, err
+		}
+		if !isRepeatableError(err) {
+			return result, err
+		}
+		tries++
+		if tries >= 3 {
+			err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures")
+			return result, err
+		}
+	}
 
 }
 
-func (s *RetryLayerJobStore) UpdateOptimistically(job *model.Job, currentStatus string) (bool, *model.AppError) {
+func (s *RetryLayerJobStore) UpdateOptimistically(job *model.Job, currentStatus string) (bool, error) {
 
-	return s.JobStore.UpdateOptimistically(job, currentStatus)
+	tries := 0
+	for {
+		result, err := s.JobStore.UpdateOptimistically(job, currentStatus)
+		if err == nil {
+			return result, err
+		}
+		if !isRepeatableError(err) {
+			return result, err
+		}
+		tries++
+		if tries >= 3 {
+			err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures")
+			return result, err
+		}
+	}
 
 }
 
-func (s *RetryLayerJobStore) UpdateStatus(id string, status string) (*model.Job, *model.AppError) {
+func (s *RetryLayerJobStore) UpdateStatus(id string, status string) (*model.Job, error) {
 
-	return s.JobStore.UpdateStatus(id, status)
+	tries := 0
+	for {
+		result, err := s.JobStore.UpdateStatus(id, status)
+		if err == nil {
+			return result, err
+		}
+		if !isRepeatableError(err) {
+			return result, err
+		}
+		tries++
+		if tries >= 3 {
+			err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures")
+			return result, err
+		}
+	}
 
 }
 
-func (s *RetryLayerJobStore) UpdateStatusOptimistically(id string, currentStatus string, newStatus string) (bool, *model.AppError) {
+func (s *RetryLayerJobStore) UpdateStatusOptimistically(id string, currentStatus string, newStatus string) (bool, error) {
 
-	return s.JobStore.UpdateStatusOptimistically(id, currentStatus, newStatus)
+	tries := 0
+	for {
+		result, err := s.JobStore.UpdateStatusOptimistically(id, currentStatus, newStatus)
+		if err == nil {
+			return result, err
+		}
+		if !isRepeatableError(err) {
+			return result, err
+		}
+		tries++
+		if tries >= 3 {
+			err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures")
+			return result, err
+		}
+	}
 
 }
 

store/sqlstore/job_store.go

@@ -5,10 +5,11 @@ package sqlstore
 
 import (
 	"database/sql"
-	"net/http"
+	"fmt"
 	"strings"
 
 	sq "github.com/Masterminds/squirrel"
+	"github.com/pkg/errors"
 
 	"github.com/mattermost/gorp"
 	"github.com/mattermost/mattermost-server/v5/model"
@@ -37,15 +38,15 @@ func (jss SqlJobStore) createIndexesIfNotExists() {
 	jss.CreateIndexIfNotExists("idx_jobs_type", "Jobs", "Type")
 }
 
-func (jss SqlJobStore) Save(job *model.Job) (*model.Job, *model.AppError) {
+func (jss SqlJobStore) Save(job *model.Job) (*model.Job, error) {
 	if err := jss.GetMaster().Insert(job); err != nil {
-		return nil, model.NewAppError("SqlJobStore.Save", "store.sql_job.save.app_error", nil, "id="+job.Id+", "+err.Error(), http.StatusInternalServerError)
+		return nil, errors.Wrap(err, "failed to save Job")
 	}
 	return job, nil
 }
 
-func (jss SqlJobStore) UpdateOptimistically(job *model.Job, currentStatus string) (bool, *model.AppError) {
+func (jss SqlJobStore) UpdateOptimistically(job *model.Job, currentStatus string) (bool, error) {
-	sql, args, err := jss.getQueryBuilder().
+	query, args, err := jss.getQueryBuilder().
 		Update("Jobs").
 		Set("LastActivityAt", model.GetMillis()).
 		Set("Status", job.Status).
@@ -53,17 +54,17 @@ func (jss SqlJobStore) UpdateOptimistically(job *model.Job, currentStatus string
 		Set("Progress", job.Progress).
 		Where(sq.Eq{"Id": job.Id, "Status": currentStatus}).ToSql()
 	if err != nil {
-		return false, model.NewAppError("SqlJobStore.UpdateOptimistically", "store.sql.build_query.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return false, errors.Wrap(err, "job_tosql")
 	}
-	sqlResult, err := jss.GetMaster().Exec(sql, args...)
+	sqlResult, err := jss.GetMaster().Exec(query, args...)
 	if err != nil {
-		return false, model.NewAppError("SqlJobStore.UpdateOptimistically", "store.sql_job.update.app_error", nil, "id="+job.Id+", "+err.Error(), http.StatusInternalServerError)
+		return false, errors.Wrap(err, "failed to update Job")
 	}
 
 	rows, err := sqlResult.RowsAffected()
 
 	if err != nil {
-		return false, model.NewAppError("SqlJobStore.UpdateStatus", "store.sql_job.update.app_error", nil, "id="+job.Id+", "+err.Error(), http.StatusInternalServerError)
+		return false, errors.Wrap(err, "unable to get rows affected")
 	}
 
 	if rows != 1 {
@@ -73,7 +74,7 @@ func (jss SqlJobStore) UpdateOptimistically(job *model.Job, currentStatus string
 	return true, nil
 }
 
-func (jss SqlJobStore) UpdateStatus(id string, status string) (*model.Job, *model.AppError) {
+func (jss SqlJobStore) UpdateStatus(id string, status string) (*model.Job, error) {
 	job := &model.Job{
 		Id:             id,
 		Status:         status,
@@ -83,34 +84,34 @@ func (jss SqlJobStore) UpdateStatus(id string, status string) (*model.Job, *mode
 	if _, err := jss.GetMaster().UpdateColumns(func(col *gorp.ColumnMap) bool {
 		return col.ColumnName == "Status" || col.ColumnName == "LastActivityAt"
 	}, job); err != nil {
-		return nil, model.NewAppError("SqlJobStore.UpdateStatus", "store.sql_job.update.app_error", nil, "id="+id+", "+err.Error(), http.StatusInternalServerError)
+		return nil, errors.Wrapf(err, "failed to update Job with id=%s", id)
 	}
 
 	return job, nil
 }
 
-func (jss SqlJobStore) UpdateStatusOptimistically(id string, currentStatus string, newStatus string) (bool, *model.AppError) {
+func (jss SqlJobStore) UpdateStatusOptimistically(id string, currentStatus string, newStatus string) (bool, error) {
-	sql := jss.getQueryBuilder().
+	builder := jss.getQueryBuilder().
 		Update("Jobs").
 		Set("LastActivityAt", model.GetMillis()).
 		Set("Status", newStatus).
 		Where(sq.Eq{"Id": id, "Status": currentStatus})
 
 	if newStatus == model.JOB_STATUS_IN_PROGRESS {
-		sql = sql.Set("StartAt", model.GetMillis())
+		builder = builder.Set("StartAt", model.GetMillis())
 	}
-	query, args, err := sql.ToSql()
+	query, args, err := builder.ToSql()
 	if err != nil {
-		return false, model.NewAppError("SqlJobStore.UpdateStatusOptimistically", "store.sql.build_query.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return false, errors.Wrap(err, "job_tosql")
 	}
 
 	sqlResult, err := jss.GetMaster().Exec(query, args...)
 	if err != nil {
-		return false, model.NewAppError("SqlJobStore.UpdateStatusOptimistically", "store.sql_job.update.app_error", nil, "id="+id+", "+err.Error(), http.StatusInternalServerError)
+		return false, errors.Wrapf(err, "failed to update Job with id=%s", id)
 	}
 	rows, err := sqlResult.RowsAffected()
 	if err != nil {
-		return false, model.NewAppError("SqlJobStore.UpdateStatus", "store.sql_job.update.app_error", nil, "id="+id+", "+err.Error(), http.StatusInternalServerError)
+		return false, errors.Wrap(err, "unable to get rows affected")
 	}
 	if rows != 1 {
 		return false, nil
@@ -119,25 +120,25 @@ func (jss SqlJobStore) UpdateStatusOptimistically(id string, currentStatus strin
 	return true, nil
 }
 
-func (jss SqlJobStore) Get(id string) (*model.Job, *model.AppError) {
+func (jss SqlJobStore) Get(id string) (*model.Job, error) {
 	query, args, err := jss.getQueryBuilder().
 		Select("*").
 		From("Jobs").
 		Where(sq.Eq{"Id": id}).ToSql()
 	if err != nil {
-		return nil, model.NewAppError("SqlJobStore.Get", "store.sql.build_query.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return nil, errors.Wrap(err, "job_tosql")
 	}
 	var status *model.Job
 	if err = jss.GetReplica().SelectOne(&status, query, args...); err != nil {
 		if err == sql.ErrNoRows {
-			return nil, model.NewAppError("SqlJobStore.Get", "store.sql_job.get.app_error", nil, "Id="+id+", "+err.Error(), http.StatusNotFound)
+			return nil, store.NewErrNotFound("Job", id)
 		}
-		return nil, model.NewAppError("SqlJobStore.Get", "store.sql_job.get.app_error", nil, "Id="+id+", "+err.Error(), http.StatusInternalServerError)
+		return nil, errors.Wrapf(err, "failed to get Job with id=%s", id)
 	}
 	return status, nil
 }
 
-func (jss SqlJobStore) GetAllPage(offset int, limit int) ([]*model.Job, *model.AppError) {
+func (jss SqlJobStore) GetAllPage(offset int, limit int) ([]*model.Job, error) {
 	query, args, err := jss.getQueryBuilder().
 		Select("*").
 		From("Jobs").
@@ -145,33 +146,33 @@ func (jss SqlJobStore) GetAllPage(offset int, limit int) ([]*model.Job, *model.A
 		Limit(uint64(limit)).
 		Offset(uint64(offset)).ToSql()
 	if err != nil {
-		return nil, model.NewAppError("SqlJobStore.GetAllPage", "store.sql.build_query.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return nil, errors.Wrap(err, "job_tosql")
 	}
 
 	var statuses []*model.Job
 	if _, err = jss.GetReplica().Select(&statuses, query, args...); err != nil {
-		return nil, model.NewAppError("SqlJobStore.GetAllPage", "store.sql_job.get_all.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return nil, errors.Wrap(err, "failed to find Jobs")
 	}
 	return statuses, nil
 }
 
-func (jss SqlJobStore) GetAllByType(jobType string) ([]*model.Job, *model.AppError) {
+func (jss SqlJobStore) GetAllByType(jobType string) ([]*model.Job, error) {
 	query, args, err := jss.getQueryBuilder().
 		Select("*").
 		From("Jobs").
 		Where(sq.Eq{"Type": jobType}).
 		OrderBy("CreateAt DESC").ToSql()
 	if err != nil {
-		return nil, model.NewAppError("SqlJobStore.GetAllByType", "store.sql.build_query.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return nil, errors.Wrap(err, "job_tosql")
 	}
 	var statuses []*model.Job
 	if _, err = jss.GetReplica().Select(&statuses, query, args...); err != nil {
-		return nil, model.NewAppError("SqlJobStore.GetAllByType", "store.sql_job.get_all.app_error", nil, "Type="+jobType+", "+err.Error(), http.StatusInternalServerError)
+		return nil, errors.Wrapf(err, "failed to find Jobs with type=%s", jobType)
 	}
 	return statuses, nil
 }
 
-func (jss SqlJobStore) GetAllByTypePage(jobType string, offset int, limit int) ([]*model.Job, *model.AppError) {
+func (jss SqlJobStore) GetAllByTypePage(jobType string, offset int, limit int) ([]*model.Job, error) {
 	query, args, err := jss.getQueryBuilder().
 		Select("*").
 		From("Jobs").
@@ -180,17 +181,17 @@ func (jss SqlJobStore) GetAllByTypePage(jobType string, offset int, limit int) (
 		Limit(uint64(limit)).
 		Offset(uint64(offset)).ToSql()
 	if err != nil {
-		return nil, model.NewAppError("SqlJobStore.GetAllByTypePage", "store.sql.build_query.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return nil, errors.Wrap(err, "job_tosql")
 	}
 
 	var statuses []*model.Job
 	if _, err = jss.GetReplica().Select(&statuses, query, args...); err != nil {
-		return nil, model.NewAppError("SqlJobStore.GetAllByTypePage", "store.sql_job.get_all.app_error", nil, "Type="+jobType+", "+err.Error(), http.StatusInternalServerError)
+		return nil, errors.Wrapf(err, "failed to find Jobs with type=%s", jobType)
 	}
 	return statuses, nil
 }
 
-func (jss SqlJobStore) GetAllByStatus(status string) ([]*model.Job, *model.AppError) {
+func (jss SqlJobStore) GetAllByStatus(status string) ([]*model.Job, error) {
 	var statuses []*model.Job
 	query, args, err := jss.getQueryBuilder().
 		Select("*").
@@ -198,19 +199,20 @@ func (jss SqlJobStore) GetAllByStatus(status string) ([]*model.Job, *model.AppEr
 		Where(sq.Eq{"Status": status}).
 		OrderBy("CreateAt ASC").ToSql()
 	if err != nil {
-		return nil, model.NewAppError("SqlJobStore.GetAllByStatus", "store.sql.build_query.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return nil, errors.Wrap(err, "job_tosql")
 	}
 
 	if _, err = jss.GetReplica().Select(&statuses, query, args...); err != nil {
-		return nil, model.NewAppError("SqlJobStore.GetAllByStatus", "store.sql_job.get_all.app_error", nil, "Status="+status+", "+err.Error(), http.StatusInternalServerError)
+		return nil, errors.Wrapf(err, "failed to find Jobs with status=%s", status)
 	}
 	return statuses, nil
 }
-func (jss SqlJobStore) GetNewestJobByStatusAndType(status string, jobType string) (*model.Job, *model.AppError) {
+
+func (jss SqlJobStore) GetNewestJobByStatusAndType(status string, jobType string) (*model.Job, error) {
 	return jss.GetNewestJobByStatusesAndType([]string{status}, jobType)
 }
 
-func (jss SqlJobStore) GetNewestJobByStatusesAndType(status []string, jobType string) (*model.Job, *model.AppError) {
+func (jss SqlJobStore) GetNewestJobByStatusesAndType(status []string, jobType string) (*model.Job, error) {
 	query, args, err := jss.getQueryBuilder().
 		Select("*").
 		From("Jobs").
@@ -218,41 +220,44 @@ func (jss SqlJobStore) GetNewestJobByStatusesAndType(status []string, jobType st
 		OrderBy("CreateAt DESC").
 		Limit(1).ToSql()
 	if err != nil {
-		return nil, model.NewAppError("SqlJobStore.GetNewestJobByStatusAndType", "store.sql.build_query.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return nil, errors.Wrap(err, "job_tosql")
 	}
 
 	var job *model.Job
-	if err = jss.GetReplica().SelectOne(&job, query, args...); err != nil && err != sql.ErrNoRows {
+	if err = jss.GetReplica().SelectOne(&job, query, args...); err != nil {
-		return nil, model.NewAppError("SqlJobStore.GetNewestJobByStatusAndType", "store.sql_job.get_newest_job_by_status_and_type.app_error", nil, "Status="+strings.Join(status, ",")+", "+err.Error(), http.StatusInternalServerError)
+		if err == sql.ErrNoRows {
+			return nil, store.NewErrNotFound("Job", fmt.Sprintf("<status, type>=<%s, %s>", strings.Join(status, ","), jobType))
+		}
+		return nil, errors.Wrapf(err, "failed to find Job with statuses=%s and type=%s", strings.Join(status, ","), jobType)
 	}
 	return job, nil
 }
 
-func (jss SqlJobStore) GetCountByStatusAndType(status string, jobType string) (int64, *model.AppError) {
+func (jss SqlJobStore) GetCountByStatusAndType(status string, jobType string) (int64, error) {
 	query, args, err := jss.getQueryBuilder().
 		Select("COUNT(*)").
 		From("Jobs").
 		Where(sq.Eq{"Status": status, "Type": jobType}).ToSql()
 	if err != nil {
-		return 0, model.NewAppError("SqlJobStore.GetCountByStatusAndType", "store.sql.build_query.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return 0, errors.Wrap(err, "job_tosql")
 	}
 	count, err := jss.GetReplica().SelectInt(query, args...)
 	if err != nil {
-		return int64(0), model.NewAppError("SqlJobStore.GetCountByStatusAndType", "store.sql_job.get_count_by_status_and_type.app_error", nil, "Status="+status+", "+err.Error(), http.StatusInternalServerError)
+		return int64(0), errors.Wrapf(err, "failed to count Jobs with status=%s and type=%s", status, jobType)
 	}
 	return count, nil
 }
 
-func (jss SqlJobStore) Delete(id string) (string, *model.AppError) {
+func (jss SqlJobStore) Delete(id string) (string, error) {
-	sql, args, err := jss.getQueryBuilder().
+	query, args, err := jss.getQueryBuilder().
 		Delete("Jobs").
 		Where(sq.Eq{"Id": id}).ToSql()
 	if err != nil {
-		return "", model.NewAppError("SqlJobStore.DeleteByType", "store.sql.build_query.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return "", errors.Wrap(err, "job_tosql")
 	}
 
-	if _, err = jss.GetMaster().Exec(sql, args...); err != nil {
+	if _, err = jss.GetMaster().Exec(query, args...); err != nil {
-		return "", model.NewAppError("SqlJobStore.DeleteByType", "store.sql_job.delete.app_error", nil, "id="+id+", "+err.Error(), http.StatusInternalServerError)
+		return "", errors.Wrapf(err, "failed to delete Job with id=%s", id)
 	}
 	return id, nil
 }

store/store.go

@@ -559,19 +559,19 @@ type ReactionStore interface {
 }
 
 type JobStore interface {
-	Save(job *model.Job) (*model.Job, *model.AppError)
+	Save(job *model.Job) (*model.Job, error)
-	UpdateOptimistically(job *model.Job, currentStatus string) (bool, *model.AppError)
+	UpdateOptimistically(job *model.Job, currentStatus string) (bool, error)
-	UpdateStatus(id string, status string) (*model.Job, *model.AppError)
+	UpdateStatus(id string, status string) (*model.Job, error)
-	UpdateStatusOptimistically(id string, currentStatus string, newStatus string) (bool, *model.AppError)
+	UpdateStatusOptimistically(id string, currentStatus string, newStatus string) (bool, error)
-	Get(id string) (*model.Job, *model.AppError)
+	Get(id string) (*model.Job, error)
-	GetAllPage(offset int, limit int) ([]*model.Job, *model.AppError)
+	GetAllPage(offset int, limit int) ([]*model.Job, error)
-	GetAllByType(jobType string) ([]*model.Job, *model.AppError)
+	GetAllByType(jobType string) ([]*model.Job, error)
-	GetAllByTypePage(jobType string, offset int, limit int) ([]*model.Job, *model.AppError)
+	GetAllByTypePage(jobType string, offset int, limit int) ([]*model.Job, error)
-	GetAllByStatus(status string) ([]*model.Job, *model.AppError)
+	GetAllByStatus(status string) ([]*model.Job, error)
-	GetNewestJobByStatusAndType(status string, jobType string) (*model.Job, *model.AppError)
+	GetNewestJobByStatusAndType(status string, jobType string) (*model.Job, error)
-	GetNewestJobByStatusesAndType(statuses []string, jobType string) (*model.Job, *model.AppError)
+	GetNewestJobByStatusesAndType(statuses []string, jobType string) (*model.Job, error)
-	GetCountByStatusAndType(status string, jobType string) (int64, *model.AppError)
+	GetCountByStatusAndType(status string, jobType string) (int64, error)
-	Delete(id string) (string, *model.AppError)
+	Delete(id string) (string, error)
 }
 
 type UserAccessTokenStore interface {

store/storetest/job_store.go

@@ -4,6 +4,7 @@
 package storetest
 
 import (
+	"errors"
 	"testing"
 
 	"time"
@@ -258,7 +259,9 @@ func testJobStoreGetNewestJobByStatusAndType(t *testing.T, ss store.Store) {
 	assert.EqualValues(t, jobs[0].Id, received.Id)
 
 	received, err = ss.Job().GetNewestJobByStatusAndType(model.NewId(), model.NewId())
-	assert.Nil(t, err)
+	assert.NotNil(t, err)
+	var nfErr *store.ErrNotFound
+	assert.True(t, errors.As(err, &nfErr))
 	assert.Nil(t, received)
 }
 
@@ -306,11 +309,14 @@ func testJobStoreGetNewestJobByStatusesAndType(t *testing.T, ss store.Store) {
 	assert.EqualValues(t, jobs[3].Id, received.Id)
 
 	received, err = ss.Job().GetNewestJobByStatusesAndType([]string{model.NewId(), model.NewId()}, model.NewId())
-	assert.Nil(t, err)
+	assert.NotNil(t, err)
+	var nfErr *store.ErrNotFound
+	assert.True(t, errors.As(err, &nfErr))
 	assert.Nil(t, received)
 
 	received, err = ss.Job().GetNewestJobByStatusesAndType([]string{status2}, jobType2)
-	assert.Nil(t, err)
+	assert.NotNil(t, err)
+	assert.True(t, errors.As(err, &nfErr))
 	assert.Nil(t, received)
 
 	received, err = ss.Job().GetNewestJobByStatusesAndType([]string{status1}, jobType2)
@@ -318,7 +324,8 @@ func testJobStoreGetNewestJobByStatusesAndType(t *testing.T, ss store.Store) {
 	assert.EqualValues(t, jobs[2].Id, received.Id)
 
 	received, err = ss.Job().GetNewestJobByStatusesAndType([]string{}, jobType1)
-	assert.Nil(t, err)
+	assert.NotNil(t, err)
+	assert.True(t, errors.As(err, &nfErr))
 	assert.Nil(t, received)
 }
 

store/storetest/mocks/JobStore.go

@@ -15,7 +15,7 @@ type JobStore struct {
 }
 
 // Delete provides a mock function with given fields: id
-func (_m *JobStore) Delete(id string) (string, *model.AppError) {
+func (_m *JobStore) Delete(id string) (string, error) {
 	ret := _m.Called(id)
 
 	var r0 string
@@ -25,20 +25,18 @@ func (_m *JobStore) Delete(id string) (string, *model.AppError) {
 		r0 = ret.Get(0).(string)
 	}
 
-	var r1 *model.AppError
+	var r1 error
-	if rf, ok := ret.Get(1).(func(string) *model.AppError); ok {
+	if rf, ok := ret.Get(1).(func(string) error); ok {
 		r1 = rf(id)
 	} else {
-		if ret.Get(1) != nil {
+		r1 = ret.Error(1)
-			r1 = ret.Get(1).(*model.AppError)
-		}
 	}
 
 	return r0, r1
 }
 
 // Get provides a mock function with given fields: id
-func (_m *JobStore) Get(id string) (*model.Job, *model.AppError) {
+func (_m *JobStore) Get(id string) (*model.Job, error) {
 	ret := _m.Called(id)
 
 	var r0 *model.Job
@@ -50,20 +48,18 @@ func (_m *JobStore) Get(id string) (*model.Job, *model.AppError) {
 		}
 	}
 
-	var r1 *model.AppError
+	var r1 error
-	if rf, ok := ret.Get(1).(func(string) *model.AppError); ok {
+	if rf, ok := ret.Get(1).(func(string) error); ok {
 		r1 = rf(id)
 	} else {
-		if ret.Get(1) != nil {
+		r1 = ret.Error(1)
-			r1 = ret.Get(1).(*model.AppError)
-		}
 	}
 
 	return r0, r1
 }
 
 // GetAllByStatus provides a mock function with given fields: status
-func (_m *JobStore) GetAllByStatus(status string) ([]*model.Job, *model.AppError) {
+func (_m *JobStore) GetAllByStatus(status string) ([]*model.Job, error) {
 	ret := _m.Called(status)
 
 	var r0 []*model.Job
@@ -75,20 +71,18 @@ func (_m *JobStore) GetAllByStatus(status string) ([]*model.Job, *model.AppError
 		}
 	}
 
-	var r1 *model.AppError
+	var r1 error
-	if rf, ok := ret.Get(1).(func(string) *model.AppError); ok {
+	if rf, ok := ret.Get(1).(func(string) error); ok {
 		r1 = rf(status)
 	} else {
-		if ret.Get(1) != nil {
+		r1 = ret.Error(1)
-			r1 = ret.Get(1).(*model.AppError)
-		}
 	}
 
 	return r0, r1
 }
 
 // GetAllByType provides a mock function with given fields: jobType
-func (_m *JobStore) GetAllByType(jobType string) ([]*model.Job, *model.AppError) {
+func (_m *JobStore) GetAllByType(jobType string) ([]*model.Job, error) {
 	ret := _m.Called(jobType)
 
 	var r0 []*model.Job
@@ -100,20 +94,18 @@ func (_m *JobStore) GetAllByType(jobType string) ([]*model.Job, *model.AppError)
 		}
 	}
 
-	var r1 *model.AppError
+	var r1 error
-	if rf, ok := ret.Get(1).(func(string) *model.AppError); ok {
+	if rf, ok := ret.Get(1).(func(string) error); ok {
 		r1 = rf(jobType)
 	} else {
-		if ret.Get(1) != nil {
+		r1 = ret.Error(1)
-			r1 = ret.Get(1).(*model.AppError)
-		}
 	}
 
 	return r0, r1
 }
 
 // GetAllByTypePage provides a mock function with given fields: jobType, offset, limit
-func (_m *JobStore) GetAllByTypePage(jobType string, offset int, limit int) ([]*model.Job, *model.AppError) {
+func (_m *JobStore) GetAllByTypePage(jobType string, offset int, limit int) ([]*model.Job, error) {
 	ret := _m.Called(jobType, offset, limit)
 
 	var r0 []*model.Job
@@ -125,20 +117,18 @@ func (_m *JobStore) GetAllByTypePage(jobType string, offset int, limit int) ([]*
 		}
 	}
 
-	var r1 *model.AppError
+	var r1 error
-	if rf, ok := ret.Get(1).(func(string, int, int) *model.AppError); ok {
+	if rf, ok := ret.Get(1).(func(string, int, int) error); ok {
 		r1 = rf(jobType, offset, limit)
 	} else {
-		if ret.Get(1) != nil {
+		r1 = ret.Error(1)
-			r1 = ret.Get(1).(*model.AppError)
-		}
 	}
 
 	return r0, r1
 }
 
 // GetAllPage provides a mock function with given fields: offset, limit
-func (_m *JobStore) GetAllPage(offset int, limit int) ([]*model.Job, *model.AppError) {
+func (_m *JobStore) GetAllPage(offset int, limit int) ([]*model.Job, error) {
 	ret := _m.Called(offset, limit)
 
 	var r0 []*model.Job
@@ -150,20 +140,18 @@ func (_m *JobStore) GetAllPage(offset int, limit int) ([]*model.Job, *model.AppE
 		}
 	}
 
-	var r1 *model.AppError
+	var r1 error
-	if rf, ok := ret.Get(1).(func(int, int) *model.AppError); ok {
+	if rf, ok := ret.Get(1).(func(int, int) error); ok {
 		r1 = rf(offset, limit)
 	} else {
-		if ret.Get(1) != nil {
+		r1 = ret.Error(1)
-			r1 = ret.Get(1).(*model.AppError)
-		}
 	}
 
 	return r0, r1
 }
 
 // GetCountByStatusAndType provides a mock function with given fields: status, jobType
-func (_m *JobStore) GetCountByStatusAndType(status string, jobType string) (int64, *model.AppError) {
+func (_m *JobStore) GetCountByStatusAndType(status string, jobType string) (int64, error) {
 	ret := _m.Called(status, jobType)
 
 	var r0 int64
@@ -173,20 +161,18 @@ func (_m *JobStore) GetCountByStatusAndType(status string, jobType string) (int6
 		r0 = ret.Get(0).(int64)
 	}
 
-	var r1 *model.AppError
+	var r1 error
-	if rf, ok := ret.Get(1).(func(string, string) *model.AppError); ok {
+	if rf, ok := ret.Get(1).(func(string, string) error); ok {
 		r1 = rf(status, jobType)
 	} else {
-		if ret.Get(1) != nil {
+		r1 = ret.Error(1)
-			r1 = ret.Get(1).(*model.AppError)
-		}
 	}
 
 	return r0, r1
 }
 
 // GetNewestJobByStatusAndType provides a mock function with given fields: status, jobType
-func (_m *JobStore) GetNewestJobByStatusAndType(status string, jobType string) (*model.Job, *model.AppError) {
+func (_m *JobStore) GetNewestJobByStatusAndType(status string, jobType string) (*model.Job, error) {
 	ret := _m.Called(status, jobType)
 
 	var r0 *model.Job
@@ -198,20 +184,18 @@ func (_m *JobStore) GetNewestJobByStatusAndType(status string, jobType string) (
 		}
 	}
 
-	var r1 *model.AppError
+	var r1 error
-	if rf, ok := ret.Get(1).(func(string, string) *model.AppError); ok {
+	if rf, ok := ret.Get(1).(func(string, string) error); ok {
 		r1 = rf(status, jobType)
 	} else {
-		if ret.Get(1) != nil {
+		r1 = ret.Error(1)
-			r1 = ret.Get(1).(*model.AppError)
-		}
 	}
 
 	return r0, r1
 }
 
 // GetNewestJobByStatusesAndType provides a mock function with given fields: statuses, jobType
-func (_m *JobStore) GetNewestJobByStatusesAndType(statuses []string, jobType string) (*model.Job, *model.AppError) {
+func (_m *JobStore) GetNewestJobByStatusesAndType(statuses []string, jobType string) (*model.Job, error) {
 	ret := _m.Called(statuses, jobType)
 
 	var r0 *model.Job
@@ -223,20 +207,18 @@ func (_m *JobStore) GetNewestJobByStatusesAndType(statuses []string, jobType str
 		}
 	}
 
-	var r1 *model.AppError
+	var r1 error
-	if rf, ok := ret.Get(1).(func([]string, string) *model.AppError); ok {
+	if rf, ok := ret.Get(1).(func([]string, string) error); ok {
 		r1 = rf(statuses, jobType)
 	} else {
-		if ret.Get(1) != nil {
+		r1 = ret.Error(1)
-			r1 = ret.Get(1).(*model.AppError)
-		}
 	}
 
 	return r0, r1
 }
 
 // Save provides a mock function with given fields: job
-func (_m *JobStore) Save(job *model.Job) (*model.Job, *model.AppError) {
+func (_m *JobStore) Save(job *model.Job) (*model.Job, error) {
 	ret := _m.Called(job)
 
 	var r0 *model.Job
@@ -248,20 +230,18 @@ func (_m *JobStore) Save(job *model.Job) (*model.Job, *model.AppError) {
 		}
 	}
 
-	var r1 *model.AppError
+	var r1 error
-	if rf, ok := ret.Get(1).(func(*model.Job) *model.AppError); ok {
+	if rf, ok := ret.Get(1).(func(*model.Job) error); ok {
 		r1 = rf(job)
 	} else {
-		if ret.Get(1) != nil {
+		r1 = ret.Error(1)
-			r1 = ret.Get(1).(*model.AppError)
-		}
 	}
 
 	return r0, r1
 }
 
 // UpdateOptimistically provides a mock function with given fields: job, currentStatus
-func (_m *JobStore) UpdateOptimistically(job *model.Job, currentStatus string) (bool, *model.AppError) {
+func (_m *JobStore) UpdateOptimistically(job *model.Job, currentStatus string) (bool, error) {
 	ret := _m.Called(job, currentStatus)
 
 	var r0 bool
@@ -271,20 +251,18 @@ func (_m *JobStore) UpdateOptimistically(job *model.Job, currentStatus string) (
 		r0 = ret.Get(0).(bool)
 	}
 
-	var r1 *model.AppError
+	var r1 error
-	if rf, ok := ret.Get(1).(func(*model.Job, string) *model.AppError); ok {
+	if rf, ok := ret.Get(1).(func(*model.Job, string) error); ok {
 		r1 = rf(job, currentStatus)
 	} else {
-		if ret.Get(1) != nil {
+		r1 = ret.Error(1)
-			r1 = ret.Get(1).(*model.AppError)
-		}
 	}
 
 	return r0, r1
 }
 
 // UpdateStatus provides a mock function with given fields: id, status
-func (_m *JobStore) UpdateStatus(id string, status string) (*model.Job, *model.AppError) {
+func (_m *JobStore) UpdateStatus(id string, status string) (*model.Job, error) {
 	ret := _m.Called(id, status)
 
 	var r0 *model.Job
@@ -296,20 +274,18 @@ func (_m *JobStore) UpdateStatus(id string, status string) (*model.Job, *model.A
 		}
 	}
 
-	var r1 *model.AppError
+	var r1 error
-	if rf, ok := ret.Get(1).(func(string, string) *model.AppError); ok {
+	if rf, ok := ret.Get(1).(func(string, string) error); ok {
 		r1 = rf(id, status)
 	} else {
-		if ret.Get(1) != nil {
+		r1 = ret.Error(1)
-			r1 = ret.Get(1).(*model.AppError)
-		}
 	}
 
 	return r0, r1
 }
 
 // UpdateStatusOptimistically provides a mock function with given fields: id, currentStatus, newStatus
-func (_m *JobStore) UpdateStatusOptimistically(id string, currentStatus string, newStatus string) (bool, *model.AppError) {
+func (_m *JobStore) UpdateStatusOptimistically(id string, currentStatus string, newStatus string) (bool, error) {
 	ret := _m.Called(id, currentStatus, newStatus)
 
 	var r0 bool
@@ -319,13 +295,11 @@ func (_m *JobStore) UpdateStatusOptimistically(id string, currentStatus string,
 		r0 = ret.Get(0).(bool)
 	}
 
-	var r1 *model.AppError
+	var r1 error
-	if rf, ok := ret.Get(1).(func(string, string, string) *model.AppError); ok {
+	if rf, ok := ret.Get(1).(func(string, string, string) error); ok {
 		r1 = rf(id, currentStatus, newStatus)
 	} else {
-		if ret.Get(1) != nil {
+		r1 = ret.Error(1)
-			r1 = ret.Get(1).(*model.AppError)
-		}
 	}
 
 	return r0, r1

store/timerlayer/timerlayer.go

@@ -3535,7 +3535,7 @@ func (s *TimerLayerGroupStore) UpsertMember(groupID string, userID string) (*mod
 	return result, err
 }
 
-func (s *TimerLayerJobStore) Delete(id string) (string, *model.AppError) {
+func (s *TimerLayerJobStore) Delete(id string) (string, error) {
 	start := timemodule.Now()
 
 	result, err := s.JobStore.Delete(id)
@@ -3551,7 +3551,7 @@ func (s *TimerLayerJobStore) Delete(id string) (string, *model.AppError) {
 	return result, err
 }
 
-func (s *TimerLayerJobStore) Get(id string) (*model.Job, *model.AppError) {
+func (s *TimerLayerJobStore) Get(id string) (*model.Job, error) {
 	start := timemodule.Now()
 
 	result, err := s.JobStore.Get(id)
@@ -3567,7 +3567,7 @@ func (s *TimerLayerJobStore) Get(id string) (*model.Job, *model.AppError) {
 	return result, err
 }
 
-func (s *TimerLayerJobStore) GetAllByStatus(status string) ([]*model.Job, *model.AppError) {
+func (s *TimerLayerJobStore) GetAllByStatus(status string) ([]*model.Job, error) {
 	start := timemodule.Now()
 
 	result, err := s.JobStore.GetAllByStatus(status)
@@ -3583,7 +3583,7 @@ func (s *TimerLayerJobStore) GetAllByStatus(status string) ([]*model.Job, *model
 	return result, err
 }
 
-func (s *TimerLayerJobStore) GetAllByType(jobType string) ([]*model.Job, *model.AppError) {
+func (s *TimerLayerJobStore) GetAllByType(jobType string) ([]*model.Job, error) {
 	start := timemodule.Now()
 
 	result, err := s.JobStore.GetAllByType(jobType)
@@ -3599,7 +3599,7 @@ func (s *TimerLayerJobStore) GetAllByType(jobType string) ([]*model.Job, *model.
 	return result, err
 }
 
-func (s *TimerLayerJobStore) GetAllByTypePage(jobType string, offset int, limit int) ([]*model.Job, *model.AppError) {
+func (s *TimerLayerJobStore) GetAllByTypePage(jobType string, offset int, limit int) ([]*model.Job, error) {
 	start := timemodule.Now()
 
 	result, err := s.JobStore.GetAllByTypePage(jobType, offset, limit)
@@ -3615,7 +3615,7 @@ func (s *TimerLayerJobStore) GetAllByTypePage(jobType string, offset int, limit
 	return result, err
 }
 
-func (s *TimerLayerJobStore) GetAllPage(offset int, limit int) ([]*model.Job, *model.AppError) {
+func (s *TimerLayerJobStore) GetAllPage(offset int, limit int) ([]*model.Job, error) {
 	start := timemodule.Now()
 
 	result, err := s.JobStore.GetAllPage(offset, limit)
@@ -3631,7 +3631,7 @@ func (s *TimerLayerJobStore) GetAllPage(offset int, limit int) ([]*model.Job, *m
 	return result, err
 }
 
-func (s *TimerLayerJobStore) GetCountByStatusAndType(status string, jobType string) (int64, *model.AppError) {
+func (s *TimerLayerJobStore) GetCountByStatusAndType(status string, jobType string) (int64, error) {
 	start := timemodule.Now()
 
 	result, err := s.JobStore.GetCountByStatusAndType(status, jobType)
@@ -3647,7 +3647,7 @@ func (s *TimerLayerJobStore) GetCountByStatusAndType(status string, jobType stri
 	return result, err
 }
 
-func (s *TimerLayerJobStore) GetNewestJobByStatusAndType(status string, jobType string) (*model.Job, *model.AppError) {
+func (s *TimerLayerJobStore) GetNewestJobByStatusAndType(status string, jobType string) (*model.Job, error) {
 	start := timemodule.Now()
 
 	result, err := s.JobStore.GetNewestJobByStatusAndType(status, jobType)
@@ -3663,7 +3663,7 @@ func (s *TimerLayerJobStore) GetNewestJobByStatusAndType(status string, jobType
 	return result, err
 }
 
-func (s *TimerLayerJobStore) GetNewestJobByStatusesAndType(statuses []string, jobType string) (*model.Job, *model.AppError) {
+func (s *TimerLayerJobStore) GetNewestJobByStatusesAndType(statuses []string, jobType string) (*model.Job, error) {
 	start := timemodule.Now()
 
 	result, err := s.JobStore.GetNewestJobByStatusesAndType(statuses, jobType)
@@ -3679,7 +3679,7 @@ func (s *TimerLayerJobStore) GetNewestJobByStatusesAndType(statuses []string, jo
 	return result, err
 }
 
-func (s *TimerLayerJobStore) Save(job *model.Job) (*model.Job, *model.AppError) {
+func (s *TimerLayerJobStore) Save(job *model.Job) (*model.Job, error) {
 	start := timemodule.Now()
 
 	result, err := s.JobStore.Save(job)
@@ -3695,7 +3695,7 @@ func (s *TimerLayerJobStore) Save(job *model.Job) (*model.Job, *model.AppError)
 	return result, err
 }
 
-func (s *TimerLayerJobStore) UpdateOptimistically(job *model.Job, currentStatus string) (bool, *model.AppError) {
+func (s *TimerLayerJobStore) UpdateOptimistically(job *model.Job, currentStatus string) (bool, error) {
 	start := timemodule.Now()
 
 	result, err := s.JobStore.UpdateOptimistically(job, currentStatus)
@@ -3711,7 +3711,7 @@ func (s *TimerLayerJobStore) UpdateOptimistically(job *model.Job, currentStatus
 	return result, err
 }
 
-func (s *TimerLayerJobStore) UpdateStatus(id string, status string) (*model.Job, *model.AppError) {
+func (s *TimerLayerJobStore) UpdateStatus(id string, status string) (*model.Job, error) {
 	start := timemodule.Now()
 
 	result, err := s.JobStore.UpdateStatus(id, status)
@@ -3727,7 +3727,7 @@ func (s *TimerLayerJobStore) UpdateStatus(id string, status string) (*model.Job,
 	return result, err
 }
 
-func (s *TimerLayerJobStore) UpdateStatusOptimistically(id string, currentStatus string, newStatus string) (bool, *model.AppError) {
+func (s *TimerLayerJobStore) UpdateStatusOptimistically(id string, currentStatus string, newStatus string) (bool, error) {
 	start := timemodule.Now()
 
 	result, err := s.JobStore.UpdateStatusOptimistically(id, currentStatus, newStatus)