Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE REQUEST] Improve errors.Deduce #906

Open
awilliams-fastly opened this issue Apr 12, 2023 · 0 comments
Open

[FEATURE REQUEST] Improve errors.Deduce #906

awilliams-fastly opened this issue Apr 12, 2023 · 0 comments

Comments

@awilliams-fastly
Copy link
Collaborator

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:

cli/pkg/errors/deduce.go

Lines 24 to 33 in 92952d8

var httpError *fastly.HTTPError
if errors.As(err, &httpError) {
remediation := BugRemediation
if httpError.StatusCode == http.StatusUnauthorized {
remediation = AuthRemediation
}
return RemediationError{Inner: SimplifyFastlyError(*httpError), Remediation: remediation}
}

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant