Skip to content

database/sql: use %w verb for valuer errors to enable errors.Is/As #64707

New issue

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

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

Already on GitHub? # to your account

Closed
joshgarrett-hellofresh opened this issue Dec 13, 2023 · 6 comments
Closed
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.

Comments

@joshgarrett-hellofresh
Copy link

joshgarrett-hellofresh commented Dec 13, 2023

Go version

go version go1.21.4 darwin/arm64

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

GO111MODULE=''
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/gofer/Library/Caches/go-build'
GOENV='/Users/gofer/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/gofer/go/pkg/mod'
GOOS='darwin'
GOPATH='/Users/gofer/go'
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/opt/homebrew/Cellar/go/1.21.4/libexec'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/opt/homebrew/Cellar/go/1.21.4/libexec/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.21.4'
GCCGO='gccgo'
AR='ar'
CC='cc'
CXX='c++'
CGO_ENABLED='1'
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 -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/cf/314tnd5d22129h1z7srmcklr0000gn/T/go-build479648600=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

package main

import (
	"context"
	"database/sql"
	"database/sql/driver"
	"errors"
)

type Valuer struct {
	field []byte
}

func (v Valuer) Value() (driver.Value, error) {
	if len(v.field) == 0 {
		// return an error that we hope to be able to handle later
		return nil, errEmpty
	}
	return v, nil
}

// errEmpty is returned when an empty value is found
var errEmpty = errors.New("empty value")

func main() {
	db, _ := sql.Open("pgx", dsn)
	_, err := db.ExecContext(
		context.Background(),
		`INSERT INTO some_table (some_bytea_column) VALUES ($1)`,
		&Valuer{nil},
	)
	// errors.Is doesn't work b/c %v verb was used for error instead of %w
	if !errors.Is(err, errEmpty) {
		panic(err)
	}
}

What did you expect to see?

errors.Is should be able to detect sentinel errors returned by valuer. error will need to be properly wrapped by fmt.Errorf with %w verb instead of %v, similar to scanner

What did you see instead?

valuer errors are wrapped by fmt.Errorf with %v verb

@mauri870
Copy link
Member

Edited your issue to add a code block.

@mauri870 mauri870 added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Dec 14, 2023
@mauri870
Copy link
Member

mauri870 commented Dec 14, 2023

I don't think this can cause any issues, so feel free to send a CL. Thanks!

@mauri870 mauri870 added help wanted NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Dec 14, 2023
aimuz added a commit to aimuz/go that referenced this issue Dec 15, 2023
Use fmt.Errorf's %w verb to wrap errors in driverArgsConnLocked,
which allows for easier unwrapping and checking of error types.

Add tests in sql_test.go to ensure that Stmt.Exec and Stmt.Query
correctly wrap underlying Valuer errors, adhering to the new change.

Fixes golang#64707.
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/550116 mentions this issue: database/sql: wrap errors with %w in driverArgsConnLocked

@callthingsoff
Copy link
Contributor

Related issue #44635, and CL https://go-review.googlesource.com/c/go/+/333990

@aimuz
Copy link
Contributor

aimuz commented Feb 19, 2024

@mauri870 Can you help me review CL? https://go.dev/cl/550116

aimuz added a commit to aimuz/go that referenced this issue Feb 20, 2024
Use fmt.Errorf's %w verb to wrap errors in driverArgsConnLocked,
which allows for easier unwrapping and checking of error types.

Add tests in sql_test.go to ensure that Stmt.Exec and Stmt.Query
correctly wrap underlying Valuer errors, adhering to the new change.

Fixes golang#64707.
aimuz added a commit to aimuz/go that referenced this issue Feb 21, 2024
Use fmt.Errorf's %w verb to wrap errors in driverArgsConnLocked,
which allows for easier unwrapping and checking of error types.

Add tests in sql_test.go to ensure that Stmt.Exec and Stmt.Query
correctly wrap underlying Valuer errors, adhering to the new change.

Fixes golang#64707.
aimuz added a commit to aimuz/go that referenced this issue Mar 4, 2024
For golang#64707
For golang#65614

Signed-off-by: aimuz <mr.imuz@gmail.com>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/568755 mentions this issue: doc/go1.23: document database/sql wrap errors

gopherbot pushed a commit that referenced this issue Mar 4, 2024
For #64707.
For #65614.

Change-Id: Ib07ac67d7652bc7c9e1363f70637938c7bb4bc72
GitHub-Last-Rev: a4d8eca
GitHub-Pull-Request: #66089
Reviewed-on: https://go-review.googlesource.com/c/go/+/568755
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
@golang golang locked and limited conversation to collaborators Mar 4, 2025
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants