From cd2fe8170ecf5342a46ffffa9765ffeca2d55f30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Fri, 4 Mar 2022 11:30:13 +0100 Subject: [PATCH] Errors: remove "error" prefix from message Instead of returning an error "error doing X", just return "doing X". The fact that it's returned as an error object says enough about that it's an error. This also makes it easier to chain error messages, without seeing the word "error" in every part of the chain. --- internal/manager/config/settings.go | 4 ++-- internal/manager/persistence/jobs.go | 12 ++++++------ internal/manager/persistence/task_scheduler.go | 6 +++--- internal/manager/persistence/workers.go | 6 +++--- internal/manager/task_logs/task_logs.go | 2 +- .../manager/task_state_machine/task_state_machine.go | 12 ++++++------ internal/worker/command_blender.go | 2 +- internal/worker/config.go | 6 +++--- internal/worker/task_executor.go | 6 +++--- internal/worker/upstream_buffer.go | 6 +++--- pkg/api/openapi_spec.gen.go | 6 +++--- pkg/api/openapi_types.gen.go | 8 ++++---- 12 files changed, 38 insertions(+), 38 deletions(-) diff --git a/internal/manager/config/settings.go b/internal/manager/config/settings.go index 4552d2cb..d9c2a77b 100644 --- a/internal/manager/config/settings.go +++ b/internal/manager/config/settings.go @@ -413,10 +413,10 @@ func (c *Conf) checkDatabase() { func (c *Conf) Overwrite() error { tempFilename := configFilename + "~" if err := c.Write(tempFilename); err != nil { - return fmt.Errorf("error writing config to %s: %w", tempFilename, err) + return fmt.Errorf("writing config to %s: %w", tempFilename, err) } if err := os.Rename(tempFilename, configFilename); err != nil { - return fmt.Errorf("error moving %s to %s: %w", tempFilename, configFilename, err) + return fmt.Errorf("moving %s to %s: %w", tempFilename, configFilename, err) } log.Info().Str("filename", configFilename).Msg("saved configuration to file") diff --git a/internal/manager/persistence/jobs.go b/internal/manager/persistence/jobs.go index 875c106c..b7b9d807 100644 --- a/internal/manager/persistence/jobs.go +++ b/internal/manager/persistence/jobs.go @@ -127,7 +127,7 @@ func (db *DB) StoreAuthoredJob(ctx context.Context, authoredJob job_compilers.Au } if err := tx.Create(&dbJob).Error; err != nil { - return fmt.Errorf("error storing job: %v", err) + return fmt.Errorf("storing job: %v", err) } uuidToTask := make(map[string]*Task) @@ -151,7 +151,7 @@ func (db *DB) StoreAuthoredJob(ctx context.Context, authoredJob job_compilers.Au // dependencies are stored below. } if err := tx.Create(&dbTask).Error; err != nil { - return fmt.Errorf("error storing task: %v", err) + return fmt.Errorf("storing task: %v", err) } uuidToTask[authoredTask.UUID] = &dbTask @@ -172,7 +172,7 @@ func (db *DB) StoreAuthoredJob(ctx context.Context, authoredJob job_compilers.Au for i, t := range authoredTask.Dependencies { depTask, ok := uuidToTask[t.UUID] if !ok { - return fmt.Errorf("error finding task with UUID %q; a task depends on a task that is not part of this job", t.UUID) + return fmt.Errorf("finding task with UUID %q; a task depends on a task that is not part of this job", t.UUID) } deps[i] = depTask } @@ -202,7 +202,7 @@ func (db *DB) SaveJobStatus(ctx context.Context, j *Job) error { Model(j). Updates(Job{Status: j.Status}) if tx.Error != nil { - return fmt.Errorf("error saving job status: %w", tx.Error) + return fmt.Errorf("saving job status: %w", tx.Error) } return nil } @@ -220,14 +220,14 @@ func (db *DB) FetchTask(ctx context.Context, taskUUID string) (*Task, error) { func (db *DB) SaveTask(ctx context.Context, t *Task) error { if err := db.gormDB.WithContext(ctx).Save(t).Error; err != nil { - return fmt.Errorf("error saving task: %w", err) + return fmt.Errorf("saving task: %w", err) } return nil } func (db *DB) SaveTaskActivity(ctx context.Context, t *Task) error { if err := db.gormDB.Model(t).Updates(Task{Activity: t.Activity}).Error; err != nil { - return fmt.Errorf("error saving task activity: %w", err) + return fmt.Errorf("saving task activity: %w", err) } return nil } diff --git a/internal/manager/persistence/task_scheduler.go b/internal/manager/persistence/task_scheduler.go index a3506156..ea9be6e0 100644 --- a/internal/manager/persistence/task_scheduler.go +++ b/internal/manager/persistence/task_scheduler.go @@ -51,7 +51,7 @@ func (db *DB) ScheduleTask(ctx context.Context, w *Worker) (*Task, error) { var err error task, err = findTaskForWorker(tx, w) if err != nil { - return fmt.Errorf("error finding task for worker: %w", err) + return fmt.Errorf("finding task for worker: %w", err) } if task == nil { // No task found, which is fine. @@ -65,7 +65,7 @@ func (db *DB) ScheduleTask(ctx context.Context, w *Worker) (*Task, error) { Str("taskID", task.UUID). Err(err). Msg("error assigning task to worker") - return fmt.Errorf("error assigning task to worker: %w", err) + return fmt.Errorf("assigning task to worker: %w", err) } return nil @@ -73,7 +73,7 @@ func (db *DB) ScheduleTask(ctx context.Context, w *Worker) (*Task, error) { if txErr != nil { logger.Error().Err(txErr).Msg("error finding task for worker") - return nil, fmt.Errorf("error finding task for worker: %w", txErr) + return nil, fmt.Errorf("finding task for worker: %w", txErr) } if task == nil { diff --git a/internal/manager/persistence/workers.go b/internal/manager/persistence/workers.go index 6e77d064..f2b98dbd 100644 --- a/internal/manager/persistence/workers.go +++ b/internal/manager/persistence/workers.go @@ -53,7 +53,7 @@ func (w *Worker) TaskTypes() []string { func (db *DB) CreateWorker(ctx context.Context, w *Worker) error { if err := db.gormDB.WithContext(ctx).Create(w).Error; err != nil { - return fmt.Errorf("error creating new worker: %w", err) + return fmt.Errorf("creating new worker: %w", err) } return nil } @@ -77,14 +77,14 @@ func (db *DB) SaveWorkerStatus(ctx context.Context, w *Worker) error { StatusRequested: w.StatusRequested, }).Error if err != nil { - return fmt.Errorf("error saving worker: %w", err) + return fmt.Errorf("saving worker: %w", err) } return nil } func (db *DB) SaveWorker(ctx context.Context, w *Worker) error { if err := db.gormDB.WithContext(ctx).Save(w).Error; err != nil { - return fmt.Errorf("error saving worker: %w", err) + return fmt.Errorf("saving worker: %w", err) } return nil } diff --git a/internal/manager/task_logs/task_logs.go b/internal/manager/task_logs/task_logs.go index ac26d51e..9a0b0874 100644 --- a/internal/manager/task_logs/task_logs.go +++ b/internal/manager/task_logs/task_logs.go @@ -66,7 +66,7 @@ func (s *Storage) Write(logger zerolog.Logger, jobID, taskID string, logText str if err := os.MkdirAll(path.Dir(filepath), 0755); err != nil { logger.Error().Err(err).Msg("unable to create directory for log file") - return fmt.Errorf("error creating directory: %w", err) + return fmt.Errorf("creating directory: %w", err) } file, err := os.OpenFile(filepath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644) diff --git a/internal/manager/task_state_machine/task_state_machine.go b/internal/manager/task_state_machine/task_state_machine.go index 9d29a580..2542030d 100644 --- a/internal/manager/task_state_machine/task_state_machine.go +++ b/internal/manager/task_state_machine/task_state_machine.go @@ -94,10 +94,10 @@ func (sm *StateMachine) TaskStatusChange( logger.Debug().Msg("task state changed") if err := sm.persist.SaveTask(ctx, task); err != nil { - return fmt.Errorf("error saving task to database: %w", err) + return fmt.Errorf("saving task to database: %w", err) } if err := sm.updateJobAfterTaskStatusChange(ctx, task, oldTaskStatus); err != nil { - return fmt.Errorf("error updating job after task status change: %w", err) + return fmt.Errorf("updating job after task status change: %w", err) } return nil } @@ -253,14 +253,14 @@ func (sm *StateMachine) JobStatusChange(ctx context.Context, job *persistence.Jo // Persist the new job status. err = sm.persist.SaveJobStatus(ctx, job) if err != nil { - return fmt.Errorf("error saving job status change %q to %q to database: %w", + return fmt.Errorf("saving job status change %q to %q to database: %w", oldJobStatus, newJobStatus, err) } // Handle the status change. newJobStatus, err = sm.updateTasksAfterJobStatusChange(ctx, logger, job, oldJobStatus) if err != nil { - return fmt.Errorf("error updating job's tasks after job status change: %w", err) + return fmt.Errorf("updating job's tasks after job status change: %w", err) } } @@ -326,7 +326,7 @@ func (sm *StateMachine) cancelTasks( fmt.Sprintf("Manager cancelled this task because the job got status %q.", job.Status), ) if err != nil { - return "", fmt.Errorf("error cancelling tasks of job %s: %w", job.UUID, err) + return "", fmt.Errorf("cancelling tasks of job %s: %w", job.UUID, err) } // If cancellation was requested, it has now happened, so the job can transition. @@ -380,7 +380,7 @@ func (sm *StateMachine) requeueTasks( } if err != nil { - return "", fmt.Errorf("error queueing tasks of job %s: %w", job.UUID, err) + return "", fmt.Errorf("queueing tasks of job %s: %w", job.UUID, err) } // TODO: also reset the 'failed by workers' blacklist. diff --git a/internal/worker/command_blender.go b/internal/worker/command_blender.go index 56bdc361..6674b807 100644 --- a/internal/worker/command_blender.go +++ b/internal/worker/command_blender.go @@ -176,7 +176,7 @@ func cmdBlenderRenderParams(logger zerolog.Logger, cmd api.Command) (BlenderPara exeArgs, err := shlex.Split(parameters.exe) if err != nil { logger.Warn().Err(err).Interface("command", cmd).Msg("error parsing 'exe' parameter with shlex") - return parameters, fmt.Errorf("error parsing 'exe' parameter %q: %w", parameters.exe, err) + return parameters, fmt.Errorf("parsing 'exe' parameter %q: %w", parameters.exe, err) } if len(exeArgs) > 1 { allArgsBefore := []string{} diff --git a/internal/worker/config.go b/internal/worker/config.go index 8af2c462..216e752a 100644 --- a/internal/worker/config.go +++ b/internal/worker/config.go @@ -66,19 +66,19 @@ func loadConfig(configWrangler FileConfigWrangler) (WorkerConfig, error) { cfg = configWrangler.DefaultConfig() err = configWrangler.WriteConfig(configFilename, "Configuration", cfg) if err != nil { - return cfg, fmt.Errorf("error writing default config: %w", err) + return cfg, fmt.Errorf("writing default config: %w", err) } err = configWrangler.LoadConfig(configFilename, &cfg) } if err != nil { - return cfg, fmt.Errorf("error loading config from %s: %w", configFilename, err) + return cfg, fmt.Errorf("loading config from %s: %w", configFilename, err) } // Validate the manager URL. if cfg.Manager != "" { _, err := ParseURL(cfg.Manager) if err != nil { - return cfg, fmt.Errorf("error parsing manager URL %s: %w", cfg.Manager, err) + return cfg, fmt.Errorf("parsing manager URL %s: %w", cfg.Manager, err) } logger.Debug().Str("url", cfg.Manager).Msg("parsed manager URL") } diff --git a/internal/worker/task_executor.go b/internal/worker/task_executor.go index aba6e70b..8c3333a7 100644 --- a/internal/worker/task_executor.go +++ b/internal/worker/task_executor.go @@ -72,7 +72,7 @@ func (te *TaskExecutor) Run(ctx context.Context, task api.AssignedTask) error { if err == ErrTaskReassigned { return ErrTaskReassigned } - return fmt.Errorf("error sending 'task started' notification to manager: %w", err) + return fmt.Errorf("sending 'task started' notification to manager: %w", err) } for _, cmd := range task.Commands { @@ -96,7 +96,7 @@ func (te *TaskExecutor) Run(ctx context.Context, task api.AssignedTask) error { if err == ErrTaskReassigned { return ErrTaskReassigned } - return fmt.Errorf("error sending 'task failed' notification to manager: %w", err) + return fmt.Errorf("sending 'task failed' notification to manager: %w", err) } return runErr } @@ -105,7 +105,7 @@ func (te *TaskExecutor) Run(ctx context.Context, task api.AssignedTask) error { if err == ErrTaskReassigned { return ErrTaskReassigned } - return fmt.Errorf("error sending 'task completed' notification to manager: %w", err) + return fmt.Errorf("sending 'task completed' notification to manager: %w", err) } return nil diff --git a/internal/worker/upstream_buffer.go b/internal/worker/upstream_buffer.go index 79524dd5..00881b2a 100644 --- a/internal/worker/upstream_buffer.go +++ b/internal/worker/upstream_buffer.go @@ -79,11 +79,11 @@ func (ub *UpstreamBufferDB) OpenDB(ctx context.Context, databaseFilename string) db, err := sql.Open("sqlite", databaseFilename) if err != nil { - return fmt.Errorf("error opening %s: %w", databaseFilename, err) + return fmt.Errorf("opening %s: %w", databaseFilename, err) } if err := db.PingContext(ctx); err != nil { - return fmt.Errorf("error accessing %s: %w", databaseFilename, err) + return fmt.Errorf("accessing %s: %w", databaseFilename, err) } ub.db = db @@ -325,7 +325,7 @@ func (ub *UpstreamBufferDB) discardRow(ctx context.Context, tx *sql.Tx, rowID in _, err := tx.ExecContext(ctx, stmt, rowID) if err != nil { - return fmt.Errorf("error un-queueing task update: %w", err) + return fmt.Errorf("un-queueing task update: %w", err) } return nil } diff --git a/pkg/api/openapi_spec.gen.go b/pkg/api/openapi_spec.gen.go index c77b810b..32ba9fe1 100644 --- a/pkg/api/openapi_spec.gen.go +++ b/pkg/api/openapi_spec.gen.go @@ -81,16 +81,16 @@ var swaggerSpec = []string{ func decodeSpec() ([]byte, error) { zipped, err := base64.StdEncoding.DecodeString(strings.Join(swaggerSpec, "")) if err != nil { - return nil, fmt.Errorf("error base64 decoding spec: %s", err) + return nil, fmt.Errorf("base64 decoding spec: %s", err) } zr, err := gzip.NewReader(bytes.NewReader(zipped)) if err != nil { - return nil, fmt.Errorf("error decompressing spec: %s", err) + return nil, fmt.Errorf("decompressing spec: %s", err) } var buf bytes.Buffer _, err = buf.ReadFrom(zr) if err != nil { - return nil, fmt.Errorf("error decompressing spec: %s", err) + return nil, fmt.Errorf("decompressing spec: %s", err) } return buf.Bytes(), nil diff --git a/pkg/api/openapi_types.gen.go b/pkg/api/openapi_types.gen.go index da42dd62..3b029f7a 100644 --- a/pkg/api/openapi_types.gen.go +++ b/pkg/api/openapi_types.gen.go @@ -330,7 +330,7 @@ func (a *JobMetadata) UnmarshalJSON(b []byte) error { var fieldVal string err := json.Unmarshal(fieldBuf, &fieldVal) if err != nil { - return fmt.Errorf("error unmarshaling field %s: %w", fieldName, err) + return fmt.Errorf("unmarshaling field %s: %w", fieldName, err) } a.AdditionalProperties[fieldName] = fieldVal } @@ -346,7 +346,7 @@ func (a JobMetadata) MarshalJSON() ([]byte, error) { for fieldName, field := range a.AdditionalProperties { object[fieldName], err = json.Marshal(field) if err != nil { - return nil, fmt.Errorf("error marshaling '%s': %w", fieldName, err) + return nil, fmt.Errorf("marshaling '%s': %w", fieldName, err) } } return json.Marshal(object) @@ -383,7 +383,7 @@ func (a *JobSettings) UnmarshalJSON(b []byte) error { var fieldVal interface{} err := json.Unmarshal(fieldBuf, &fieldVal) if err != nil { - return fmt.Errorf("error unmarshaling field %s: %w", fieldName, err) + return fmt.Errorf("unmarshaling field %s: %w", fieldName, err) } a.AdditionalProperties[fieldName] = fieldVal } @@ -399,7 +399,7 @@ func (a JobSettings) MarshalJSON() ([]byte, error) { for fieldName, field := range a.AdditionalProperties { object[fieldName], err = json.Marshal(field) if err != nil { - return nil, fmt.Errorf("error marshaling '%s': %w", fieldName, err) + return nil, fmt.Errorf("marshaling '%s': %w", fieldName, err) } } return json.Marshal(object)