Use %w to wrap errors, enable errorlint #2519
Conversation
There was a problem hiding this comment.
Thanks!
The code changes all LGTM.
As for the linter … #2512 (review) says the linter has a 100% failure rate for me. On second check, I’m afraid that’s my mistake — without --max-same-issues -1 the output stopped at the copy.go examples, but the linter did find the other ones.
As mentioned elsewhere, in general I’m worried about signing up to maintain a detailed tuned linter configuration, but, *shrug*, let’s try this.
Some code was not using it while it should (this allows a caller to better inspect the error, if a need arises). Note in a single case where we have two errors, we only make the "primary" one unwrappable (by using %w), and explicitly convert the "secondary" one to a string (which is a way to tell a code reviewer and a linter that we don't want it to be unwrappable). Found by errorlint linter. Signed-off-by: Kir Kolyshkin <[email protected]>
Consolidate the [duplicated] code that handles and wraps the errors from Close to a function, and call it twice. Keep the code which deliberately converts an error from Close to a string (rather than wrapping it) as we don't want those errors to be unwrappable (those are not "primary" errors). Signed-off-by: Kir Kolyshkin <[email protected]>
Use a minimal config to only warn about not using %w to wrap errors. Signed-off-by: Kir Kolyshkin <[email protected]>
I generally agree, the more complicated the config is the harder it is to maintain. In this case, I hope, this won't change much over time as the linter code is basically in maintenance mode for the last few years. Nevertheless, let me know if you want me to drop the last commit. |
Based on patches from #2512, but much more conservative this time.
See individual commit descriptions for details.