Alternatives to wrapping errors

[Jonathan Hall]

I was recently catching up on the latest state of the github.com/pkg/errors package (https://github.com/pkg/errors/issues/245), when I noticed that Dave Cheney said:

> I no longer use this package, in fact I no longer wrap errors.

I'm curious what approach Dave uses now, but unless/until he writes on the topic, I'm also curious what other approaches people use for clean and cohesive error handling and reporting.

If you don't wrap errors, how do you ensure meaningful error handling and reporting in your application?

Thanks,
Jonathan

[Kevin Chowski]

Have you seen https://go.dev/blog/go1.13-errors?

If one wants to use error unwrapping, these days generally it is suggested to use the functionality from the standard package "errors" (or fmt.Errorf) rather than some third-party package. That way everyone does it the same way, making third-party packages more composable by default.

I personally wrap errors with fmt.Errorf and the %w verb when I want to add additional context to some error I received, and the root cause of why my function failed is *precisely* the same as the 'error' value I received. This is less often than I used to think, and it is definitely a maintenance risk when this error comes from another package: if that package changes what error type it returns, that may break some other part of my code. (that risk is perhaps one reason why folks choose not to wrap errors by default, though note that errors from Go's stdlib are less likely to change over time than some random third-party library.)

In general, I prefer to wrap errors that come from a package that I own (again assuming the root cause is exactly the same AND I want to add some annotation), and prefer not to wrap errors that come from a package owned by someone else. But neither are hard rules for code I write or review.

One last thought is: if you never unwrap an error, there are fewer good reasons to ever wrap them. If you want to unwrap them, then it obviously starts to make sense to wrap some :)

[Jonathan Hall]

Yes, of course I'm quite familiar with Go 1.13's error capabilities.  github.com/pkg/errors is still useful when one wants to attach stack traces to errors.

But this leaves my question open:  What are alternatives to wrapping errors? (Other than passing around errors with no semantic meaning, as was the case before wrapping was widespread)

[Brian Candler]

ISTM the alternative is return a semantically-meaningful error at the current level of execution.

e.g. if you're trying to open a config file and it fails, you return "Unable to open config file"; or better, include the text version of the underlying error: e.g. "Unable to open config file: 'foo': Permission denied".

This is different from just returning the underlying error, and also different from returning a wrapped error where the caller can explicitly extract and match the type/value of the underlying error.

Wrapping invites people to couple to the underlying error types.  Having said that: if you don't wrap, and someone really wants to branch based on the underlying condition, they will match on the text content of the message (which is worse IMO).

[mineg...@gmail.com]

In practice, wrapping errors is handy for interfaces that have predefined generic error types that will end up being logged. In code, you'd check the error against that constant error with errors.Is(err, gateway.InvalidUser) and handle it the same way, but when you log that error the extra messaging will assist in debugging the problem.

Imagine you have a gateway that provides details about users, and you have different implementations which read from redis and one that reads from the database. You code would call CheckUser() and the implementation would first check redis, see it is missing, then check your database, and still see it is missing. The redis layer would pick up on that InvalidUser error, cache it, and return the unmodified database error. The calling code would do a simple errors.Is(err, gateway.InvalidUser), log the error message, and continue processing how it needs to. The next time that user is check again, it hits redis and see that it is an invalid user. Redis returns a wrapped InvalidUser error stating that it knows it is an invalid user at the cache layer, maybe including the time it was stored. The calling code checks errors.Is(err, gateway.InvalidUser), logs, and continues.

But at that same time, the user was created in the database but the cache wasn't invalidated. That user fails to log into the system, and reports a bug. The developer investigates the log, sees that first database InvalidUser log message, followed by a bunch of redis InvalidUser log lines, intermixed with a log message saying the user was created. They investigate the user creation code and notice it doesn't invalid the cache. They update the code, flush the redis cache for invalid users, and resolved a tricky caching issue that could have taken them a long time to debug since their recreation steps may not have been to try to first login as an invalid user before creating the user.

As a toy example, check out: https://go.dev/play/p/7RWsj0HW1xR

[Brian Candler]

In your code, the cache layer always returns a non-nil error, even when the value was found in the cache *and* the value was error-free.  I don't think that's realistic.  You're changing "error" from an indication of a problem, into auxiliary metadata about the value being returned (e.g. "this value came from the cache").  It means the consumer can no longer use "err != nil" to detect a problem, which is the most basic contract for error.

This can be fixed:

https://go.dev/play/p/b_xLb06goXH

But then you lose the provenance information for for error-free data (i.e. did it come from the cache or not?).  And I still don't like the idea that a *real* error that happened can sometimes be wrapped in a cache error and sometimes not. I think that's because the cache error isn't really a caching error (the cache retrieved the value successfully!), but an auxiliary indication that this error value was retrieved via the cache.  The error itself is really the same.

The point of this thread was whether wrapping is necessary, or what the alternative approaches are.  I would suggest here that you can avoid wrapping by following the way fs.PathError works, which is a typed error rather than a singleton:

https://go.dev/play/p/a3cc4Watwie

That still doesn't give you the metadata "this [valid/error] value came from cache".  I believe that information belongs in a separate channel from error.  For example, you could return a UserData struct which contains the data you asked for, with a separate field for its provenance - or else return a completely separate provenance value (not part of the error).

[Henry]

The purpose of error wrapping is to give contextual information about the error. It is supposed to help you understand and locate the error. For instance, "invalid filename" is not particularly useful when compared to "fail to load customer data: invalid filename". Error wrapping is a stack trace in a human-friendly language. It is not a very efficient way to trace errors, but it explains to you like a good friend about what went wrong.

The problem with error wrapping is that it is difficult to write good error messages. I have seen wrapped error messages that are very wordy and yet don't add much information beyond the original error messages. 

A stack trace is a superior alternative to error wrapping. It allows you to accurately trace the error and doesn't require much effort on your part beyond writing the original error messages.  

[Brian Candler]

On Friday, 31 December 2021 at 08:10:49 UTC Henry wrote:

The purpose of error wrapping is to give contextual information about the error. It is supposed to help you understand and locate the error. For instance, "invalid filename" is not particularly useful when compared to "fail to load customer data: invalid filename". Error wrapping is a stack trace in a human-friendly language.

I disagree with that explanation, because what you describe can be done without wrapping:

return fmt.Errorf("Failed to load customer data: %v", err)

or with wrapping:

return fmt.Errorf("Failed to load customer data: %w", err)

Both provide exactly the same human-friendly trace.  The difference is that with the wrapped error, the error value can be destructured to retrieve the "cause" error object, and hence to match on that object. It's an invitation to your API consumer to couple directly to error types which your code doesn't return, but which originate from elsewhere, typically system libraries or third-party libraries.

The question then is, under what circumstances is that a good idea?  It depends.  Perhaps one example is if your code is using an underlying database API, and you are happy to expose raw database-level errors.  That allows your own library to wash its hands of responsibility of how to deal with these, and let the consumer deal with them if it wants to.  (Was it a temporary connection error? Was it a uniqueness constraint violation?  Not My Problem™)

[David Finkel]

There's a good argument to be made against wrapping errors at api-boundaries in some cases. IMO, that's something that's definitely case-by-case as there are situations where bubbling up the underlying error is 100% the right thing to do.

e.g. io/fs.FS implementations are constrained to returning the io/fs.PathError type by the interface documentation in some cases. However, there are specific underlying sentinel values that should be bubbled up and accessible.

IMO, within a package (or somewhat intentionally nebulously: "between API boundaries"), it's generally a good idea to use error wrapping so outer layers can make the actual wrapping, unwrapping, switching decisions.

By using error-wrapping, you're free to break functions up and add context as it bubbles up, without losing the ability to switch on the original error. (or at least use errors.Is or errors.As)

As an example of a the above trade-off, here's an io/fs.FS implementation I pushed up recently: https://gitlab.com/dfinkel/go-safefs/-/blob/ca532e3c5c5c/fs_linux.go#L78-105

It's switching on errno values and wrapping specific sentinel errors with context for the errno values I knew I could see, with specific diagnostics for those errors.

Worth noting: stack traces rarely have argument values, and don't generally have other context information, e.g. EMFILE handling specifically checks the rlimit to get the fd limit value to include in the error message. Stack traces are nice for the programmer if they've opened the right version of the code, but useless for everyone else. Errors with context are orders of magnitude more useful if written correctly. (one of the reasons I don't like exceptions is that most of the time you just get the stack trace and original error, which is only barely useful for debugging if you have the most trivial bugs)

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/fa907dec-1dc7-4909-9743-4d91eb146481n%40googlegroups.com.