If in doubt prefer to wrap errors with %v, not %w?

[Adrian Ratnapala]

Part of the culture of Go is that we are careful to promise as little
as possible in APIs, as we will be stuck with those promises long into
the future.

Now with Go 1.13 we can do `fmt.Errorf("It broken: %w")` which means
the same thing as `fmt.Errorf("It broken: %v")` except callers get
better ways to inspect the result. But as the Go blog says, this
means the use of "%w" becomes a way to expose an API:

> In other words, wrapping an error makes that error part of your API. If you don't want to commit to supporting that error as part of your API in the future, you shouldn't wrap the error.

Given the preference for *not* introducing APIs, doesn't that mean
authors should stick to "%v" until they have clear reasons for using
"%w". After all, it's always possible to switch to "%w" later.


--
Adrian Ratnapala

[Marcin Romaszewicz]

In my opinion, this is a much nicer errors package than Go's library, and I've been using it everywhere:

https://github.com/pkg/errors

Instead of fmt.Errorf("%w"), you do errors.Wrap(err, "message"), and errors.Unwrap(...) where you want to inspect the error. It's much more explicit and less error prone.

-- Marcin

--
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/CAN%2BHj7jgoMSoyTpcOL%3Da2Rd51MvO%2Bgp0XRzTHjtNZcqPdK8zOg%40mail.gmail.com.

[Axel Wagner]

Yes, IMO the decision to expose errors should in general be deliberate. However, there are cases where it makes sense to wrap from the get-go. Two examples:

• A function takes an io.Reader to parse the stream into some data structure (think a json-parser). It obviously uses no other input or abstraction or state. It makes sense to wrap the error - this allows you to provide structured extra information (like offsets) to the user of the library and better error messages, while maintaining your users ability to switch on what actually went wrong.

• OTOH, you might have a library providing abstracted access to files - for example, a configuration library. In this case, it might make sense to not wrap the error (until it's proven necessary). That way, you are free to swap the backend - maybe the config format changes, or data is loaded from the environment, or some cloud storage. The user doesn't directly know what the backend is or will be and wrapping errors might cause them to depend on implementation details that you want to stay flexible on.

The difference between these, I think, is that in second case, you know the concrete error types and don't want to expose them to the user whereas in the first case, your *user* is the one who knows the concrete error types and you shouldn't mess with them.

Note that this distinction was already possible before the new error-wrapping, by the way. For example, os.PathError already wrapped errors to expose the underlying source, while annotating it with more details. So, the decision of whether or not to expose wrapped errors has not actually changed that much. What *has* changed, is that a) you now have a more convenient middle-ground - if you *want* to expose the underlying error, but you don't really want to annotate it with structured information, you now have a convenient way to just write that down without needing to declare an extra type. And b) it provides a standard way to introspect errors, to save users from having to type out the entire chain of type-assertions. Basically, it made the idea of wrapping more usable, for the cases where it does make sense.

That latter part, BTW, is IMO a blessing and a curse. It's a blessing, because it can reduce coupling from details in the wrapping case. If foo wraps a bar.Error which wraps a baz.Error, and then decides that it doesn't need to use bar and starts directly wrapping a baz.Error, the new way will just continue to work. In the old way, the type-assertion to a bar.Error had to be typed out and will now fail. So, by adding this standardized wrapping and convenience wrappers to walk the chain, implementation details of foo can remain hidden.

It's also a curse though, because while error-wrapping now provides a way of subtyping (that's what makes this convenience work in the first place), this type-system is not checked at compile-time. So if you *where* checking for bar.Error (and handling that) before, after the switch your code now breaks, because your error handlers don't trigger anymore - but the compiler doesn't care.

Sorry if this message is a bit chaotic and stream-of-consciousnes-y :) I haven't quite figured out how to talk about all of this yet :)

I guess the tl;dr is, that I tend to agree - don't just use %w without thinking. Make a deliberate choice if you want to commit to this API detail. And as usually in API design: If in doubt, prefer to start out with less commitment, flexibility and surface.

[Adrian Ratnapala]

On Mon, 23 Mar 2020 at 11:22, Axel Wagner <axel.wa...@googlemail.com> wrote:

> • A function takes an io.Reader to parse the stream into some data structure (think a json-parser). It obviously uses no other input or abstraction or state. It makes sense to wrap the error - this allows you to provide structured extra information (like offsets) to the user of the library and better error messages, while maintaining your users ability to switch on what actually went wrong.

That's a good point, and I suppose it generalises. Whenever you
receive an interace from client code, then you have no
compiler-enforced way to predict which errors you'll get from calls
to that interface; but your client might. Therefore you should either
pass the error up directly or else user wrapping.

(Microsoft has/had the problem that many Windows APIs let clients
implement abstract classes for OS middleware to call. The MSDN page
for each method would list which error codes were allowed for the
method, but they had no way to enforce this, so the middleware had to
try and handle all possible errors).

> That latter part, BTW, is IMO a blessing and a curse. It's a blessing, because it can reduce coupling from details in the wrapping case. ...

I think this is the classic dynamic vs. static typing trade-off. Go
has always been a mostly static language but happy to sprinkle in
dynamic typing stategically through interfaces and reflection. By
standardising on the `error` interface, Go chose error handling as one
of those dynamic sprinkles.

> Sorry if this message is a bit chaotic and stream-of-consciousnes-y :) I haven't quite figured out how to talk about all of this yet :)
> I guess the tl;dr is, that I tend to agree - don't just use %w without thinking. Make a deliberate choice if you want to commit to this API detail. And as usually in API design: If in doubt, prefer to start out with less commitment, flexibility and surface.

So to turn this discussion into something actionable: should this
advice be added into the documentation? And if so, where (in package
fmt or errors)?

Lots of programers will assume that because %w is new, then it must be
the Go team's recommended "best practice". I think I was unconciously
thinking that until I read the blog post.



--
Adrian Ratnapala

[Axel Wagner]

On Mon, Mar 23, 2020 at 10:28 AM Adrian Ratnapala <adrian.r...@gmail.com> wrote:

I think this is the classic dynamic vs. static typing trade-off.

I don't fully agree here - I think it's perfectly possible to have all of the flexibility and all of the static type-checking. I'm having trouble putting it into words exactly, but I think both Java's subtype based checked exceptions and Haskell's HM type inference are approaches to achieve that. Of course, both have their own downsides, so you can't fully escape the tradeoff. But personally, I still kind of feel that it would've been possible to add variance to `func()` and `interface` type constructors and get a pretty decent way to solve these problems with static type checking. But it's not actually trivial, so I won't complain about it :)

So to turn this discussion into something actionable: should this
advice be added into the documentation?  And if so, where (in package
fmt or errors)?

I don't think there is broad concensus on this in the community. At least that was my impression whenever I talked about it with people at conferences and meetups. Even worse, I don't even think there is a phrasing of this advice that is both sufficiently broad to be applicable and sufficiently specific to be agreeable *to myself*. Like, there is a reason I chose to mention examples, instead of coming up with a rule. To me, at least, API design is hard and kind of an art form. I sort of scoff at sage advice about it, because as far as I can tell, all of it has exceptions and sometimes it feels that it has more exceptions than regular cases :)

Without consensus, I don't think there will be more "official" advice than what you've already mentioned in your original message :) And even that is pretty clear:

> There is no single answer to this question; it depends on the context in which the new error is created.