From 329fcc64982d13927a6bb04ec65b0004cba2380e Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Tue, 19 Sep 2023 18:12:50 -0700 Subject: [PATCH] kvdb+sqldb: update SQL error parsing to account for non wrapped errs Some sub-systems like btcwallet will return an error from the database, but they won't properly wrap it. As a result, we were unable to actually catch the serialization errors in the first place. To work around this, we'll now attempt to parse the error string directly. --- kvdb/sqlbase/sqlerrors_postgres.go | 14 ++++++++++++++ kvdb/sqlbase/sqlerrors_sqlite.go | 16 ++++++++++++++++ sqldb/sqlerrors.go | 25 +++++++++++++++++++++++++ 3 files changed, 55 insertions(+) diff --git a/kvdb/sqlbase/sqlerrors_postgres.go b/kvdb/sqlbase/sqlerrors_postgres.go index 3915a34f4..1b12dda66 100644 --- a/kvdb/sqlbase/sqlerrors_postgres.go +++ b/kvdb/sqlbase/sqlerrors_postgres.go @@ -5,6 +5,7 @@ package sqlbase import ( "errors" "fmt" + "strings" "github.com/jackc/pgconn" "github.com/jackc/pgerrcode" @@ -13,6 +14,19 @@ import ( // parsePostgresError attempts to parse a postgres error as a database agnostic // SQL error. func parsePostgresError(err error) error { + if err == nil { + return nil + } + + // Sometimes the error won't be properly wrapped, so we'll need to + // inspect raw error itself to detect something we can wrap properly. + const postgresErrMsg = "could not serialize access" + if strings.Contains(err.Error(), postgresErrMsg) { + return &ErrSerializationError{ + DBError: err, + } + } + var pqErr *pgconn.PgError if !errors.As(err, &pqErr) { return nil diff --git a/kvdb/sqlbase/sqlerrors_sqlite.go b/kvdb/sqlbase/sqlerrors_sqlite.go index c29146610..9df93893a 100644 --- a/kvdb/sqlbase/sqlerrors_sqlite.go +++ b/kvdb/sqlbase/sqlerrors_sqlite.go @@ -5,6 +5,7 @@ package sqlbase import ( "errors" "fmt" + "strings" "modernc.org/sqlite" sqlite3 "modernc.org/sqlite/lib" @@ -13,6 +14,21 @@ import ( // parseSqliteError attempts to parse a sqlite error as a database agnostic // SQL error. func parseSqliteError(err error) error { + if err == nil { + return nil + } + + // If the error isn't wrapped properly, the errors.As call with fail, + // so we'll also try to check the expected error message directly. + // This is taken from: + // https://gitlab.com/cznic/sqlite/-/blob/v1.25.0/sqlite.go#L75. + const sqliteErrMsg = "SQLITE_BUSY" + if strings.Contains(err.Error(), sqliteErrMsg) { + return &ErrSerializationError{ + DBError: err, + } + } + var sqliteErr *sqlite.Error if !errors.As(err, &sqliteErr) { return nil diff --git a/sqldb/sqlerrors.go b/sqldb/sqlerrors.go index a939ec499..6cc1b1cf8 100644 --- a/sqldb/sqlerrors.go +++ b/sqldb/sqlerrors.go @@ -5,6 +5,7 @@ package sqldb import ( "errors" "fmt" + "strings" "github.com/jackc/pgconn" "github.com/jackc/pgerrcode" @@ -21,6 +22,10 @@ var ( // MapSQLError attempts to interpret a given error as a database agnostic SQL // error. func MapSQLError(err error) error { + if err == nil { + return nil + } + // Attempt to interpret the error as a sqlite error. var sqliteErr *sqlite.Error if errors.As(err, &sqliteErr) { @@ -33,6 +38,26 @@ func MapSQLError(err error) error { return parsePostgresError(pqErr) } + // Sometimes the error won't be properly wrapped, so we'll need to + // inspect raw error itself to detect something we can wrap properly. + // This handles a postgres variant of the error. + const postgresErrMsg = "could not serialize access" + if strings.Contains(err.Error(), postgresErrMsg) { + return &ErrSerializationError{ + DBError: err, + } + } + + // We'll also attempt to catch this for sqlite, that uses a slightly + // different error message. This is taken from: + // https://gitlab.com/cznic/sqlite/-/blob/v1.25.0/sqlite.go#L75. + const sqliteErrMsg = "SQLITE_BUSY" + if strings.Contains(err.Error(), sqliteErrMsg) { + return &ErrSerializationError{ + DBError: err, + } + } + // Return original error if it could not be classified as a database // specific error. return err