From 6f7b55fdab7c0b38b818dd358bc49556ae79ee1b Mon Sep 17 00:00:00 2001 From: Jonathan Hall Date: Tue, 23 Jul 2024 22:11:48 +0200 Subject: [PATCH 1/2] Handle DB not found errors --- x/sqlite/changes.go | 2 +- x/sqlite/createdoc.go | 2 +- x/sqlite/createdoc_test.go | 1 + x/sqlite/db.go | 19 ++++ x/sqlite/db_test.go | 193 +++++++++++++++++++++++++++++++++++ x/sqlite/delete.go | 2 +- x/sqlite/deleteattachment.go | 2 +- x/sqlite/getrev.go | 2 +- x/sqlite/openrevs.go | 2 +- x/sqlite/purge.go | 2 +- x/sqlite/put.go | 8 +- x/sqlite/putattachment.go | 2 +- x/sqlite/query.go | 4 +- x/sqlite/revsdiff.go | 2 +- x/sqlite/util.go | 4 +- x/sqlite/views.go | 2 +- 16 files changed, 231 insertions(+), 18 deletions(-) diff --git a/x/sqlite/changes.go b/x/sqlite/changes.go index c785d8a72..a9a9706d2 100644 --- a/x/sqlite/changes.go +++ b/x/sqlite/changes.go @@ -189,7 +189,7 @@ func (d *db) newNormalChanges(ctx context.Context, opts optsMap, since, lastSeq c.rows, err = d.db.QueryContext(ctx, query, args...) //nolint:rowserrcheck,sqlclosecheck // Err checked in Next if err != nil { - return nil, err + return nil, d.errDatabaseNotFound(err) } // The first row is used to calculate the ETag; it's done as part of the diff --git a/x/sqlite/createdoc.go b/x/sqlite/createdoc.go index 60c327611..8cf1047af 100644 --- a/x/sqlite/createdoc.go +++ b/x/sqlite/createdoc.go @@ -52,7 +52,7 @@ func (d *db) CreateDoc(ctx context.Context, doc interface{}, _ driver.Options) ( ) `), data.ID).Scan(&exists) if err != nil && !errors.Is(err, sql.ErrNoRows) { - return "", "", err + return "", "", d.errDatabaseNotFound(err) } if exists { return "", "", &kerrors.Error{Status: http.StatusConflict, Message: "document update conflict"} diff --git a/x/sqlite/createdoc_test.go b/x/sqlite/createdoc_test.go index c5ea0a087..b41912bf2 100644 --- a/x/sqlite/createdoc_test.go +++ b/x/sqlite/createdoc_test.go @@ -148,6 +148,7 @@ func TestDBCreateDoc(t *testing.T) { }) /* TODO: + - nil doc - different UUID configuration options???? - retry in case of duplicate random uuid ??? - support batch mode? diff --git a/x/sqlite/db.go b/x/sqlite/db.go index a658e8fd5..f9034553c 100644 --- a/x/sqlite/db.go +++ b/x/sqlite/db.go @@ -15,9 +15,12 @@ package sqlite import ( "context" "database/sql" + "fmt" "log" + "net/http" "github.com/go-kivik/kivik/v4/driver" + internal "github.com/go-kivik/kivik/v4/int/errors" ) type db struct { @@ -43,6 +46,7 @@ func (d *db) Close() error { return d.db.Close() } +// TODO: I think Ping belongs on *client, not *db func (d *db) Ping(ctx context.Context) error { return d.db.PingContext(ctx) } @@ -88,3 +92,18 @@ func (db) DeleteIndex(context.Context, string, string, driver.Options) error { func (db) Explain(context.Context, interface{}, driver.Options) (*driver.QueryPlan, error) { return nil, nil } + +// errDatabaseNotFound converts a sqlite "no such table" error into a kivik +// database not found error +func (d *db) errDatabaseNotFound(err error) error { + if err == nil { + return nil + } + if errIsNoSuchTable(err) { + return &internal.Error{ + Status: http.StatusNotFound, + Message: fmt.Sprintf("database not found: %s", d.name), + } + } + return err +} diff --git a/x/sqlite/db_test.go b/x/sqlite/db_test.go index 49ad81abf..e367b62c1 100644 --- a/x/sqlite/db_test.go +++ b/x/sqlite/db_test.go @@ -16,8 +16,17 @@ package sqlite import ( + "context" "database/sql" + "encoding/json" + "net/http" "testing" + + "gitlab.com/flimzy/testy" + + "github.com/go-kivik/kivik/v4" + "github.com/go-kivik/kivik/v4/driver" + "github.com/go-kivik/kivik/v4/int/mock" ) type leaf struct { @@ -52,3 +61,187 @@ func readRevisions(t *testing.T, db *sql.DB) []leaf { } return leaves } + +func Test_db_not_found(t *testing.T) { + t.Parallel() + const ( + wantStatus = http.StatusNotFound + wantErr = "database not found: db_not_found" + ) + + type test struct { + call func(*db) error + } + + tests := testy.NewTable() + tests.Add("Changes", test{ + call: func(d *db) error { + _, err := d.Changes(context.Background(), mock.NilOption) + return err + }, + }) + tests.Add("Changes, since=now", test{ + call: func(d *db) error { + _, err := d.Changes(context.Background(), kivik.Param("since", "now")) + return err + }, + }) + tests.Add("Changes, longpoll", test{ + call: func(d *db) error { + _, err := d.Changes(context.Background(), kivik.Param("feed", "longpoll")) + return err + }, + }) + tests.Add("CreateDoc", test{ + call: func(d *db) error { + _, _, err := d.CreateDoc(context.Background(), map[string]string{}, mock.NilOption) + return err + }, + }) + tests.Add("DeleteAttachment", test{ + call: func(d *db) error { + _, err := d.DeleteAttachment(context.Background(), "doc", "att", kivik.Rev("1-x")) + return err + }, + }) + tests.Add("Delete", test{ + call: func(d *db) error { + _, err := d.Delete(context.Background(), "doc", kivik.Rev("1-x")) + return err + }, + }) + tests.Add("Find", test{ + call: func(d *db) error { + _, err := d.Find(context.Background(), json.RawMessage(`{"selector":{}}`), mock.NilOption) + return err + }, + }) + tests.Add("GetAttachment", test{ + call: func(d *db) error { + _, err := d.GetAttachment(context.Background(), "doc", "att", mock.NilOption) + return err + }, + }) + tests.Add("GetAttachmentMeta", test{ + call: func(d *db) error { + _, err := d.GetAttachmentMeta(context.Background(), "doc", "att", mock.NilOption) + return err + }, + }) + tests.Add("Get", test{ + call: func(d *db) error { + _, err := d.Get(context.Background(), "doc", mock.NilOption) + return err + }, + }) + tests.Add("GetRev", test{ + call: func(d *db) error { + _, err := d.GetRev(context.Background(), "doc", mock.NilOption) + return err + }, + }) + tests.Add("OpenRevs", test{ + call: func(d *db) error { + _, err := d.OpenRevs(context.Background(), "doc", nil, mock.NilOption) + return err + }, + }) + tests.Add("Purge", test{ + call: func(d *db) error { + _, err := d.Purge(context.Background(), map[string][]string{"doc": {"1-x"}}) + return err + }, + }) + tests.Add("PutAttachment", test{ + call: func(d *db) error { + _, err := d.PutAttachment(context.Background(), "doc", &driver.Attachment{}, mock.NilOption) + return err + }, + }) + tests.Add("Put", test{ + call: func(d *db) error { + _, err := d.Put(context.Background(), "doc", map[string]string{}, mock.NilOption) + return err + }, + }) + tests.Add("Put, new_edits=false", test{ + call: func(d *db) error { + _, err := d.Put(context.Background(), "doc", map[string]any{ + "_rev": "1-x", + }, kivik.Param("new_edits", false)) + return err + }, + }) + tests.Add("Put, new_edits=false + _revisions", test{ + call: func(d *db) error { + _, err := d.Put(context.Background(), "doc", map[string]any{ + "_revisions": map[string]interface{}{ + "start": 1, + "ids": []string{"x"}, + }, + }, kivik.Param("new_edits", false)) + return err + }, + }) + tests.Add("Put, _revisoins + new_edits=true", test{ + call: func(d *db) error { + _, err := d.Put(context.Background(), "doc", map[string]any{ + "_revisions": map[string]interface{}{ + "start": 1, + "ids": []string{"x", "y", "z"}, + }, + }, mock.NilOption) + return err + }, + }) + tests.Add("AllDocs", test{ + call: func(d *db) error { + _, err := d.AllDocs(context.Background(), mock.NilOption) + return err + }, + }) + tests.Add("Query", test{ + call: func(d *db) error { + _, err := d.Query(context.Background(), "ddoc", "view", mock.NilOption) + return err + }, + }) + tests.Add("Query, group=true", test{ + call: func(d *db) error { + _, err := d.Query(context.Background(), "ddoc", "view", kivik.Param("group", true)) + return err + }, + }) + tests.Add("RevsDiff", test{ + call: func(d *db) error { + _, err := d.RevsDiff(context.Background(), map[string][]string{"doc": {"1-x"}}) + return err + }, + }) + /* + TODO: + */ + + tests.Run(t, func(t *testing.T, tt test) { + t.Parallel() + client, err := (drv{}).NewClient(":memory:", mock.NilOption) + if err != nil { + t.Fatal(err) + } + d, err := client.DB("db_not_found", mock.NilOption) + if err != nil { + t.Fatal(err) + } + + err = tt.call(d.(*db)) + if err == nil { + t.Fatal("Expected error") + } + if wantErr != err.Error() { + t.Errorf("Unexpected error: %s", err) + } + if status := kivik.HTTPStatus(err); status != wantStatus { + t.Errorf("Unexpected status: %d", status) + } + }) +} diff --git a/x/sqlite/delete.go b/x/sqlite/delete.go index 465d40c43..4b347ffd7 100644 --- a/x/sqlite/delete.go +++ b/x/sqlite/delete.go @@ -48,7 +48,7 @@ func (d *db) Delete(ctx context.Context, docID string, options driver.Options) ( data.MD5sum, err = d.isLeafRev(ctx, tx, docID, curRev.rev, curRev.id) if err != nil { - return "", err + return "", d.errDatabaseNotFound(err) } data.Deleted = true data.Doc = []byte("{}") diff --git a/x/sqlite/deleteattachment.go b/x/sqlite/deleteattachment.go index 1b64a3664..fcb096ff5 100644 --- a/x/sqlite/deleteattachment.go +++ b/x/sqlite/deleteattachment.go @@ -45,7 +45,7 @@ func (d *db) DeleteAttachment(ctx context.Context, docID, filename string, optio data.MD5sum, err = d.isLeafRev(ctx, tx, docID, curRev.rev, curRev.id) if err != nil { - return "", err + return "", d.errDatabaseNotFound(err) } // Read list of current attachments, then remove the requested one diff --git a/x/sqlite/getrev.go b/x/sqlite/getrev.go index 343009d65..d67b3d8b9 100644 --- a/x/sqlite/getrev.go +++ b/x/sqlite/getrev.go @@ -133,7 +133,7 @@ func (d *db) getCoreDoc(ctx context.Context, tx queryer, id string, rev revision case errors.Is(err, sql.ErrNoRows) || deleted && rev.IsZero(): return nil, revision{}, &internal.Error{Status: http.StatusNotFound, Message: "not found"} case err != nil: - return nil, revision{}, err + return nil, revision{}, d.errDatabaseNotFound(err) } return &fullDoc{ diff --git a/x/sqlite/openrevs.go b/x/sqlite/openrevs.go index 7bf42bac0..4630a243c 100644 --- a/x/sqlite/openrevs.go +++ b/x/sqlite/openrevs.go @@ -167,7 +167,7 @@ func (d *db) OpenRevs(ctx context.Context, docID string, revs []string, options `), strings.Join(values, ", ")) rows, err := d.db.QueryContext(ctx, query, args...) //nolint:rowserrcheck // Err checked in Next if err != nil { - return nil, err + return nil, d.errDatabaseNotFound(err) } // Call rows.Next() to see if we get any results at all. If zero results, diff --git a/x/sqlite/purge.go b/x/sqlite/purge.go index cff198ef3..0bdfbc55b 100644 --- a/x/sqlite/purge.go +++ b/x/sqlite/purge.go @@ -44,7 +44,7 @@ func (d *db) Purge(ctx context.Context, request map[string][]string) (*driver.Pu // Non-leaf rev, do nothing continue default: - return nil, err + return nil, d.errDatabaseNotFound(err) } } diff --git a/x/sqlite/put.go b/x/sqlite/put.go index 37eedf4a8..895688bf4 100644 --- a/x/sqlite/put.go +++ b/x/sqlite/put.go @@ -62,7 +62,7 @@ func (d *db) Put(ctx context.Context, docID string, doc interface{}, options dri for _, r := range revs[:len(revs)-1] { err := stmt.QueryRowContext(ctx, data.ID, r.rev, r.id).Scan(&exists) if err != nil { - return "", err + return "", d.errDatabaseNotFound(err) } if !exists { return "", &internal.Error{Status: http.StatusConflict, Message: "document update conflict"} @@ -101,7 +101,7 @@ func (d *db) Put(ctx context.Context, docID string, doc interface{}, options dri r := r _, err := stmt.ExecContext(ctx, data.ID, r.rev, r.id, parentRev, parentRevID) if err != nil { - return "", err + return "", d.errDatabaseNotFound(err) } parentRev = &r.rev parentRevID = &r.id @@ -121,7 +121,7 @@ func (d *db) Put(ctx context.Context, docID string, doc interface{}, options dri ON CONFLICT DO NOTHING `), docID, rev.rev, rev.id) if err != nil { - return "", err + return "", d.errDatabaseNotFound(err) } } var newRev string @@ -163,7 +163,7 @@ func (d *db) Put(ctx context.Context, docID string, doc interface{}, options dri return "", &internal.Error{Status: http.StatusConflict, Message: "document update conflict"} } case err != nil: - return "", err + return "", d.errDatabaseNotFound(err) } r, err := d.createRev(ctx, tx, data, curRev) diff --git a/x/sqlite/putattachment.go b/x/sqlite/putattachment.go index a649150b3..e9046bc1e 100644 --- a/x/sqlite/putattachment.go +++ b/x/sqlite/putattachment.go @@ -53,7 +53,7 @@ func (d *db) PutAttachment(ctx context.Context, docID string, att *driver.Attach // This means the doc is being created, and is empty other than the attachment data.Doc = []byte("{}") case err != nil: - return "", err + return "", d.errDatabaseNotFound(err) } content, err := io.ReadAll(att.Content) diff --git a/x/sqlite/query.go b/x/sqlite/query.go index d8be9d111..55e477834 100644 --- a/x/sqlite/query.go +++ b/x/sqlite/query.go @@ -107,7 +107,7 @@ func (d *db) performQuery( for { rev, err := d.updateIndex(ctx, ddoc, view, vopts.update) if err != nil { - return nil, err + return nil, d.errDatabaseNotFound(err) } args := []interface{}{ @@ -304,7 +304,7 @@ func (d *db) performGroupQuery(ctx context.Context, ddoc, view string, vopts *vi for { rev, err = d.updateIndex(ctx, ddoc, view, vopts.update) if err != nil { - return nil, err + return nil, d.errDatabaseNotFound(err) } args := []any{"_design/" + ddoc, rev.rev, rev.id, view, kivik.EndKeySuffix, true, vopts.updateSeq} diff --git a/x/sqlite/revsdiff.go b/x/sqlite/revsdiff.go index a27bcac2e..3985a5ea8 100644 --- a/x/sqlite/revsdiff.go +++ b/x/sqlite/revsdiff.go @@ -67,7 +67,7 @@ func (d *db) RevsDiff(ctx context.Context, revMap interface{}) (driver.Rows, err rows, err := d.db.QueryContext(ctx, query, ids...) //nolint:rowserrcheck // Err checked in Next if err != nil { - return nil, err + return nil, d.errDatabaseNotFound(err) } return &revsDiffResponse{ diff --git a/x/sqlite/util.go b/x/sqlite/util.go index d7fd2627f..aac6857ee 100644 --- a/x/sqlite/util.go +++ b/x/sqlite/util.go @@ -93,7 +93,7 @@ func (d *db) winningRev(ctx context.Context, tx queryer, docID string) (revision if err != nil && errors.Is(err, sql.ErrNoRows) { return curRev, &internal.Error{Status: http.StatusNotFound, Message: "missing"} } - return curRev, err + return curRev, d.errDatabaseNotFound(err) } type stmtCache map[string]*sql.Stmt @@ -251,7 +251,7 @@ func (d *db) lastSeq(ctx context.Context) (uint64, error) { err := d.db.QueryRowContext(ctx, d.query(` SELECT COALESCE(MAX(seq), 0) FROM {{ .Docs }} `)).Scan(&lastSeq) - return lastSeq, err + return lastSeq, d.errDatabaseNotFound(err) } // discard implements the [database/sql.Scanner] interface, but discards the diff --git a/x/sqlite/views.go b/x/sqlite/views.go index e06bc43b3..1cdbea7dc 100644 --- a/x/sqlite/views.go +++ b/x/sqlite/views.go @@ -191,7 +191,7 @@ func (d *db) queryBuiltinView( `), vopts.buildOrderBy(), strings.Join(where, " AND "), vopts.limit, vopts.skip, vopts.bookmarkWhere()) results, err := d.db.QueryContext(ctx, query, args...) //nolint:rowserrcheck // Err checked in Next if err != nil { - return nil, err + return nil, d.errDatabaseNotFound(err) } meta, err := readFirstRow(results, vopts) From 764e41e36c25ab2436960ea391e2aa4adbf71f4e Mon Sep 17 00:00:00 2001 From: Jonathan Hall Date: Tue, 23 Jul 2024 22:54:42 +0200 Subject: [PATCH 2/2] Revert inappropriate typo fix --- pouchdb/find_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pouchdb/find_test.go b/pouchdb/find_test.go index 22d33accb..9bf9d5448 100644 --- a/pouchdb/find_test.go +++ b/pouchdb/find_test.go @@ -80,7 +80,7 @@ func TestExplain(t *testing.T) { tests.Add("query error", test{ db: &db{db: bindings.GlobalPouchDB().New("foo", nil)}, query: nil, - err: "TypeError: Cannot read properties", + err: "TypeError: Cannot read propert", }) tests.Add("simple selector", func(t *testing.T) interface{} { options := map[string]interface{}{