From 57bd9d12d147a8672f30ffb36d19bd6b944e07d5 Mon Sep 17 00:00:00 2001 From: Jesse Rittner Date: Fri, 23 Aug 2019 20:59:27 -0400 Subject: [PATCH 1/3] adding SystemErrno to Error, and fixing error logic when open fails --- error.go | 19 +++++++++++++++++-- error_test.go | 26 ++++++++++++++++++++++++++ sqlite3.go | 29 +++++++++++++++++++++-------- 3 files changed, 64 insertions(+), 10 deletions(-) diff --git a/error.go b/error.go index 49ab8903..1df83daf 100644 --- a/error.go +++ b/error.go @@ -5,7 +5,15 @@ package sqlite3 +/* +#ifndef USE_LIBSQLITE3 +#include +#else +#include +#endif +*/ import "C" +import "syscall" // ErrNo inherit errno. type ErrNo int @@ -20,6 +28,7 @@ type ErrNoExtended int type Error struct { Code ErrNo /* The error code returned by SQLite */ ExtendedCode ErrNoExtended /* The extended error code returned by SQLite */ + SystemErrno syscall.Errno /* The system errno returned by the OS through SQLite, if applicable */ err string /* The error string returned by sqlite3_errmsg(), this usually contains more specific details. */ } @@ -72,10 +81,16 @@ func (err ErrNoExtended) Error() string { } func (err Error) Error() string { + var str string if err.err != "" { - return err.err + str = err.err + } else { + str = C.GoString(C.sqlite3_errstr(C.int(err.Code))) } - return errorString(err) + if err.SystemErrno != 0 { + str += ": " + err.SystemErrno.Error() + } + return str } // result codes from http://www.sqlite.org/c3ref/c_abort_rollback.html diff --git a/error_test.go b/error_test.go index 7fccd572..49a75b25 100644 --- a/error_test.go +++ b/error_test.go @@ -238,5 +238,31 @@ func TestExtendedErrorCodes_Unique(t *testing.T) { extended, expected) } } +} + +func TestError_SystemErrno(t *testing.T) { + // open a non-existent database in read-only mode so we get an IO error. + db, err := sql.Open("sqlite3", "file:nonexistent.db?mode=ro") + if err != nil { + t.Fatal(err) + } + defer db.Close() + + err = db.Ping() + if err == nil { + t.Fatal("expected error pinging read-only non-existent database, but got nil") + } + + serr, ok := err.(Error) + if !ok { + t.Fatalf("expected error to be of type Error, but got %[1]T %[1]v", err) + } + if serr.SystemErrno == 0 { + t.Fatal("expected SystemErrno to be set") + } + + if !os.IsNotExist(serr.SystemErrno) { + t.Errorf("expected SystemErrno to be a not exists error, but got %v", serr.SystemErrno) + } } diff --git a/sqlite3.go b/sqlite3.go index 4000173c..6067206d 100644 --- a/sqlite3.go +++ b/sqlite3.go @@ -198,6 +198,7 @@ import ( "strconv" "strings" "sync" + "syscall" "time" "unsafe" ) @@ -749,15 +750,28 @@ func (c *SQLiteConn) lastError() error { return lastError(c.db) } +// Note: may be called with db == nil func lastError(db *C.sqlite3) error { - rv := C.sqlite3_errcode(db) + rv := C.sqlite3_errcode(db) // returns SQLITE_NOMEM if db == nil if rv == C.SQLITE_OK { return nil } + extrv := C.sqlite3_extended_errcode(db) // returns SQLITE_NOMEM if db == nil + errStr := C.GoString(C.sqlite3_errmsg(db)) // returns "out of memory" if db == nil + + // https://www.sqlite.org/c3ref/system_errno.html + // sqlite3_system_errno is only meaningful if the error code was SQLITE_CANTOPEN, + // or it was SQLITE_IOERR and the extended code was not SQLITE_IOERR_NOMEM + var systemErrno syscall.Errno + if rv == C.SQLITE_CANTOPEN || (rv == C.SQLITE_IOERR && extrv != C.SQLITE_IOERR_NOMEM) { + systemErrno = syscall.Errno(C.sqlite3_system_errno(db)) + } + return Error{ Code: ErrNo(rv), - ExtendedCode: ErrNoExtended(C.sqlite3_extended_errcode(db)), - err: C.GoString(C.sqlite3_errmsg(db)), + ExtendedCode: ErrNoExtended(extrv), + SystemErrno: systemErrno, + err: errStr, } } @@ -869,10 +883,6 @@ func (c *SQLiteConn) begin(ctx context.Context) (driver.Tx, error) { return &SQLiteTx{c}, nil } -func errorString(err Error) string { - return C.GoString(C.sqlite3_errstr(C.int(err.Code))) -} - // Open database and return a new connection. // // A pragma can take either zero or one argument. @@ -1342,10 +1352,13 @@ func (d *SQLiteDriver) Open(dsn string) (driver.Conn, error) { mutex|C.SQLITE_OPEN_READWRITE|C.SQLITE_OPEN_CREATE, nil) if rv != 0 { + // Save off the error _before_ closing the database. + // This is safe even if db is nil. + err := lastError(db) if db != nil { C.sqlite3_close_v2(db) } - return nil, Error{Code: ErrNo(rv)} + return nil, err } if db == nil { return nil, errors.New("sqlite succeeded without returning a database") From c268ded71cc32448f29d53316e6a63a63d4906c6 Mon Sep 17 00:00:00 2001 From: Jesse Rittner Date: Fri, 23 Aug 2019 21:41:14 -0400 Subject: [PATCH 2/3] fix for old versions of libsqlite3 that do not have sqlite3_system_errno defined --- error_test.go | 4 ++++ sqlite3.go | 13 +++++++++++++ 2 files changed, 17 insertions(+) diff --git a/error_test.go b/error_test.go index 49a75b25..f595b238 100644 --- a/error_test.go +++ b/error_test.go @@ -241,6 +241,10 @@ func TestExtendedErrorCodes_Unique(t *testing.T) { } func TestError_SystemErrno(t *testing.T) { + if !hasSystemErrno { + t.Skip("sqlite3_system_errno not supported") + } + // open a non-existent database in read-only mode so we get an IO error. db, err := sql.Open("sqlite3", "file:nonexistent.db?mode=ro") if err != nil { diff --git a/sqlite3.go b/sqlite3.go index 6067206d..f085aaae 100644 --- a/sqlite3.go +++ b/sqlite3.go @@ -182,6 +182,15 @@ static int _sqlite3_limit(sqlite3* db, int limitId, int newLimit) { #else return sqlite3_limit(db, limitId, newLimit); #endif + +#if SQLITE_VERSION_NUMBER >= 3012000 +#define _HAS_SQLITE3_SYSTEM_ERRNO 1 +#else +#define _HAS_SQLITE3_SYSTEM_ERRNO 0 +int sqlite3_system_errno(sqlite3 *db) { + return 0; +} +#endif } */ import "C" @@ -227,6 +236,10 @@ const ( columnTimestamp string = "timestamp" ) +// Indicates whether sqlite3_system_errno is defined in this version of SQLite. +// Needed for unit testing. +const hasSystemErrno = C._HAS_SQLITE3_SYSTEM_ERRNO != 0 + func init() { sql.Register("sqlite3", &SQLiteDriver{}) } From 4581f04151d37a9490c4ed841506145e535da101 Mon Sep 17 00:00:00 2001 From: Jesse Rittner Date: Fri, 23 Aug 2019 21:58:21 -0400 Subject: [PATCH 3/3] fixing pre-processor logic --- error_test.go | 5 +++-- sqlite3.go | 13 +++---------- sqlite3_test.go | 4 ++-- 3 files changed, 8 insertions(+), 14 deletions(-) diff --git a/error_test.go b/error_test.go index f595b238..f8a905e8 100644 --- a/error_test.go +++ b/error_test.go @@ -241,8 +241,9 @@ func TestExtendedErrorCodes_Unique(t *testing.T) { } func TestError_SystemErrno(t *testing.T) { - if !hasSystemErrno { - t.Skip("sqlite3_system_errno not supported") + _, n, _ := Version() + if n < 3012000 { + t.Skip("sqlite3_system_errno requires sqlite3 >= 3.12.0") } // open a non-existent database in read-only mode so we get an IO error. diff --git a/sqlite3.go b/sqlite3.go index f085aaae..db610d31 100644 --- a/sqlite3.go +++ b/sqlite3.go @@ -182,16 +182,13 @@ static int _sqlite3_limit(sqlite3* db, int limitId, int newLimit) { #else return sqlite3_limit(db, limitId, newLimit); #endif +} -#if SQLITE_VERSION_NUMBER >= 3012000 -#define _HAS_SQLITE3_SYSTEM_ERRNO 1 -#else -#define _HAS_SQLITE3_SYSTEM_ERRNO 0 -int sqlite3_system_errno(sqlite3 *db) { +#if SQLITE_VERSION_NUMBER < 3012000 +static int sqlite3_system_errno(sqlite3 *db) { return 0; } #endif -} */ import "C" import ( @@ -236,10 +233,6 @@ const ( columnTimestamp string = "timestamp" ) -// Indicates whether sqlite3_system_errno is defined in this version of SQLite. -// Needed for unit testing. -const hasSystemErrno = C._HAS_SQLITE3_SYSTEM_ERRNO != 0 - func init() { sql.Register("sqlite3", &SQLiteDriver{}) } diff --git a/sqlite3_test.go b/sqlite3_test.go index 806ab8db..d5b7323d 100644 --- a/sqlite3_test.go +++ b/sqlite3_test.go @@ -303,8 +303,8 @@ func TestInsert(t *testing.T) { func TestUpsert(t *testing.T) { _, n, _ := Version() - if !(n >= 3024000) { - t.Skip("UPSERT requires sqlite3 => 3.24.0") + if n < 3024000 { + t.Skip("UPSERT requires sqlite3 >= 3.24.0") } tempFilename := TempFilename(t) defer os.Remove(tempFilename)