On: Structured Errors in Go (2022)

[masklinn]

This is covered by the “ergonomics” section of TFA:

> With custom error structs however, it's a lot of writing to create your own error type and thus it becomes more of a burden to encourage your team members to do this.

Because you need a type per layer, and that type needs to implement both error and unwrap.

[atombender → masklinn]

In practice, I have found the benefit of explicit typing to outweigh the downsides of needing to declare the types.

As a concrete example, it means you can target types with precision in the API layer:

    switch e := err.(type) {
      case UserNotFound:
        writeJSONResponse(w, 404, "User not found")
      case interface { Timeout() bool }:
        if e.Timeout() {
          writeJSONResponse(w, 503, "Timeout")
        }
    }

I skimmed the article and didn't see the author proposing a way to do that with their arbitrary key/value map.

Of course, you could use something else like error codes to translate groups of errors. But then why not just use types?

But as I suggested in my other comment, you could also generalize it. For example:

    return meta.Wrap(err, "storing user", "userID", userID)

Here, Wrap() is something like:

    func Wrap(err error, msg string, kvs ...any) {
      return &KV{
        KV:    kvs,
        cause: err,
        msg:   msg,
      }
    }

This is the inverse of the context solution. The point is to provide data at the point of error, not at every call site.

You can always merge these later into a single map and pay the allocation cost there:

    var fields map[string]any
    for err != nil {
      if e, ok := err.(*KV); ok {
        for i := 0; i < len(KV.KV); i += 2 {
          fields[e.KV[i].(string)] = e.KV[i+1]
        }
      }
      err = errors.Unwrap(err)
    }

[johnmaguire → atombender]

You can also just assign your errors to variables, and typically you only need to do so for the innermost error. Wrapping calls can just use Errorf with the %w operator.

Horses for courses.

[kiitos → atombender]

err.(type) fails when errors are wrapped, which is common and needs to be accommodated. To do what you're trying to do, you need to use errors.As.

[atombender → kiitos]

Sure. It was not intended to be a complete example.

[kiitos → atombender]

It's not that the example is incomplete, it's that it's incorrect! :)

[atombender → kiitos]

No, it's just incomplete. The same code just needs to unwrap:

    for err != nil {
      switch e := err.(type) {
        case UserNotFound:
          writeJSONResponse(w, 404, "User not found")
          return
        case interface { Timeout() bool }:
          if e.Timeout() {
            writeJSONResponse(w, 503, "Timeout")
            return
          }
      }
      err = errors.Unwrap(err)
    }

[kiitos → atombender]

`err.(type)` is incorrect, at least in general. Calling `errors.Unwrap` in application code like this is almost always a red flag indicating a design error. And in this case it definitely is!

[atombender → kiitos]

That is literally what errors.As() does, finding a value up the cause chain that can be coerced to the target type.

[kiitos → atombender]

If you're re-implementing errors.As's unwrapping behavior in your application code in order to parse/evaluate errors, that's a mistake and a design error. You'd never call Unwrap outside of a custom error type's method set, and even then you'd never use a `for` loop like you're doing in your example.

> it means you can target types with precision in the API layer

The only situation where you need to get precise error types is when you need to provide specific details from those specific types to the consumer, which is rare. And even in those rare cases, user code does that work via errors.As, not this manual Unwrap loop process you're suggesting here.

[atombender → kiitos]

> The only situation where you need to get precise error types is when you need to provide specific details from those specific types to the consumer, which is rare.

It's not rare in my experience. All they apps I work on have a central unhandled error handler in the API that converts Go errors to HTTP or gRPC error responses, and then falls back to a general "internal error" if no specific error could be mapped. I can think of many other instances where we have a switch over half a dozen error typed in order to translate them into other types across RPC or pub/sub boundaries.

> And even in those rare cases, user code does that work via errors.As, not this manual Unwrap loop process you're suggesting here.

As() does not work with switch statements unless you pre-declare a ton (in our case, often a couple of dozen) error variables. Secondly, it is deeply inefficient. As() traverses the cause tree recursively for every single error, so if you have 30 possible error types to compare, and an error typically wraps 3 layers deep, that's a worst case of 30 loop iterations with 90 cases, as opposed to my method, which is 3 loops.

[kiitos → atombender]

> t is deeply inefficient. As() traverses the cause tree recursively for every single error, so if you have 30 possible error types to compare, and an error typically wraps 3 layers deep, that's a worst case of 30 loop iterations with 90 cases ...

I have no idea how you came to this conclusion. It's certainly not what happens when you call errors.As in your application code.

There's no situation where your application code would ever have 30 error types to compare against, if that were ever the case you have seriously fucked up!

[atombender → kiitos]

If you do this:

  var a, b, c error1, error2, error3
  switch {
    case errors.As(&a):
      ...
    case errors.As(&b):
      ...
    case errors.As(&c):
      ...
  }

…then yes, you will be doing 3 searches, each of which will do a loop (sometimes recursively if Unwrap() returns []error) over the chain of causes.

> There's no situation where your application code would ever have 30 error types to compare against, if that were ever the case you have seriously fucked up!

That is your opinion. In my experience, that is not the case, because there are lots of cases where you want to centrally translate a canonical set of errors into another canonical set.

> It literally is not. Is and As are not merely convenience functions, they're canonical …

This is just your opinion. If you actually read the documentation, you will see that it merely says Is() and As() are "preferable" to checking.

[kiitos → atombender]

Any code that looks like that is almost certainly broken. If the things you're describing as "my opinion" are counter-indicated in the code that you're used to working with, then you're working with code that's seriously unconventional. Do with that feedback what you will.

[atombender → kiitos]

You're wrong. You've not offered any evidence for why this is not just your opinion, and your claims are easily contradicted by examples.

As an example, say we have an API implemented on top of a complex data store. Every data store implementation can return errors like ObjectNotFound, InsufficientPermissions, and a dozen others. Every data store call can potentially return these. As well as, of course, standard Go errors like DeadlineExceeded or internal errors that cannot be exposed as user-facing API responses. However, some translation error has to translate those errors into API responses.

This cannot conveniently and consistently be done in each API handler, as it would repeat the same error translation for the same errors. An InsufficientPermissions error may happen in a "create" route as well as an in a "update" route, but also in any other route that deals with objects not being accessible.

Therefore it must be done in a central error translator. By definition. And this translation must either do a dozen+ Is() and As() calls, or it can be done efficiently, as I've described.

Anyway, I've said all I have needed to say and won't respond any further.

[kiitos → atombender]

> it must be done in a central error translator. By definition. And this translation must either do a dozen+ Is() and As() calls, or it can be done efficiently, as I've described.

These claims are, bluntly, incorrect. There are no widely-used modules that work this way, and there are no properties of the language or its conventions that would suggest that this is a viable way to design an API. errors.Is and errors.As provide capabilities that type assertions -- as you've described -- factually do not provide. They're not equivalent, they're not normally used, they're not anything other than red flags in bad code that should be eliminated.

I'm not trying to pick a fight with you, I'm honestly just trying to prevent other people, reading this comment thread, from making the kinds of design mistakes that you're describing here as viable and efficient. They truly aren't.

[atombender → kiitos]

That is just your opinion. There is absolutely nothing wrong with doing so, and there is nothing in the documentation that asserts what you claim.

The documentation is clear that comparing an error value or casting it without following the Unwrap() chain is only an antipattern because it would not work with wrapped errors.

Is() and As() are merely convenience functions, and the documentation is clear that all they're doing is calling Unwrap(), which you can do yourself.

[kiitos → atombender]

It literally is not. Is and As are not merely convenience functions, they're canonical and conventional expectations of Go code, if you're doing that work yourself you're almost certainly doing it wrong.