3 ms·

`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

by kiitos 1y ago

`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 1y ago
That is literally what errors.As() does, finding a value up the cause chain that can be coerced to the target type.
- kiitos 1y ago
  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 1y ago
    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 1y ago
      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.
  - atombender 1y ago
    > 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 1y ago
      > 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 1y ago
        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.

by kiitos 1y ago

`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 1y ago

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

kiitos 1y ago
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 1y ago
  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 1y ago
    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.
- atombender 1y ago
  > 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 1y ago
    > 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 1y ago
      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.