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

builtin: deprecate errwrap.Wrapf() throughout #11430

Merged
merged 20 commits into from
Apr 22, 2021

Conversation

alrs
Copy link
Contributor

@alrs alrs commented Apr 21, 2021

github.com/hashicorp/errwrap has deprecated Wrapf(), and recommends using fmt.Errorf().

fmt.Errorf() added support for error wrapping in go1.13. Vault has a hard requirement of go1.16.2, so this isn't an issue.

This PR deprecates errwrap.Wrapf throughout github.com/hashicorp/vault/builtin and all of its subpackages. I organized the PR as one-commit-per-package to aid anyone who should ever need to do a git bisect, but I'm just as happy to squash this into a single commit.

@vercel vercel bot temporarily deployed to Preview – vault April 21, 2021 15:11 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook April 21, 2021 15:11 Inactive
@ncabatoff
Copy link
Collaborator

Hey @alrs,

This is great, thanks! I'm going to have to do some investigation / due diligence to reassure myself that this won't break anything, but I hope to merge this soon.

@alrs
Copy link
Contributor Author

alrs commented Apr 21, 2021

I'm going to have to do some investigation / due diligence to reassure myself that this won't break anything

Makes sense to me, thanks for taking a look at this instead of letting it sink to the bottom of the tank. :).

Copy link
Collaborator

@ncabatoff ncabatoff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't find a good example of a changed line in this PR that was being consumed by other code which was testing for an embedded error. So to reassure myself I modified reloadutil.Reload to use fmt.Errorf, and checked that TestReload_KeyWithPassphrase still worked. Since all the "query" errwrap func are built on errwrap.Walk, I don't feel the need to check each of them. And of course there were already tests in hashicorp/errwrap#9, I just wanted to be doubly sure.

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.

2 participants