From da4b6275ba3065f45e66e457b336236ba9b350c9 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 20 Feb 2025 18:10:36 +0100 Subject: [PATCH] explicitly handle errors when wrapping them The errors.Wrap and errors.Wrapf functions gracefully handle nil-errors. This allows them to be used unconditionally regardless if an error was produced. While this can be convenient, it can also be err-prone, as replacing these with stdlib errors means they unconditionally produce an error. This patch replaces code uses of errors.Wrap to be gated by a check for nil-errors to future-proof our code. Signed-off-by: Sebastiaan van Stijn --- cli/command/formatter/formatter.go | 4 ++-- cli/command/system/version.go | 6 ++++-- cli/compose/interpolation/interpolation.go | 5 ++++- cli/registry/client/client.go | 10 ++++++++-- opts/gpus.go | 5 ++++- 5 files changed, 22 insertions(+), 8 deletions(-) diff --git a/cli/command/formatter/formatter.go b/cli/command/formatter/formatter.go index 5873cce8d4..a077138996 100644 --- a/cli/command/formatter/formatter.go +++ b/cli/command/formatter/formatter.go @@ -76,9 +76,9 @@ func (c *Context) preFormat() { func (c *Context) parseFormat() (*template.Template, error) { tmpl, err := templates.Parse(c.finalFormat) if err != nil { - return tmpl, errors.Wrap(err, "template parsing error") + return nil, errors.Wrap(err, "template parsing error") } - return tmpl, err + return tmpl, nil } func (c *Context) postFormat(tmpl *template.Template, subContext SubContext) { diff --git a/cli/command/system/version.go b/cli/command/system/version.go index f25446b06d..0e0422bd40 100644 --- a/cli/command/system/version.go +++ b/cli/command/system/version.go @@ -210,8 +210,10 @@ func newVersionTemplate(templateFormat string) (*template.Template, error) { } tmpl := templates.New("version").Funcs(template.FuncMap{"getDetailsOrder": getDetailsOrder}) tmpl, err := tmpl.Parse(templateFormat) - - return tmpl, errors.Wrap(err, "template parsing error") + if err != nil { + return nil, errors.Wrap(err, "template parsing error") + } + return tmpl, nil } func getDetailsOrder(v types.ComponentVersion) []string { diff --git a/cli/compose/interpolation/interpolation.go b/cli/compose/interpolation/interpolation.go index ee11656f60..a8c4edbd28 100644 --- a/cli/compose/interpolation/interpolation.go +++ b/cli/compose/interpolation/interpolation.go @@ -67,7 +67,10 @@ func recursiveInterpolate(value any, path Path, opts Options) (any, error) { return newValue, nil } casted, err := caster(newValue) - return casted, newPathError(path, errors.Wrap(err, "failed to cast to expected type")) + if err != nil { + return casted, newPathError(path, errors.Wrap(err, "failed to cast to expected type")) + } + return casted, nil case map[string]any: out := map[string]any{} diff --git a/cli/registry/client/client.go b/cli/registry/client/client.go index bbc7f4c584..6f15659bc6 100644 --- a/cli/registry/client/client.go +++ b/cli/registry/client/client.go @@ -121,7 +121,10 @@ func (c *client) PutManifest(ctx context.Context, ref reference.Named, manifest } dgst, err := manifestService.Put(ctx, manifest, opts...) - return dgst, errors.Wrapf(err, "failed to put manifest %s", ref) + if err != nil { + return dgst, errors.Wrapf(err, "failed to put manifest %s", ref) + } + return dgst, nil } func (c *client) getRepositoryForReference(ctx context.Context, ref reference.Named, repoEndpoint repositoryEndpoint) (distribution.Repository, error) { @@ -157,7 +160,10 @@ func (c *client) getHTTPTransportForRepoEndpoint(ctx context.Context, repoEndpoi c.userAgent, repoEndpoint.actions, ) - return httpTransport, errors.Wrap(err, "failed to configure transport") + if err != nil { + return nil, errors.Wrap(err, "failed to configure transport") + } + return httpTransport, nil } // GetManifest returns an ImageManifest for the reference diff --git a/opts/gpus.go b/opts/gpus.go index 993f6da9ae..492a190837 100644 --- a/opts/gpus.go +++ b/opts/gpus.go @@ -20,7 +20,10 @@ func parseCount(s string) (int, error) { return -1, nil } i, err := strconv.Atoi(s) - return i, errors.Wrap(err, "count must be an integer") + if err != nil { + return 0, errors.Wrap(err, "count must be an integer") + } + return i, nil } // Set a new mount value