From 8b779110a82b5643b6c2bc957461da4f8bc268b4 Mon Sep 17 00:00:00 2001 From: Scott Gress Date: Mon, 11 May 2026 11:48:52 -0700 Subject: [PATCH] Remove verb and url from gitops errors (#44555) **Related issue:** Resolves #43721 # Checklist for submitter If some of the following don't apply, delete the relevant line. - [X] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files) for more information. ## Testing - [X] Added/updated automated tests - [X] QA'd all new/changed functionality manually Added a Google Calendar integration to gitops .yml with `client_email` missing from the `api_json_key`. - [X] on main, got error: ``` Error: applying fleet config: PATCH /api/latest/fleet/config received status 422 Validation Failed: client_email is required (API time: 13ms) ``` - [X] on this branch, got: ``` Error: applying fleet config: Validation Failed: client_email is required (API time: 134ms) ``` ## Summary by CodeRabbit * **Bug Fixes** * Cleaner CLI error messages: removed extraneous HTTP path/status-code details from GitOps-related errors, making output easier to read. * **Tests** * Added tests to verify the improved error message handling and nil/non-wrapped error behavior. [![Review Change Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/fleetdm/fleet/pull/44555) --- changes/43721-clean-up-gitops-errors | 1 + cmd/fleetctl/fleetctl/clean_error.go | 38 +++++++++++++++++++++++ cmd/fleetctl/fleetctl/clean_error_test.go | 37 ++++++++++++++++++++++ cmd/fleetctl/main.go | 4 +-- 4 files changed, 78 insertions(+), 2 deletions(-) create mode 100644 changes/43721-clean-up-gitops-errors create mode 100644 cmd/fleetctl/fleetctl/clean_error.go create mode 100644 cmd/fleetctl/fleetctl/clean_error_test.go 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 {