Worker: use explicit types for command parameter errors

Introduce `ParameterMissingError` and `ParameterInvalidError` structs, to
be returned from command executors. These replace free-form `fmt.Errorf()`
style errors.
This commit is contained in:
Sybren A. Stüvel 2022-06-16 15:45:09 +02:00
parent 8af1b9d976
commit d5d0893b05
6 changed files with 86 additions and 21 deletions

View File

@ -157,26 +157,26 @@ func cmdBlenderRenderParams(logger zerolog.Logger, cmd api.Command) (BlenderPara
if parameters.exe, ok = cmdParameter[string](cmd, "exe"); !ok || parameters.exe == "" { if parameters.exe, ok = cmdParameter[string](cmd, "exe"); !ok || parameters.exe == "" {
logger.Warn().Interface("command", cmd).Msg("missing 'exe' parameter") 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 { if parameters.argsBefore, ok = cmdParameterAsStrings(cmd, "argsBefore"); !ok {
logger.Warn().Interface("command", cmd).Msg("invalid 'argsBefore' parameter") 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 == "" { if parameters.blendfile, ok = cmdParameter[string](cmd, "blendfile"); !ok || parameters.blendfile == "" {
logger.Warn().Interface("command", cmd).Msg("missing 'blendfile' parameter") 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 { if parameters.args, ok = cmdParameterAsStrings(cmd, "args"); !ok {
logger.Warn().Interface("command", cmd).Msg("invalid 'args' parameter") 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'. // Move any CLI args from 'exe' to 'argsBefore'.
exeArgs, err := shlex.Split(parameters.exe) exeArgs, err := shlex.Split(parameters.exe)
if err != nil { if err != nil {
logger.Warn().Err(err).Interface("command", cmd).Msg("error parsing 'exe' parameter with shlex") 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 { if len(exeArgs) > 1 {
allArgsBefore := []string{} allArgsBefore := []string{}

View File

@ -162,34 +162,34 @@ func cmdFramesToVideoParams(logger zerolog.Logger, cmd api.Command) (CreateVideo
if parameters.exe, ok = cmdParameter[string](cmd, "exe"); !ok || parameters.exe == "" { if parameters.exe, ok = cmdParameter[string](cmd, "exe"); !ok || parameters.exe == "" {
logger.Warn().Interface("command", cmd).Msg("missing 'exe' parameter") 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 { if parameters.fps, ok = cmdParameter[float64](cmd, "fps"); !ok || parameters.fps == 0.0 {
logger.Warn().Interface("command", cmd).Msg("missing 'fps' parameter") 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 == "" { if parameters.inputGlob, ok = cmdParameter[string](cmd, "inputGlob"); !ok || parameters.inputGlob == "" {
logger.Warn().Interface("command", cmd).Msg("missing 'inputGlob' parameter") 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 == "" { if parameters.outputFile, ok = cmdParameter[string](cmd, "outputFile"); !ok || parameters.outputFile == "" {
logger.Warn().Interface("command", cmd).Msg("missing 'outputFile' parameter") 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 { if parameters.argsBefore, ok = cmdParameterAsStrings(cmd, "argsBefore"); !ok {
logger.Warn().Interface("command", cmd).Msg("invalid 'argsBefore' parameter") 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 { if parameters.args, ok = cmdParameterAsStrings(cmd, "args"); !ok {
logger.Warn().Interface("command", cmd).Msg("invalid 'args' parameter") 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'. // Move any CLI args from 'exe' to 'argsBefore'.
exeArgs, err := shlex.Split(parameters.exe) exeArgs, err := shlex.Split(parameters.exe)
if err != nil { if err != nil {
logger.Warn().Err(err).Interface("command", cmd).Msg("error parsing 'exe' parameter with shlex") 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 { if len(exeArgs) > 1 {
allArgsBefore := []string{} allArgsBefore := []string{}

View File

@ -26,11 +26,11 @@ func (ce *CommandExecutor) cmdMoveDirectory(ctx context.Context, logger zerolog.
if src, ok = cmdParameter[string](cmd, "src"); !ok || src == "" { if src, ok = cmdParameter[string](cmd, "src"); !ok || src == "" {
logger.Warn().Interface("command", cmd).Msg("missing 'src' parameter") 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 == "" { if dest, ok = cmdParameter[string](cmd, "dest"); !ok || dest == "" {
logger.Warn().Interface("command", cmd).Msg("missing 'dest' parameter") 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(). logger = logger.With().
@ -39,12 +39,11 @@ func (ce *CommandExecutor) cmdMoveDirectory(ctx context.Context, logger zerolog.
Logger() Logger()
if !fileExists(src) { if !fileExists(src) {
logger.Warn().Msg("source path does not exist, not moving anything") 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) 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 { if err := ce.listener.LogProduced(ctx, taskID, msg); err != nil {
return err return err
} }
return fmt.Errorf(msg) return NewParameterInvalidError("src", cmd, "path does not exist")
} }
if fileExists(dest) { if fileExists(dest) {

View File

@ -39,7 +39,11 @@ func TestCmdMoveDirectoryNonExistentSourceDir(t *testing.T) {
f.mocks.listener.EXPECT().LogProduced(gomock.Any(), taskID, f.mocks.listener.EXPECT().LogProduced(gomock.Any(), taskID,
"move-directory: source path \"render/output/here__intermediate\" does not exist, not moving anything") "move-directory: source path \"render/output/here__intermediate\" does not exist, not moving anything")
err := f.run() err := f.run()
assert.Error(t, err) var paramErr ParameterInvalidError
if assert.ErrorAs(t, err, &paramErr) {
assert.Equal(t, "src", paramErr.Parameter)
assert.Equal(t, "path does not exist", paramErr.Message)
}
} }
func TestCmdMoveDirectoryHappy(t *testing.T) { func TestCmdMoveDirectoryHappy(t *testing.T) {

View File

@ -6,7 +6,6 @@ package worker
import ( import (
"context" "context"
"errors"
"fmt" "fmt"
"time" "time"
@ -20,7 +19,7 @@ import (
func (ce *CommandExecutor) cmdEcho(ctx context.Context, logger zerolog.Logger, taskID string, cmd api.Command) error { func (ce *CommandExecutor) cmdEcho(ctx context.Context, logger zerolog.Logger, taskID string, cmd api.Command) error {
message, ok := cmd.Parameters["message"] message, ok := cmd.Parameters["message"]
if !ok { if !ok {
return fmt.Errorf("missing 'message' setting") return NewParameterMissingError("message", cmd)
} }
messageStr := fmt.Sprintf("%v", message) 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"] sleepTime, ok := cmd.Parameters["duration_in_seconds"]
if !ok { if !ok {
return errors.New("missing setting 'duration_in_seconds'") return NewParameterMissingError("duration_in_seconds", cmd)
} }
var duration time.Duration var duration time.Duration
@ -47,7 +46,7 @@ func (ce *CommandExecutor) cmdSleep(ctx context.Context, logger zerolog.Logger,
duration = time.Duration(v) * time.Second duration = time.Duration(v) * time.Second
default: default:
log.Warn().Interface("duration_in_seconds", v).Msg("bad type for setting 'duration_in_seconds', expected int") 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") log.Info().Str("duration", duration.String()).Msg("sleep")

63
internal/worker/errors.go Normal file
View File

@ -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{}