go-errorlint icon indicating copy to clipboard operation
go-errorlint copied to clipboard

Prevent exposing external errors in package API

Open HTechHQ opened this issue 1 year ago • 2 comments

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

HTechHQ avatar Feb 01 '24 15:02 HTechHQ

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

polyfloyd avatar Feb 02 '24 19:02 polyfloyd

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"

HTechHQ avatar Oct 03 '24 10:10 HTechHQ