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

Switch from pkg/errors to Go 1.13+ error wrapping #2290

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

kolyshkin
Copy link
Contributor

Fixes: #1614

The github.com/pkg/errors is mostly obsoleted since Go 1.13 introduced
%w-style error wrapping. It is also not maintained and is now archived
by the owner.

Some part of this change was done manually, and some changes were done
by using https://github.com/AkihiroSuda/go-wrap-to-percent-w tool.

In a few places this also:

  • changes '%s' or "%s" to %q;
  • removes extra context from the error message (such as, errors from os
    functions dealing with files do contain the file name already, and
    strconv.Atoi errors already contains the string which it failed to
    parse).

Note that there is a single place which uses StackTrace functionality of
pkg/errors, which is removed by this PR.

A few remaining users of pkg/errors vendored here (directly and
indirectly) are:

There is no need to wrap errors from os package, as they already contain
extra information such as filename and which operation has failed.

Signed-off-by: Kir Kolyshkin <[email protected]>
The github.com/pkg/errors is mostly obsoleted since Go 1.13 introduced
%w-style error wrapping. It is also not maintained and is now archived
by the owner.

Some part of this change was done manually, and some changes were done
by using github.com/AkihiroSuda/go-wrap-to-percent-w tool.

In a few places this also:
 - changes '%s' or \"%s\" to %q;
 - removes extra context from the error message (such as, errors from os
   functions dealing with files do contain the file name already, and
   strconv.Atoi errors already contains the string which it failed to
   parse).

Note that there is a single place which uses StackTrace functionality of
pkg/errors, which is removed by this commit.

A few remaining users of pkg/errors vendored here (directly and
indirectly) are:
 - github.com/containerd/go-runc (needs to be bumped to v1.1.0);
 - github.com/microsoft/didx509go (needs microsoft/didx509go#19);
 - github.com/docker/cli (needs docker/cli#3618 fixed);
 - github.com/docker/docker (?)
 - github.com/linuxkit/virtsock (needs linuxkit/virtsock#69 merged);

Signed-off-by: Kir Kolyshkin <[email protected]>
@kolyshkin kolyshkin marked this pull request as ready for review October 9, 2024 22:05
@kolyshkin kolyshkin requested a review from a team as a code owner October 9, 2024 22:05
@kolyshkin
Copy link
Contributor Author

Is this repo maintained?

@kolyshkin
Copy link
Contributor Author

@kevpar @anmaxvl @katiewasnothere PTAL

@anmaxvl anmaxvl self-assigned this Nov 6, 2024
@kolyshkin
Copy link
Contributor Author

Some of linter errors are pre-existing and some are new. I can fix them all (either here or in a separate PR) if there's enough interest.

@anmaxvl
Copy link
Contributor

anmaxvl commented Nov 7, 2024

@kolyshkin, thanks for the PR. We need to figure out what we want to do with StackTrace, since it'll be a breaking change for our LCOW (Linux containers on Windows) scenario.

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

Successfully merging this pull request may close these issues.

github.com/pkg/errors is archived
3 participants