Test all combinations

This adds a test to ensure that sql.ErrNoRows and other database sql errors are also not selectable. The error message is also preserved. This prevents the need for logging the error as well as returning the error.
SimonRichardson · May 15, 2024 · f32f3c3 · f32f3c3
1 parent fdacdae
commit f32f3c3
Show file tree

Hide file tree

Showing 2 changed files with 48 additions and 9 deletions.
diff --git a/domain/errors.go b/domain/errors.go
@@ -23,10 +23,7 @@ func CoerceError(err error) error {
 
 	// If the error is a sql error, a dqlite error or a database error, we mask
 	// the error to prevent it from being unwrapped.
-	if errors.Is(err, sql.ErrNoRows) ||
-		database.IsError(err) ||
-		errors.Is(err, sql.ErrTxDone) ||
-		errors.Is(err, sql.ErrConnDone) {
+	if isDatabaseError(err) {
 		return errors.Trace(maskError{error: err})
 	}
 
@@ -57,9 +54,17 @@ func (e maskError) As(target any) bool {
 // Is implements standard errors Is interface. Is will check if the target type
 // is a sql error that is trying to be retrieved and return false.
 func (e maskError) Is(target error) bool {
-	if database.IsError(target) {
+	if isDatabaseError(target) {
 		return false
 	}
 
 	return errors.Is(e.error, target)
 }
+
+// isDatabaseError checks if the error is a sql, sqlite or dqlite error.
+func isDatabaseError(err error) bool {
+	return errors.Is(err, sql.ErrNoRows) ||
+		database.IsError(err) ||
+		errors.Is(err, sql.ErrTxDone) ||
+		errors.Is(err, sql.ErrConnDone)
+}
diff --git a/domain/errors_test.go b/domain/errors_test.go
@@ -4,6 +4,7 @@
 package domain
 
 import (
+	"database/sql"
 	"fmt"
 
 	dqlite "github.com/canonical/go-dqlite/driver"
@@ -39,29 +40,62 @@ func (e *errorsSuite) TestMaskErrorIsHidesSqlErrors(c *gc.C) {
 	tests := []struct {
 		Name  string
 		Error error
-		Rval  bool
 	}{
 		{
 			Name: "Test sqlite3 errors are hidden from Is()",
 			Error: sqlite3.Error{
 				Code:         sqlite3.ErrAbort,
 				ExtendedCode: sqlite3.ErrBusyRecovery,
 			},
-			Rval: false,
 		},
 		{
 			Name: "Test dqlite errors are hidden from Is()",
 			Error: dqlite.Error{
 				Code:    dqlite.ErrBusy,
 				Message: "something went wrong",
 			},
-			Rval: false,
+		},
+		{
+			Name:  "Test sql.ErrNoRows errors are hidden from Is()",
+			Error: sql.ErrNoRows,
+		},
+		{
+			Name:  "Test sql.ErrTxDone errors are hidden from Is()",
+			Error: sql.ErrTxDone,
+		},
+		{
+			Name:  "Test sql.ErrConnDone errors are hidden from Is()",
+			Error: sql.ErrConnDone,
 		},
 	}
 
 	for _, test := range tests {
 		err := maskError{fmt.Errorf("%q %w", test.Name, test.Error)}
-		c.Check(test.Rval, gc.Equals, errors.Is(err, test.Error), gc.Commentf(test.Name))
+		c.Check(errors.Is(err, test.Error), jc.IsFalse, gc.Commentf(test.Name))
+	}
+}
+
+func (e *errorsSuite) TestErrorMessagePreserved(c *gc.C) {
+	tests := []struct {
+		Error    error
+		Expected string
+	}{
+		{
+			Error:    fmt.Errorf("wrap orig error: %w", sql.ErrNoRows),
+			Expected: "wrap orig error: sql: no rows in result set",
+		},
+		{
+			Error:    fmt.Errorf("wrap orig error: %w%w", sql.ErrNoRows, dqlite.Error{Code: dqlite.ErrBusy}),
+			Expected: "wrap orig error: sql: no rows in result set",
+		},
+		{
+			Error:    fmt.Errorf("wrap orig error: %w - %w", sql.ErrNoRows, fmt.Errorf("nested error")),
+			Expected: "wrap orig error: sql: no rows in result set - nested error",
+		},
+	}
+	for _, test := range tests {
+		err := CoerceError(test.Error)
+		c.Check(err.Error(), gc.Equals, test.Expected)
 	}
 }