Error-checking with errors.As() is brittle with regards to plain vs pointer types

[cpu...@gmail.com]

Consider https://go.dev/play/p/jgPMwLRRsqe:

errors.As(err, <plain type>) will not match if error is of pointer type. As a result, a library consumer needs to understand if a library returns Error or *Error. However, that is not part of the API spec as both returns would satisfy error if Error() is implemented on the plain type.

A potential workaround would be using Error.As(any) to match plain/pointer type as part of the library. However, it seems counterintuitive having to do so if errors.As() doesn't by default.

Would it make sense (and I would like to propose) to expand errors.As() to match plain receiver types even when err is a pointer to the same underlying plain type.

What do you think?

Cheers,

Andi

[Tamás Gulácsi]

plain struct error is brittle in other ways, too: you can shoot yourself on foot with "err != nil" check.
So: error should be a pointer type.

[Andreas Götz]

That is a *very* good point. It still seems more obvious that one would check for error instead of pointer to error. If we don't want to do this as part of errors.As (or cannot) it might make sense for golangci-lint. I'm seeing that plain errors is a quite popular pattern?

[Sean Liao]

The documented api includes

> An error matches target if the error's concrete value is assignable to the value pointed to by target

*T is not assignable to T

- sean

--
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/9c5fe0fb-de0c-4c09-b884-2f8153348fb7n%40googlegroups.com.

[Martin Schnabel]

Sorry, to barge in but this specific thing is not really an issue as
long as you use the error type as result type. because any value with an
Error() string method does implement the error interface. and even an
empty struct or a nil pointer to a applicable type will do just fine an
matches err != nil

have fun

> --
> 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

> <mailto:golang-nuts...@googlegroups.com>.

> To view this discussion on the web visit

> https://groups.google.com/d/msgid/golang-nuts/b9d94e2f-860e-44eb-9f9e-efcd0a82b7d2n%40googlegroups.com
> <https://groups.google.com/d/msgid/golang-nuts/b9d94e2f-860e-44eb-9f9e-efcd0a82b7d2n%40googlegroups.com?utm_medium=email&utm_source=footer>.

[burak serdar]

That is not always true. I saw code that looks like this a few days ago: what the author was trying to do was to handle a specific type of error differently, and pass it along if the error is some other type:

type CustomError struct{}

func (e CustomError) Error() string { return "error" }

func g() error {...}

func f() (error, bool) {
  err, ok := g().(CustomError)
  if ok {
    return err, true
   }
  return err, false
}

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

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/84507652-eabc-c149-bdfb-8032657536ed%40mb0.org.

[Martin Schnabel]

hi Burak,

yes. but this is a different problem based maybe on a oversight or a
missunderstanding how to use type assertions.

the original statement to use pointer types to implement error may be
harmless, but the reason given that things are brittel otherwise is not
correct and may lead to uncertainty and confusion.

however i think it is a good default position to use pointer values to
implement interfaces generally, if there is no other good reason not to.

On 9/22/22 19:54, burak serdar wrote:
> That is not always true. I saw code that looks like this a few days ago:
> what the author was trying to do was to handle a specific type of error
> differently, and pass it along if the error is some other type:
>
> type CustomError struct{}
>
> func (e CustomError) Error() string { return "error" }
>
> func g() error {...}
>
> func f() (error, bool) {
>   err, ok := g().(CustomError)
>   if ok {
>     return err, true
>    }
>   return err, false
> }
>

> https://go.dev/play/p/YoIjKtoynuI <https://go.dev/play/p/YoIjKtoynuI>

>
>
>
> On Thu, Sep 22, 2022 at 11:38 AM Martin Schnabel <m...@mb0.org
> <mailto:m...@mb0.org>> wrote:
>
> Sorry, to barge in but this specific thing is not really an issue as
> long as you use the error type as result type. because any value with an
> Error() string method does implement the error interface. and even an
> empty struct or a nil pointer to a applicable type will do just fine an
> matches err != nil
>
> have fun
>
> On 9/22/22 17:29, Tamás Gulácsi wrote:
> > plain struct error is brittle in other ways, too: you can shoot
> yourself
> > on foot with "err != nil" check.
> > So: error should be a pointer type.
> >

> > cpu...@gmail.com <mailto:cpu...@gmail.com> a következőt írta

> <mailto:golang-nuts%2Bunsu...@googlegroups.com>
> > <mailto:golang-nuts...@googlegroups.com
> <mailto:golang-nuts%2Bunsu...@googlegroups.com>>.

> > To view this discussion on the web visit
> >
> https://groups.google.com/d/msgid/golang-nuts/b9d94e2f-860e-44eb-9f9e-efcd0a82b7d2n%40googlegroups.com
> <https://groups.google.com/d/msgid/golang-nuts/b9d94e2f-860e-44eb-9f9e-efcd0a82b7d2n%40googlegroups.com>
>
> >

> <https://groups.google.com/d/msgid/golang-nuts/b9d94e2f-860e-44eb-9f9e-efcd0a82b7d2n%40googlegroups.com?utm_medium=email&utm_source=footer

> <https://groups.google.com/d/msgid/golang-nuts/b9d94e2f-860e-44eb-9f9e-efcd0a82b7d2n%40googlegroups.com?utm_medium=email&utm_source=footer>>.
>
> --
> 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

> <mailto:golang-nuts%2Bunsu...@googlegroups.com>.

> To view this discussion on the web visit

> https://groups.google.com/d/msgid/golang-nuts/84507652-eabc-c149-bdfb-8032657536ed%40mb0.org
> <https://groups.google.com/d/msgid/golang-nuts/84507652-eabc-c149-bdfb-8032657536ed%40mb0.org>.

>
> --
> 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
> <mailto:golang-nuts...@googlegroups.com>.
> To view this discussion on the web visit

> https://groups.google.com/d/msgid/golang-nuts/CAMV2Rqp0wPvgMoH8ZnmbGEQf5PGzO2ySy1wG9pXkh3Jbkyii6w%40mail.gmail.com
> <https://groups.google.com/d/msgid/golang-nuts/CAMV2Rqp0wPvgMoH8ZnmbGEQf5PGzO2ySy1wG9pXkh3Jbkyii6w%40mail.gmail.com?utm_medium=email&utm_source=footer>.

[Kevin Chowski]

If some func advertises the type of error it returns, it should mention the precise type. That is, if a function returns *ABC in error conditions, it should document that it returns *ABC (not ABC, that is a different type).

Why is that no sufficient? An API already must document the type of error they are returning, otherwise you shouldn't be depending on those internal implementation details anyway, so I see the problem of checking for "*ABC" vs "ABC" no different than the problem of checking for "SomeError" and "AnotherRandomErrorType": you must rely on the documentation to be correct in all cases.