My rule for errors is to add only information that the caller isn't aware of. So don't do:
func GetFooByID(ctx context.Context, id int) (Foo, error) {
row, err := LookupRow(ctx, "foo", id)
if err != nil {
return nil, errors.Wrapf(err, "GetFooByID id=%v", id)
}
return row, nil
}
The caller already knows the id it's looking up, and that the name of the function is GetFooByID. It might include those in its wrapping of that error, but GetFooByID shouldn't.Meanwhile, this is good:
func UpgradeFoos(ctx context.Context, tx *Tx) error {
foos, err := GetAll(ctx, tx, "foos")
if err != nil {
return errors.Wrap(err, "GetAll")
}
for _, foo := range foos {
if err := UpgradeFoo(ctx, tx, foo.ID); err != nil {
return errors.Wrapf(err, "UpgradeFoo(%v)", foo.ID)
}
}
return nil
}
Now your logs look something like "server failed to startup error=apply migration: upgrade foos: UpgradeFoo(42): i/o timeout" instead of "server failed to startup error=i/o timeout". This saves you from "hmm, maybe it's too slow to get all the foos in a batch like that, we should change that". But nope, it's actually UpgradeFoo(42) that's broken.Wrapping applied well is my favorite feature of Go. Most people don't do it. The standard library doesn't follow my rules. But if you do it this way, every error you see is so easy to debug and resolve. Less downtime, more reliability.
(As an aside, that loop where you accumulate errors can easily accumulate into a multi-error as of recent versions of Go. I always prefer to try everything possible and return all the errors, then you can fix multiple problems with your input on one go. It's also good for cases where you are doing a main operation and an ancillary operation, like closing something. People often ignore errors on "Close" and "Sync", but by joining those into a multierror, then you no longer have questions like "there were no errors, but this file isn't on disk". https://pkg.go.dev/go.uber.org/multierr#hdr-Deferred_Functio... is a really nice approach for the common case of "defer fh.Close()". I have an `errors.Close` wrapper I use: https://github.com/pachyderm/pachyderm/blob/master/src/inter.... Can't live without it!)