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

Prevent exposing external errors in package API #68

Open
HTechHQ opened this issue Feb 1, 2024 · 2 comments
Open

Prevent exposing external errors in package API #68

HTechHQ opened this issue Feb 1, 2024 · 2 comments

Comments

@HTechHQ
Copy link

HTechHQ commented Feb 1, 2024

The linter reports the following statement return fmt.Errorf("%w: %v", ErrInvalidValue, err) with the message non-wrapping format verb for fmt.Errorf. Use %w to format errors (errorlint)

I consider this a valid pattern, as it is preventing expose of an underlying error as part of my package API but still using the %w verb. In my view this is a false-positive of the linter.

From the Go docs: Errors and package APIs

A package which returns errors (and most do) should describe what properties of those errors programmers may rely on. A well-designed package will also avoid returning errors with properties that should not be relied upon.

...

In all cases, care should be taken not to expose internal details to the user. As we touched on in “Whether to Wrap” above, when you return an error from another package you should convert the error to a form that does not expose the underlying error, unless you are willing to commit to returning that specific error in the future.

f, err := os.Open(filename)
if err != nil {
    // The *os.PathError returned by os.Open is an internal detail.
    // To avoid exposing it to the caller, repackage it as a new
    // error with the same text. We use the %v formatting verb, since
    // %w would permit the caller to unwrap the original *os.PathError.
    return fmt.Errorf("%v", err)
}

https://go.dev/blog/go1.13-errors


Expected Behaviour
To be more in line with the official recommendation of the docs, as well as to support a very conscious choice over which errors to expose as part of the package API, I propose to:

  • Allow the pattern like fmt.Errorf("%w: %v", ErrInvalidValue, err)
  • If at least one %w verb is used, other errors can be returned with he %v verb
@polyfloyd
Copy link
Owner

This is a good suggestion! It would also be nice to have an additional lint that raises a warning if an error from another library is accidentally exposed

@HTechHQ
Copy link
Author

HTechHQ commented Oct 3, 2024

To get around this, I opted for the following configuration of golangci-lint:

issues:
  exclude-rules:
    # Prevent external errors to become part of the package API
    # At least one error needs to be wrapped, then other errors don't have to
    # See: https://github.com/polyfloyd/go-errorlint/issues/68
    - linters:
        - errorlint
      source: ".*%w.*: %v"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants