From 8eb1567b7881cc14dc53b56df3f25080a3395bc5 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Wed, 9 Oct 2024 17:21:41 -0700 Subject: [PATCH] cli-plugins/manager: simplify generating errors 1. Ditch wrapAsPluginError as we can simply use NewPluginError now. 2. Add a TODO to remove pluginError.Cause method. 3. Stop referring to pkg/errors method (except for 2 above). Signed-off-by: Kir Kolyshkin --- cli-plugins/manager/error.go | 16 ++++------------ cli-plugins/manager/error_test.go | 2 +- cli-plugins/manager/plugin.go | 8 ++++---- 3 files changed, 9 insertions(+), 17 deletions(-) diff --git a/cli-plugins/manager/error.go b/cli-plugins/manager/error.go index 2ffbf4ee7c..e8538ade3a 100644 --- a/cli-plugins/manager/error.go +++ b/cli-plugins/manager/error.go @@ -23,12 +23,13 @@ func (e *pluginError) Error() string { return e.cause.Error() } -// Cause satisfies the errors.causer interface for pluginError. +// Cause satisfies the github.com/pkg/errors.causer interface for pluginError. +// TODO: remove this once all users switch away from github.com/pkg/errors. func (e *pluginError) Cause() error { return e.cause } -// Unwrap provides compatibility for Go 1.13 error chains. +// Unwrap provides compatibility for Go 1.13+ error chains. func (e *pluginError) Unwrap() error { return e.cause } @@ -38,17 +39,8 @@ func (e *pluginError) MarshalText() (text []byte, err error) { return []byte(e.cause.Error()), nil } -// wrapAsPluginError wraps an error in a pluginError with an -// additional message, analogous to errors.Wrapf. -func wrapAsPluginError(err error, msg string) error { - if err == nil { - return nil - } - return &pluginError{cause: fmt.Errorf(msg+": %w", err)} -} - // NewPluginError creates a new pluginError, analogous to -// errors.Errorf. +// [fmt.Errorf]. func NewPluginError(msg string, args ...any) error { return &pluginError{cause: fmt.Errorf(msg, args...)} } diff --git a/cli-plugins/manager/error_test.go b/cli-plugins/manager/error_test.go index c4cb19bd55..66f257722b 100644 --- a/cli-plugins/manager/error_test.go +++ b/cli-plugins/manager/error_test.go @@ -14,7 +14,7 @@ func TestPluginError(t *testing.T) { assert.Check(t, is.Error(err, "new error")) inner := errors.New("testing") - err = wrapAsPluginError(inner, "wrapping") + err = NewPluginError("wrapping: %w", inner) assert.Check(t, is.Error(err, "wrapping: testing")) assert.Check(t, is.ErrorIs(err, inner)) diff --git a/cli-plugins/manager/plugin.go b/cli-plugins/manager/plugin.go index 2c0ff43438..16a81520fd 100644 --- a/cli-plugins/manager/plugin.go +++ b/cli-plugins/manager/plugin.go @@ -86,12 +86,12 @@ func newPlugin(c Candidate, cmds []*cobra.Command) (Plugin, error) { // We are supposed to check for relevant execute permissions here. Instead we rely on an attempt to execute. meta, err := c.Metadata() if err != nil { - p.Err = wrapAsPluginError(err, "failed to fetch metadata") + p.Err = NewPluginError("failed to fetch metadata: %w", err) return p, nil } if err := json.Unmarshal(meta, &p.Metadata); err != nil { - p.Err = wrapAsPluginError(err, "invalid metadata") + p.Err = NewPluginError("invalid metadata: %w", err) return p, nil } if p.Metadata.SchemaVersion != "0.1.0" { @@ -110,7 +110,7 @@ func newPlugin(c Candidate, cmds []*cobra.Command) (Plugin, error) { func (p *Plugin) RunHook(ctx context.Context, hookData HookPluginData) ([]byte, error) { hDataBytes, err := json.Marshal(hookData) if err != nil { - return nil, wrapAsPluginError(err, "failed to marshall hook data") + return nil, NewPluginError("failed to marshall hook data: %w", err) } pCmd := exec.CommandContext(ctx, p.Path, p.Name, HookSubcommandName, string(hDataBytes)) @@ -118,7 +118,7 @@ func (p *Plugin) RunHook(ctx context.Context, hookData HookPluginData) ([]byte, pCmd.Env = append(pCmd.Env, ReexecEnvvar+"="+os.Args[0]) hookCmdOutput, err := pCmd.Output() if err != nil { - return nil, wrapAsPluginError(err, "failed to execute plugin hook subcommand") + return nil, NewPluginError("failed to execute plugin hook subcommand: %w", err) } return hookCmdOutput, nil