diff --git a/changes/43721-clean-up-gitops-errors b/changes/43721-clean-up-gitops-errors new file mode 100644 index 0000000000..f989896ecc --- /dev/null +++ b/changes/43721-clean-up-gitops-errors @@ -0,0 +1 @@ +- Improved errors returned from API when running fleetctl commands by dropping path / status code. diff --git a/cmd/fleetctl/fleetctl/clean_error.go b/cmd/fleetctl/fleetctl/clean_error.go new file mode 100644 index 0000000000..654c95c4d0 --- /dev/null +++ b/cmd/fleetctl/fleetctl/clean_error.go @@ -0,0 +1,38 @@ +package fleetctl + +import ( + "errors" + "regexp" + + "github.com/fleetdm/fleet/v4/client" +) + +// statusCodeErrPrefixRE matches the " received status " +// prefix that BaseClient.ParseResponse adds when wrapping a *StatusCodeErr. +// VERB is uppercase (GET, POST, PATCH, ...) and path starts with "/". +var statusCodeErrPrefixRE = regexp.MustCompile(`[A-Z]+ /\S* received status \d+ `) + +// CleanStatusCodeErr returns a copy of err suitable for display in fleetctl +// CLI output. If the chain contains a *client.StatusCodeErr, the verbose +// "VERB /path received status N " prefix added by the HTTP client is stripped +// so users see only the server-provided reason. If no StatusCodeErr is in the +// chain, err is returned unchanged. +// +// The returned value is for printing only — it does not preserve the original +// error chain, so callers that need to inspect err with errors.Is / errors.As +// should do so against the original error before passing it here. +func CleanStatusCodeErr(err error) error { + if err == nil { + return nil + } + var sce *client.StatusCodeErr + if !errors.As(err, &sce) { + return err + } + // Strip the first instance of the status code error prefix from the message, if present. + msg := err.Error() + if loc := statusCodeErrPrefixRE.FindStringIndex(msg); loc != nil { + msg = msg[:loc[0]] + msg[loc[1]:] + } + return errors.New(msg) +} diff --git a/cmd/fleetctl/fleetctl/clean_error_test.go b/cmd/fleetctl/fleetctl/clean_error_test.go new file mode 100644 index 0000000000..42542d3763 --- /dev/null +++ b/cmd/fleetctl/fleetctl/clean_error_test.go @@ -0,0 +1,37 @@ +package fleetctl + +import ( + "errors" + "fmt" + "testing" + + "github.com/fleetdm/fleet/v4/client" + "github.com/stretchr/testify/require" +) + +func TestCleanStatusCodeErr(t *testing.T) { + sce := &client.StatusCodeErr{Code: 422, Body: "Validation Failed: name may not be empty"} + wrapped := fmt.Errorf("PATCH /api/latest/fleet/config received status %w", sce) + outer := fmt.Errorf("applying fleet config: %w", wrapped) + + cases := []struct { + name string + in error + want string + }{ + {"nil", nil, ""}, + {"plain error untouched", errors.New("boom"), "boom"}, + {"bare wrapped status code err", wrapped, "Validation Failed: name may not be empty"}, + {"outer-wrapped status code err", outer, "applying fleet config: Validation Failed: name may not be empty"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := CleanStatusCodeErr(tc.in) + if tc.in == nil { + require.NoError(t, got) + return + } + require.Equal(t, tc.want, got.Error()) + }) + } +} diff --git a/cmd/fleetctl/main.go b/cmd/fleetctl/main.go index e2d1f1d398..2e47be997d 100644 --- a/cmd/fleetctl/main.go +++ b/cmd/fleetctl/main.go @@ -23,7 +23,7 @@ func main() { app := fleetctl.CreateApp(os.Stdin, os.Stdout, os.Stderr, exitErrHandler) fleetctl.StashRawArgs(app, os.Args) if err := app.Run(os.Args); err != nil { - fmt.Fprintf(os.Stdout, "Error: %+v\n", err) + fmt.Fprintf(os.Stdout, "Error: %+v\n", fleetctl.CleanStatusCodeErr(err)) os.Exit(1) } } @@ -34,7 +34,7 @@ func exitErrHandler(c *cli.Context, err error) { return } - fmt.Fprintf(c.App.ErrWriter, "Error: %+v\n", err) + fmt.Fprintf(c.App.ErrWriter, "Error: %+v\n", fleetctl.CleanStatusCodeErr(err)) if errors.Is(err, fs.ErrPermission) { switch runtime.GOOS {