cli icon indicating copy to clipboard operation
cli copied to clipboard

[FEATURE REQUEST] Improve `errors.Deduce`

Open awilliams-fastly opened this issue 2 years ago • 0 comments

The errors.Deduce function drops important error information in certain cases.

Example

func TestDeduce_FastlyError(t *testing.T) {
	fstHTTPErr := fastly.HTTPError{
		Errors: []*fastly.ErrorObject{
			{Title: "the title of the HTTPError"},
		},
		StatusCode: http.StatusTeapot,
	}

	prefix := "important detail here"
	wrapped := fmt.Errorf("%s: %w", prefix, &fstHTTPErr)

	if got := errors.Deduce(wrapped).Error(); !strings.Contains(got, prefix) {
		t.Fatalf("got: %q\nwant contain: %q", got, prefix)
	} else {
		t.Logf("got: %s", got)
	}
}

=== RUN   TestDeduce_FastlyError
    deduce_test.go:80: got: "the Fastly API returned 418 I'm a teapot: the title of the HTTPError"
        want contain: "important detail here"

Description

The Deduce function will unwrap any errors that wrap a fastly.HTTPError: https://github.com/fastly/cli/blob/92952d83c5327f4ff2d0da42ea99fdb4963088e3/pkg/errors/deduce.go#L24-L33

In some cases, like the example above and here, this can remove important error context from the output error message.

Ideas

  • I thought about creating my own RemediationError instead of using fmt.Errorf with wrapping, but that'd mean re-implementing the logic in Deduce (i.e. the Authentication error handling).
  • I think an approach would be extracting this logic into a standalone exported functions that package users can call. They could then add a prefix to the Remediation.Inner field.

awilliams-fastly avatar Apr 12 '23 17:04 awilliams-fastly