Pipeline: Start using error wrapping

[bobcatfish]

Expected Behavior

When we have layers of function calls that can each return errors, e.g. A->B->C where C returns an error, B needs to return it and then A needs to return that, we should use B and A to add context to that error, but retain the original error from C, e.g.:

_, err := ioutil.ReadAll(r)
if err != nil {
        return errors.Wrap(err, "read failed")
}

Actual Behavior

Throughout our codebase, we have many instances of errors being "wrapped" via string formatting, e.g.:

https://github.com/tektoncd/pipeline/blob/11e03b73e549ab1508827182f6cc05ccf1a3080c/pkg/reconciler/v1alpha1/pipelinerun/resources/pipelinerunresolution.go#L109-L112

Instead of actually wrapping the error, we turn it into a string. And sometimes we can have 3+ levels of nesting within error messages like this!

Additional Info

https://github.com/pkg/errors is a solid candidate; we should use this unless we have a strong reason to use something else.

A couple ways that completing this issue could look:

• Replace all error instances with wrapped errors (probably too tedious and time consuming to be feasible)
• (More likely) add the dependency + replace a few instances; we can use this as a precedent going forward

[chmouel]

👍 on dave chenney's pkg/errors package!

It would be nice tho to have a linter run in CI that supports that check! I looked around and i had some hope for errcheck but it doesn't seem to be supported there or in other golinters 😿

[u5surf]

@bobcatfish I'd like to try this issue.

[bobcatfish]

Quick update: we ended up using https://godoc.org/golang.org/x/xerrors instead of https://github.com/pkg/errors b/c @ImJasonH pointed out that xerrors will be moving into the Go std lib in 1.13 :D

xerrors.Errorf lets the user use Errorf style syntax, but:

If the last argument is an error and the format string ends with ": %w", the returned error implements Wrapper with an Unwrap method returning it.

Which gives us the same benefits as errors.Wrap.

(I'm mostly writing this b/c I couldn't understand why we were using xerrors.Errorf and I had to do some investigation before I understood that it was also doing error wrapping XD)

[bobcatfish]

Thanks @u5surf !! :tada: :heart: :tada: