Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

errors: As can be confusing to use when given a pointer to a type that does not implement error, but a pointer to that type does #37625

Open
alvaroaleman opened this issue Mar 3, 2020 · 7 comments
Assignees
Labels
Milestone

Comments

@alvaroaleman
Copy link

@alvaroaleman alvaroaleman commented Mar 3, 2020

What version of Go are you using (go version)?

$ go version
go version go1.13.8 linux/amd64

Does this issue reproduce with the latest release?

asumming the playground uses the latest release, yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/alvaro/.gocache"
GOENV="/home/alvaro/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/alvaro/git/golang"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/alvaro/git/golang/src/k8s.io/kubernetes/staging/src/k8s.io/apimachinery/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build251903277=/tmp/go-build -gno-record-gcc-switches"

What did you do?

https://play.golang.org/p/TeXqOwa0wGM

What did you expect to see?

Successful compilation

What did you see instead?

second argument to errors.As must be a pointer to an interface or a type implementing error

The workaround is to pass in a double pointer, but that is super unintuitive: https://play.golang.org/p/kehktd-1b5t

@alvaroaleman alvaroaleman changed the title errors.Is does not work for error types that implement error via pointer receiver errors.As does not work for error types that implement error via pointer receiver Mar 3, 2020
@dmitshur dmitshur changed the title errors.As does not work for error types that implement error via pointer receiver errors: As does not work for error types that implement error via pointer receiver Mar 3, 2020
@dmitshur dmitshur added this to the Backlog milestone Mar 3, 2020
@dmitshur

This comment has been minimized.

Copy link
Member

@dmitshur dmitshur commented Mar 3, 2020

The second argument to errors.As must be a pointer, because As needs to modify the value that is being passed in. If you provide it a value rather than a pointer, that value gets copied, and any changes made to by errors.As would not be visible to the caller.

Additionally, the pointer must be to a type that implements error, since errors.As needs to deal with errors.

In your original snippet, the type *ptrErr implements the error interface. The type ptrErr does not. Note that this fails to compile:

var _ error = ptrErr{}

// ./prog.go:15:5: cannot use ptrErr literal (type ptrErr) as type error in assignment:
// 	ptrErr does not implement error (Error method has pointer receiver)

There isn't a way errors.As can do its job if you don't provide it with a pointer to a type that implements the error interface, that's why you see the error from it:

second argument to errors.As must be a pointer to an interface or a type implementing error

Do you think the error text can be improved to make this more clear?

/cc @jba @neild

@dmitshur dmitshur changed the title errors: As does not work for error types that implement error via pointer receiver errors: As can be confusing to use when given a pointer to a type that does not implement error, but a pointer to that type does Mar 3, 2020
@alvaroaleman

This comment has been minimized.

Copy link
Author

@alvaroaleman alvaroaleman commented Mar 3, 2020

There isn't a way errors.As can do its job if you don't provide it with a pointer to a type that implements the error interface, that's why you see the error from it

But couldn't it check if the pointer type implements the error interface, rather than dereferencing it unconditionally first? As mentioned, its possible to workaround this by passing in a double pointer, but that is IMHO far from intuitive.

@dmitshur

This comment has been minimized.

Copy link
Member

@dmitshur dmitshur commented Mar 3, 2020

But couldn't it check if the pointer type implements the error interface, rather than dereferencing it unconditionally first?

At this time, I'm not sure if this is possible.

its possible to workaround this by passing in a double pointer

As far as I know, that's not just a workaround, that's the intended usage. A double pointer can be thought of as a single pointer to a value that implements the error interface.

From errors package documentation:

var perr *os.PathError
if errors.As(err, &perr) {
	fmt.Println(perr.Path)
}

Note that perr value is a pointer to os.PathError, and the second argument being passed to errors.As is a pointer to that.

Can you clarify if you're only interested in the boolean result value from errors.As? In the original snippet you shared, even if it didn't panic, it wouldn't be possible to get the value of the second argument to errors.As.

@dmitshur

This comment has been minimized.

Copy link
Member

@dmitshur dmitshur commented Mar 3, 2020

Also see https://blog.golang.org/go1.13-errors#TOC_3.2. if you haven't already.

@alvaroaleman

This comment has been minimized.

Copy link
Author

@alvaroaleman alvaroaleman commented Mar 3, 2020

I see, thanks for your responses. So I think the main source of confusion for me here was that I read the error second argument to errors.As must be a pointer to an interface or a type implementing error as second argument to errors.As must be a (pointer to an interface) or (a type implementing error) rather than second argument to errors.As must be pointer to (an interface) or (a type implementing error).

Maybe we can reword it to sth like second argument to errors.As must be pointer to an interface or a pointer to a type implementing error (which itself may be a pointer) ?

Can you clarify if you're only interested in the boolean result value from errors.As? In the original snippet you shared, even if it didn't panic, it wouldn't be possible to get the value of the second argument to errors.As.

Yes I am, I just tried to keep the snippet short

@dmitshur

This comment has been minimized.

Copy link
Member

@dmitshur dmitshur commented Mar 3, 2020

I'm glad we figured out the source of confusion, that's very helpful and can be used to improve the error message.

There is yet another possible interpretation: second argument to errors.As must be a pointer to (an interface or a type) implementing error.

So, it should be helpful to reword the error text to make the interpretation less ambiguous.

@jba jba self-assigned this Mar 3, 2020
@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Mar 3, 2020

Change https://golang.org/cl/221877 mentions this issue: go/analysis/passes/erroras: clarify message

gopherbot pushed a commit to golang/tools that referenced this issue Mar 3, 2020
Make it clear that the second argument must be a non-nil pointer.

The new message text matches the phrasing used in the errors.As doc.

Updates golang/go#37625.

Change-Id: I69dc2e34a5f3a5573030ba0f63f20e0821be1816
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221877
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
@dmitshur dmitshur added NeedsFix and removed NeedsInvestigation labels Mar 3, 2020
@dmitshur dmitshur modified the milestones: Backlog, Go1.15 Mar 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.