Remove uses of github.com/pkg/errors
- Dominant language
- Go
- Stars
- 12.7k
- Forks
- 5k
- Avg merge
- 2d 15m
- Merged PRs (30d)
- 385
Description
The package is no longer maintained or recommended. It does not interact seamlessly with the error wrapping in the Go standard library.
Quoting a comment in a PR that originally proposed allowing use of the package again: https://github.com/elastic/beats/pull/31683#discussion_r877586230
> I think this should stay. It was a nice experiment when it was introduced, but it has fairly significant performance impacts since every wrap calls runtime.Callers. It also incorrectly handled the printing of stacks (from the [docs](https://pkg.go.dev/runtime#Callers), "Iterating over the returned slice of PCs directly is discouraged, as is using FuncForPC on any of the returned PCs, since these cannot account for inlining or return program counter adjustment.").
>
> The issue with compatibility is a subtle one; use of pkg/errrors is essentially viral if there is a use of the errors.Cause function anywhere in the codebase. This happens because stdlib-wrapped errors wrap with an error implementing the Unwrap() error method while errors.Cause [only looks for the Cause() error method](https://github.com/pkg/errors/blob/5dd12d0cfe7f152f80558d591504ce685299311e/errors.go#L275-L288). So stdlib-wrapped errors will be missed by errors.Cause calls. This can be avoided by banning the use of that function since pkg/errors do provide the stdlib unwrap.
>
> Not for this PR, but given the complexity of cleaning this out, I would suggest making an internal hard fork/compatibility layer that provides just the wrapping functionality. This gives a path forward to complete removal since we can then make an experiment that checks for non-nil errors being passed in (in dev) to find cases that are not guarded by nil checks. This compatibility layer would also be able to silently (or experimentally noisily) add a shim between Cause() error and the stdlib wrapping.
>
> It's worth noting that Dave has said that he does not use this anymore, which is partly why it is now archived.
We can make the following changes to facilitate moving away from github.com/pkg/errors:
- [x] Remove all uses of `errors.Cause` in the beats code. There are not that many of them so this is reasonable to do.
- [ ] Write a compatibility layer for `github.com/pkg/errors` that passes through to the standard library but returns `nil` when `nil` is passed to `errors.Wrap` to preserve the existing behaviour. Automated replacement of `github.com/pkg/errors` failed because of frequent use of this feature of the `Wrap` implementation. See the many failing tests in https://github.com/elastic/beats/pull/31622 for example.
Contributor guide
Assessment
This issue has not been assessed yet.