Wrapping Errors the Right Way
Don't include information the caller already knows in your errors.
I’ve seen a few examples of error handling like the following lately:
func MightFail(id string) error {
err := sqlStatement()
if err != nil {
return fmt.Errorf("mightFail failed with id %v because of sql: %w", id, err
}
...
return nil
}See the problem? It becomes more clear when you start to use the function:
func business(ids []string) error {
for _, id := range ids {
err := MightFail(id)
if err != nil {
return fmt.Errorf("business failed MightFail on id %v: %w", id, err)
}
}
}If we run this code, and MightFail actually does fail, we’ll get an error message like the following:
business failed MightFail on id Test: mightFail failed with id Test because of sql: <sql error>There’s a lot of repetition in this error. We’re told two times that MightFail failed, and we’re told the ID twice as well. This error would be much more readable if it looked something like this:
MightFail on id Test: sql statement: <sql error>This gets straight to the point. We have the ID, the chain of function calls that led to the issue, and the underlying failure. But how can we ensure, across a big code base, that we end up with errors like this?
Lets imagine we’re writing the MightFail function from scratch. We know we want to call sqlStatement and we know we need to handle its error, but what information should we include when we wrap it? You might think “it would be useful to have the id we called the sql statement with when debugging” which is true, however this same logic applies at the call site too. The developer writing the business function will probably also think having the id would be useful, and they’re liable to include in their wrapped error message.
Instead, only include information the caller doesn’t have. The caller is free to annotate your errors with information such as the name of your function, arguments they passed in, etc. There is no need for you to provide that information to them, as its obvious up front. If this same logic is applied consistently you end up with error messages that are high-signal and to-the-point.
Conclusion
When you wrap errors your message should include:
A pointer to where within your method the failure occurred
Values that will be useful during debugging (e.g ids)
(sometimes) details about why the error occurred
Other relevant info the caller doesnt know
And your message should NOT include:
The name of your function
Any of the arguments to your function
Any other information that is already known to the caller
What about the case when func you are calling is hidden behind an interface?
Example:
```
func read(r io.Reader) error {
_, _ := r.Read(x) // here we dont realy know what function we are calling
}
read()
```
I think the opposite approach is more suitable: function should always put information about himself into the error message.
Like this: fmt.Errorf("pkg.fn: %w", err)
Also it's how standart library doing it.
While I agree with your approach on a philosophical level, it goes against the established practices of the Go standard library (and community libraries in the style of the Go standard library). It turns out that the opposite approach — “don't repeat information that was supplied to the callee that produced the error” — seems to be equally coherent if applied consistently.
(In my experience the idiomatic Go approach does require more unwrapping and repacking of errors to avoid redundancy, but it at least avoids an impedance mismatch between user APIs and the standard library.)