From: aimuz <mr.imuz@gmail.com>
Date: Wed, 21 Feb 2024 02:15:40 +0000 (+0000)
Subject: database/sql: wrap errors with %w in driverArgsConnLocked
X-Git-Tag: go1.23rc1~1110
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=d8311c86239eebe36e9882760297fd85197add3c;p=gostls13.git

database/sql: wrap errors with %w in driverArgsConnLocked

Use fmt.Errorf %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 #64707.

Change-Id: Id9f80e265735d0849ee7abba63e58e4c26e658ad
GitHub-Last-Rev: 0df367e0fb5b213513d4e0ab7f5a87984798f96d
GitHub-Pull-Request: golang/go#64728
Reviewed-on: https://go-review.googlesource.com/c/go/+/550116
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Reviewed-by: Mauri de Souza Meneguzzo <mauri870@gmail.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
---

diff --git a/src/database/sql/convert.go b/src/database/sql/convert.go
index cca5d15e07..b291c1557c 100644
--- a/src/database/sql/convert.go
+++ b/src/database/sql/convert.go
@@ -192,7 +192,7 @@ func driverArgsConnLocked(ci driver.Conn, ds *driverStmt, args []any) ([]driver.
 			}
 			goto nextCheck
 		default:
-			return nil, fmt.Errorf("sql: converting argument %s type: %v", describeNamedValue(nv), err)
+			return nil, fmt.Errorf("sql: converting argument %s type: %w", describeNamedValue(nv), err)
 		}
 	}
 
diff --git a/src/database/sql/driver/types.go b/src/database/sql/driver/types.go
index 0380572ab1..a322f85277 100644
--- a/src/database/sql/driver/types.go
+++ b/src/database/sql/driver/types.go
@@ -34,6 +34,10 @@ type ValueConverter interface {
 
 // Valuer is the interface providing the Value method.
 //
+// Errors returned by the [Value] method are wrapped by the database/sql package.
+// This allows callers to use [errors.Is] for precise error handling after operations
+// like [database/sql.Query], [database/sql.Exec], or [database/sql.QueryRow].
+//
 // Types implementing Valuer interface are able to convert
 // themselves to a driver [Value].
 type Valuer interface {
diff --git a/src/database/sql/sql_test.go b/src/database/sql/sql_test.go
index c38a348ab4..eb4e22fcd8 100644
--- a/src/database/sql/sql_test.go
+++ b/src/database/sql/sql_test.go
@@ -4398,6 +4398,49 @@ func TestRowsScanProperlyWrapsErrors(t *testing.T) {
 	}
 }
 
+type alwaysErrValuer struct{}
+
+// errEmpty is returned when an empty value is found
+var errEmpty = errors.New("empty value")
+
+func (v alwaysErrValuer) Value() (driver.Value, error) {
+	return nil, errEmpty
+}
+
+// Issue 64707: Ensure that Stmt.Exec and Stmt.Query properly wraps underlying errors.
+func TestDriverArgsWrapsErrors(t *testing.T) {
+	db := newTestDB(t, "people")
+	defer closeDB(t, db)
+
+	t.Run("exec", func(t *testing.T) {
+		_, err := db.Exec("INSERT|keys|dec1=?", alwaysErrValuer{})
+		if err == nil {
+			t.Fatal("expecting back an error")
+		}
+		if !errors.Is(err, errEmpty) {
+			t.Fatalf("errors.Is mismatch\n%v\nWant: %v", err, errEmpty)
+		}
+		// Ensure that error substring matching still correctly works.
+		if !strings.Contains(err.Error(), errEmpty.Error()) {
+			t.Fatalf("Error %v does not contain %v", err, errEmpty)
+		}
+	})
+
+	t.Run("query", func(t *testing.T) {
+		_, err := db.Query("INSERT|keys|dec1=?", alwaysErrValuer{})
+		if err == nil {
+			t.Fatal("expecting back an error")
+		}
+		if !errors.Is(err, errEmpty) {
+			t.Fatalf("errors.Is mismatch\n%v\nWant: %v", err, errEmpty)
+		}
+		// Ensure that error substring matching still correctly works.
+		if !strings.Contains(err.Error(), errEmpty.Error()) {
+			t.Fatalf("Error %v does not contain %v", err, errEmpty)
+		}
+	})
+}
+
 func TestContextCancelDuringRawBytesScan(t *testing.T) {
 	for _, mode := range []string{"nocancel", "top", "bottom", "go"} {
 		t.Run(mode, func(t *testing.T) {