errors: As does not identify error of the same type

[jamesphillpotts-fr]

Go version

1.23.0

Output of go env in your module/workspace:

$ go env
GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/james-phillpotts/.cache/go-build'
GOENV='/home/james-phillpotts/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/james-phillpotts/dev/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/james-phillpotts/dev/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/home/james-phillpotts/sdk/go1.23.0'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/home/james-phillpotts/sdk/go1.23.0/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.23.0'
GODEBUG=''
GOTELEMETRY='local'
GOTELEMETRYDIR='/home/james-phillpotts/.config/go/telemetry'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/dev/null'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build75294450=/tmp/go-build -gno-record-gcc-switches'

What did you do?

Attempting to use errors.As to check the content of a custom error type that has not been wrapped.

See https://go.dev/play/p/AVAu1v9QGNX for simple reproduction.

It seems that this will never work for errors.As (even though the doc says it should) due to the use of targetType in the call to the private as function: return as(err, target, val, targetType) - targetType is typ.Elem() so this will never satisfy the check in the first if statement in as:

if reflectlite.TypeOf(err).AssignableTo(targetType) {

As targetType is an element type but reflectlite.TypeOf(err) will always be a pointer type.

I then thought it should be easy to fix, so started working on a PR, starting with a failing unit test, which failed as expected:

diff --git a/src/errors/wrap_test.go b/src/errors/wrap_test.go
index 58ed95fd9a..20399b19c1 100644
--- a/src/errors/wrap_test.go
+++ b/src/errors/wrap_test.go
@@ -195,6 +195,11 @@ func TestAs(t *testing.T) {
        &errT,
        false,
        nil,
+   }, {
+       errorT{"T"},
+       &errT,
+       false,
+       errorT{"T"},
    }}
    for i, tc := range testCases {
        name := fmt.Sprintf("%d:As(Errorf(..., %v), %v)", i, tc.err, tc.target)

I then attempted to fix this:

diff --git a/src/errors/wrap.go b/src/errors/wrap.go
index eec9591dae..54134a9a89 100644
--- a/src/errors/wrap.go
+++ b/src/errors/wrap.go
@@ -110,7 +110,7 @@ func As(err error, target any) bool {
    if targetType.Kind() != reflectlite.Interface && !targetType.Implements(errorType) {
        panic("errors: *target must be interface or implement error")
    }
-   return as(err, target, val, targetType)
+   return as(err, target, val, typ)
 }

 func as(err error, target any, targetVal reflectlite.Value, targetType reflectlite.Type) bool {

What did you see happen?

Unfortunately with the fix in place, the all.bash script no longer works, failing with:

$ ./all.bash 
Building Go cmd/dist using /home/james-phillpotts/sdk/go1.23.0. (go1.23.0 linux/amd64)
Building Go toolchain1 using /home/james-phillpotts/sdk/go1.23.0.
Building Go bootstrap cmd/go (go_bootstrap) using Go toolchain1.
Building Go toolchain2 using go_bootstrap and Go toolchain1.
Building Go toolchain3 using go_bootstrap and Go toolchain2.
Building packages and commands for linux/amd64.

##### Test execution environment.
# GOARCH: amd64
# CPU: 12th Gen Intel(R) Core(TM) i7-1260P
# GOOS: linux
# OS Version: Linux 6.8.0-48-generic #48~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Mon Oct  7 11:24:13 UTC 2 x86_64

##### Testing packages.
not a flag: "archive/tar"
usage: go test [build/test flags] [packages] [build/test flags & test binary flags]
Run 'go help test' and 'go help testflag' for details.
go tool dist: Failed: exit status 2

Debugging, the command line args that are being processed can be seen to be:

/home/james-phillpotts/dev/go-src/bin/go test -count=1 -timeout=9m0s -short archive/tar archive/zip bufio bytes cmp compress/bzip2 compress/flate compress/gzip compress/lzw compress/zlib container/heap container/list container/ring context crypto crypto/aes crypto/cipher crypto/des crypto/dsa crypto/ecdh crypto/ecdsa crypto/ed25519 crypto/elliptic crypto/hmac crypto/internal/boring crypto/internal/boring/bcache crypto/internal/fips/aes crypto/internal/fips/aes/gcm crypto/internal/fips/bigmod crypto/internal/fips/ecdh crypto/internal/fips/ecdsa crypto/internal/fips/edwards25519 crypto/internal/fips/edwards25519/field crypto/internal/fips/mlkem crypto/internal/fips/nistec crypto/internal/fips/nistec/fiat crypto/internal/fipsdeps crypto/internal/fipstest crypto/internal/hpke crypto/internal/sysrand crypto/md5 crypto/rand crypto/rc4 crypto/rsa crypto/sha1 crypto/sha256 crypto/sha512 crypto/subtle crypto/tls crypto/x509 database/sql database/sql/driver debug/buildinfo debug/dwarf debug/elf debug/gosym debug/macho debug/pe debug/plan9obj embed embed/internal/embedtest encoding/ascii85 encoding/asn1 encoding/base32 encoding/base64 encoding/binary encoding/csv encoding/gob encoding/hex encoding/json encoding/pem encoding/xml errors expvar flag fmt go/ast go/ast/internal/tests go/build go/build/constraint go/constant go/doc go/doc/comment go/format go/importer go/internal/gccgoimporter go/internal/gcimporter go/internal/srcimporter go/parser go/printer go/scanner go/token go/types go/version hash hash/adler32 hash/crc32 hash/crc64 hash/fnv hash/maphash html html/template image image/color image/draw image/gif image/jpeg image/png index/suffixarray internal/abi internal/buildcfg internal/chacha8rand internal/coverage/cfile internal/coverage/cformat internal/coverage/cmerge internal/coverage/pods internal/coverage/slicereader internal/coverage/slicewriter internal/coverage/test internal/cpu internal/dag internal/diff internal/fmtsort internal/fuzz internal/godebug internal/godebugs internal/gover internal/itoa internal/pkgbits internal/platform internal/poll internal/profile internal/reflectlite internal/runtime/atomic internal/runtime/maps internal/runtime/math internal/runtime/sys internal/runtime/syscall internal/saferio internal/singleflight internal/sync internal/synctest internal/syscall/unix internal/sysinfo internal/testenv internal/trace internal/trace/internal/oldtrace internal/types/errors internal/unsafeheader internal/xcoff internal/zstd io io/fs io/ioutil iter log log/slog log/slog/internal/benchmarks log/slog/internal/buffer log/syslog maps math math/big math/bits math/cmplx math/rand math/rand/v2 mime mime/multipart mime/quotedprintable net net/http net/http/cgi net/http/cookiejar net/http/fcgi net/http/httptest net/http/httptrace net/http/httputil net/http/internal net/http/internal/ascii net/http/pprof net/internal/cgotest net/internal/socktest net/mail net/netip net/rpc net/rpc/jsonrpc net/smtp net/textproto net/url os os/exec os/exec/internal/fdtest os/signal os/user path path/filepath plugin reflect regexp regexp/syntax runtime runtime/cgo runtime/debug runtime/internal/wasitest runtime/metrics runtime/pprof runtime/trace slices sort strconv strings sync sync/atomic syscall testing testing/fstest testing/iotest testing/quick testing/slogtest text/scanner text/tabwriter text/template text/template/parse time unicode unicode/utf16 unicode/utf8 unique weak cmd/addr2line cmd/api cmd/asm/internal/asm cmd/asm/internal/lex cmd/cgo/internal/swig cmd/cgo/internal/test cmd/cgo/internal/testcarchive cmd/cgo/internal/testcshared cmd/cgo/internal/testerrors cmd/cgo/internal/testfortran cmd/cgo/internal/testgodefs cmd/cgo/internal/testlife cmd/cgo/internal/testnocgo cmd/cgo/internal/testplugin cmd/cgo/internal/testsanitizers cmd/cgo/internal/testshared cmd/cgo/internal/testso cmd/cgo/internal/teststdio cmd/cgo/internal/testtls cmd/compile cmd/compile/internal/abt cmd/compile/internal/amd64 cmd/compile/internal/base cmd/compile/internal/compare cmd/compile/internal/devirtualize cmd/compile/internal/dwarfgen cmd/compile/internal/importer cmd/compile/internal/inline/inlheur cmd/compile/internal/ir cmd/compile/internal/liveness cmd/compile/internal/logopt cmd/compile/internal/loopvar cmd/compile/internal/noder cmd/compile/internal/rangefunc cmd/compile/internal/reflectdata cmd/compile/internal/ssa cmd/compile/internal/ssagen cmd/compile/internal/syntax cmd/compile/internal/test cmd/compile/internal/typecheck cmd/compile/internal/types cmd/compile/internal/types2 cmd/covdata cmd/cover cmd/dist cmd/distpack cmd/doc cmd/fix cmd/go cmd/go/internal/auth cmd/go/internal/cache cmd/go/internal/cfg cmd/go/internal/envcmd cmd/go/internal/fsys cmd/go/internal/generate cmd/go/internal/gover cmd/go/internal/imports cmd/go/internal/load cmd/go/internal/lockedfile cmd/go/internal/lockedfile/internal/filelock cmd/go/internal/modfetch cmd/go/internal/modfetch/codehost cmd/go/internal/modfetch/zip_sum_test cmd/go/internal/modindex cmd/go/internal/modload cmd/go/internal/mvs cmd/go/internal/str cmd/go/internal/test cmd/go/internal/toolchain cmd/go/internal/vcs cmd/go/internal/vcweb cmd/go/internal/vcweb/vcstest cmd/go/internal/web cmd/go/internal/work cmd/gofmt cmd/internal/archive cmd/internal/bootstrap_test cmd/internal/buildid cmd/internal/cov cmd/internal/dwarf cmd/internal/edit cmd/internal/goobj cmd/internal/moddeps cmd/internal/obj cmd/internal/obj/arm64 cmd/internal/obj/loong64 cmd/internal/obj/ppc64 cmd/internal/obj/riscv cmd/internal/obj/s390x cmd/internal/obj/x86 cmd/internal/objabi cmd/internal/osinfo cmd/internal/par cmd/internal/pgo cmd/internal/pkgpath cmd/internal/pkgpattern cmd/internal/quoted cmd/internal/src cmd/internal/sys cmd/internal/test2json cmd/link cmd/link/internal/benchmark cmd/link/internal/ld cmd/link/internal/loader cmd/nm cmd/objdump cmd/pack cmd/pprof cmd/relnote cmd/trace cmd/vet

The error is from parsing the first non-flag argument.

What did you expect to see?

I expected the fix to work, I think there is a bug in the handling of cmdflag.NonFlagError in src/cmd/go/internal/test/testflag.go - it seems like without the fix to the errors.As function, that if block is actually never being hit even when a NonFlagError is returned from cmdflag.ParseOne - looking at the . I'm not quite sure how that should be fixed, so here's a bug report instead of a PR.

[gabyhelp]

Related Issues

• cmd/doc: garbled -src output #37028
• 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 (closed)
• errors, cmd/vet: too easy to pass a pointer-to-pointer to `errors.As` when it should be a pointer-to-value #34091
• x/tools/gopls: errors.As warning for no apparent reason #41903 (closed)
• cmd/compile: ICE with `assertion failed` when using `GODEBUG=gotypesalias=1` #65893 (closed)
• go/types: uncommon pointer sizes result in an assertion failure #61249 (closed)
• runtime: panic when using errors.As with validation errors from github.com/go-ozzo/ozzo-validation #35408 (closed)
• everything: fail to check err == nil when assigning explicit type to `error` #56930 (closed)
• go/types: can't check interface satisfaction against interfaces defined in package builtin #69717 (closed)

Related Discussions

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

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[seankhliao]

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

err's type is *myError, target has to point to that, so target's type must be **myError

closing as working as intended

[jamesphillpotts-fr]

@seankhliao Sure, but the issue in testflag.go is also what I'm reporting:

if nf := (cmdflag.NonFlagError{}); errors.As(err, &nf) {

and later

if nd := (cmdflag.FlagNotDefinedError{}); errors.As(err, &nd) {

This suffers the same problem as my mistake?

[seankhliao]

as far as i can tell, cmd/go is using it correctly. you should try to produce a test case that targets testFlags.

[jamesphillpotts-fr]

It's using exactly the same pattern as my code sample that you pointed out was wrong?

[jamesphillpotts-fr]

That is, it's passing *cmdflag.NonFlagErorr not **cmdflag.NonFlagError

[ianlancetaylor]

The code in cmd/go looks correct to me. The type of the error returned by cmdflag.ParseOne is NonFlagError, not *NonFlagError. The type passed to errors.As is *NonFlagError, which will, as expected, match an error of type NonFlagError.

Further discussion should go to a forum, not the issue tracker. See https://go.dev/wiki/Questions. Thanks.