diff --git a/internal/worker/command_blender.go b/internal/worker/command_blender.go index ced7e431..03ab52fd 100644 --- a/internal/worker/command_blender.go +++ b/internal/worker/command_blender.go @@ -157,26 +157,26 @@ func cmdBlenderRenderParams(logger zerolog.Logger, cmd api.Command) (BlenderPara if parameters.exe, ok = cmdParameter[string](cmd, "exe"); !ok || parameters.exe == "" { logger.Warn().Interface("command", cmd).Msg("missing 'exe' parameter") - return parameters, fmt.Errorf("missing 'exe' parameter: %+v", cmd.Parameters) + return parameters, NewParameterMissingError("exe", cmd) } if parameters.argsBefore, ok = cmdParameterAsStrings(cmd, "argsBefore"); !ok { logger.Warn().Interface("command", cmd).Msg("invalid 'argsBefore' parameter") - return parameters, fmt.Errorf("invalid 'argsBefore' parameter: %+v", cmd.Parameters) + return parameters, NewParameterInvalidError("argsBefore", cmd, "cannot convert to list of strings") } if parameters.blendfile, ok = cmdParameter[string](cmd, "blendfile"); !ok || parameters.blendfile == "" { logger.Warn().Interface("command", cmd).Msg("missing 'blendfile' parameter") - return parameters, fmt.Errorf("missing 'blendfile' parameter: %+v", cmd.Parameters) + return parameters, NewParameterMissingError("blendfile", cmd) } if parameters.args, ok = cmdParameterAsStrings(cmd, "args"); !ok { logger.Warn().Interface("command", cmd).Msg("invalid 'args' parameter") - return parameters, fmt.Errorf("invalid 'args' parameter: %+v", cmd.Parameters) + return parameters, NewParameterInvalidError("args", cmd, "cannot convert to list of strings") } // Move any CLI args from 'exe' to 'argsBefore'. 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("parsing 'exe' parameter %q: %w", parameters.exe, err) + return parameters, NewParameterInvalidError("exe", cmd, err.Error()) } if len(exeArgs) > 1 { allArgsBefore := []string{} diff --git a/internal/worker/command_ffmpeg.go b/internal/worker/command_ffmpeg.go index 88c25197..445bb2d2 100644 --- a/internal/worker/command_ffmpeg.go +++ b/internal/worker/command_ffmpeg.go @@ -162,34 +162,34 @@ func cmdFramesToVideoParams(logger zerolog.Logger, cmd api.Command) (CreateVideo if parameters.exe, ok = cmdParameter[string](cmd, "exe"); !ok || parameters.exe == "" { logger.Warn().Interface("command", cmd).Msg("missing 'exe' parameter") - return parameters, fmt.Errorf("missing 'exe' parameter: %+v", cmd.Parameters) + return parameters, NewParameterMissingError("exe", cmd) } if parameters.fps, ok = cmdParameter[float64](cmd, "fps"); !ok || parameters.fps == 0.0 { logger.Warn().Interface("command", cmd).Msg("missing 'fps' parameter") - return parameters, fmt.Errorf("missing 'fps' parameter: %+v", cmd.Parameters) + return parameters, NewParameterMissingError("fps", cmd) } if parameters.inputGlob, ok = cmdParameter[string](cmd, "inputGlob"); !ok || parameters.inputGlob == "" { logger.Warn().Interface("command", cmd).Msg("missing 'inputGlob' parameter") - return parameters, fmt.Errorf("missing 'inputGlob' parameter: %+v", cmd.Parameters) + return parameters, NewParameterMissingError("inputGlob", cmd) } if parameters.outputFile, ok = cmdParameter[string](cmd, "outputFile"); !ok || parameters.outputFile == "" { logger.Warn().Interface("command", cmd).Msg("missing 'outputFile' parameter") - return parameters, fmt.Errorf("missing 'outputFile' parameter: %+v", cmd.Parameters) + return parameters, NewParameterMissingError("outputFile", cmd) } if parameters.argsBefore, ok = cmdParameterAsStrings(cmd, "argsBefore"); !ok { logger.Warn().Interface("command", cmd).Msg("invalid 'argsBefore' parameter") - return parameters, fmt.Errorf("invalid 'argsBefore' parameter: %+v", cmd.Parameters) + return parameters, NewParameterInvalidError("argsBefore", cmd, "cannot convert to list of strings") } if parameters.args, ok = cmdParameterAsStrings(cmd, "args"); !ok { logger.Warn().Interface("command", cmd).Msg("invalid 'args' parameter") - return parameters, fmt.Errorf("invalid 'args' parameter: %+v", cmd.Parameters) + return parameters, NewParameterInvalidError("args", cmd, "cannot convert to list of strings") } // Move any CLI args from 'exe' to 'argsBefore'. 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("parsing 'exe' parameter %q: %w", parameters.exe, err) + return parameters, NewParameterInvalidError("exe", cmd, err.Error()) } if len(exeArgs) > 1 { allArgsBefore := []string{} diff --git a/internal/worker/command_file_mgmt.go b/internal/worker/command_file_mgmt.go index 2fd75ec6..eeae6b57 100644 --- a/internal/worker/command_file_mgmt.go +++ b/internal/worker/command_file_mgmt.go @@ -26,11 +26,11 @@ func (ce *CommandExecutor) cmdMoveDirectory(ctx context.Context, logger zerolog. if src, ok = cmdParameter[string](cmd, "src"); !ok || src == "" { logger.Warn().Interface("command", cmd).Msg("missing 'src' parameter") - return fmt.Errorf("missing 'src' parameter: %+v", cmd.Parameters) + return NewParameterMissingError("src", cmd) } if dest, ok = cmdParameter[string](cmd, "dest"); !ok || dest == "" { logger.Warn().Interface("command", cmd).Msg("missing 'dest' parameter") - return fmt.Errorf("missing 'dest' parameter: %+v", cmd.Parameters) + return NewParameterMissingError("dest", cmd) } logger = logger.With(). @@ -39,12 +39,11 @@ func (ce *CommandExecutor) cmdMoveDirectory(ctx context.Context, logger zerolog. Logger() if !fileExists(src) { logger.Warn().Msg("source path does not exist, not moving anything") - msg := fmt.Sprintf("%s: source path %q does not exist, not moving anything", cmd.Name, src) if err := ce.listener.LogProduced(ctx, taskID, msg); err != nil { return err } - return fmt.Errorf(msg) + return NewParameterInvalidError("src", cmd, "path does not exist") } if fileExists(dest) { diff --git a/internal/worker/command_file_mgmt_test.go b/internal/worker/command_file_mgmt_test.go index c51da53c..02ae0a96 100644 --- a/internal/worker/command_file_mgmt_test.go +++ b/internal/worker/command_file_mgmt_test.go @@ -39,7 +39,11 @@ func TestCmdMoveDirectoryNonExistentSourceDir(t *testing.T) { f.mocks.listener.EXPECT().LogProduced(gomock.Any(), taskID, "move-directory: source path \"render/output/here__intermediate\" does not exist, not moving anything") err := f.run() - assert.Error(t, err) + var paramErr ParameterInvalidError + if assert.ErrorAs(t, err, ¶mErr) { + assert.Equal(t, "src", paramErr.Parameter) + assert.Equal(t, "path does not exist", paramErr.Message) + } } func TestCmdMoveDirectoryHappy(t *testing.T) { diff --git a/internal/worker/command_misc.go b/internal/worker/command_misc.go index ad992d8f..b40d0465 100644 --- a/internal/worker/command_misc.go +++ b/internal/worker/command_misc.go @@ -6,7 +6,6 @@ package worker import ( "context" - "errors" "fmt" "time" @@ -20,7 +19,7 @@ import ( func (ce *CommandExecutor) cmdEcho(ctx context.Context, logger zerolog.Logger, taskID string, cmd api.Command) error { message, ok := cmd.Parameters["message"] if !ok { - return fmt.Errorf("missing 'message' setting") + return NewParameterMissingError("message", cmd) } messageStr := fmt.Sprintf("%v", message) @@ -36,7 +35,7 @@ func (ce *CommandExecutor) cmdSleep(ctx context.Context, logger zerolog.Logger, sleepTime, ok := cmd.Parameters["duration_in_seconds"] if !ok { - return errors.New("missing setting 'duration_in_seconds'") + return NewParameterMissingError("duration_in_seconds", cmd) } var duration time.Duration @@ -47,7 +46,7 @@ func (ce *CommandExecutor) cmdSleep(ctx context.Context, logger zerolog.Logger, duration = time.Duration(v) * time.Second default: log.Warn().Interface("duration_in_seconds", v).Msg("bad type for setting 'duration_in_seconds', expected int") - return fmt.Errorf("bad type for setting 'duration_in_seconds', expected int, not %T", v) + return NewParameterInvalidError("duration_in_seconds", cmd, "bad type %T, expecting int or float", sleepTime) } log.Info().Str("duration", duration.String()).Msg("sleep") diff --git a/internal/worker/errors.go b/internal/worker/errors.go new file mode 100644 index 00000000..c96c020f --- /dev/null +++ b/internal/worker/errors.go @@ -0,0 +1,63 @@ +package worker + +// SPDX-License-Identifier: GPL-3.0-or-later + +import ( + "fmt" + + "git.blender.org/flamenco/pkg/api" +) + +// This file contains error types used in the rest of the Worker code. + +// ParameterInvalidError is returned by command executors when a mandatory +// command parameter is missing. +type ParameterMissingError struct { + Parameter string + Cmd api.Command +} + +func NewParameterMissingError(parameter string, cmd api.Command) ParameterMissingError { + return ParameterMissingError{ + Parameter: parameter, + Cmd: cmd, + } +} + +func (err ParameterMissingError) Error() string { + return fmt.Sprintf("%s: mandatory parameter %q is missing: %+v", + err.Cmd.Name, err.Parameter, err.Cmd.Parameters) +} + +// ParameterInvalidError is returned by command executors when a command +// parameter is invalid. +type ParameterInvalidError struct { + Parameter string + Cmd api.Command + Message string +} + +func NewParameterInvalidError(parameter string, cmd api.Command, message string, fmtArgs ...interface{}) ParameterInvalidError { + if len(fmtArgs) > 0 { + message = fmt.Sprintf(message, fmtArgs...) + } + return ParameterInvalidError{ + Parameter: parameter, + Cmd: cmd, + Message: message, + } +} + +func (err ParameterInvalidError) Error() string { + return fmt.Sprintf("%s: parameter %q has invalid value %+v: %s", + err.Cmd.Name, err.Parameter, err.ParamValue(), err.Message) +} + +// ParamValue returns the value of the invalid parameter. +func (err ParameterInvalidError) ParamValue() interface{} { + return err.Cmd.Parameters[err.Parameter] +} + +// Ensure the structs above adhere to the error interface. +var _ error = ParameterMissingError{} +var _ error = ParameterInvalidError{}