improve error handling (#98)

* remove pkg/errors

* don't panic in RemoteDB.Stats()

* fix some panics

* update tests

* changelog tweaks

CHANGELOG.md

@@ -4,16 +4,19 @@
 
 ### Breaking Changes
 
-- Batch `Set()`, `Delete()`, and `Close()` may now error (@erikgrinaker)
+- [\#98](https://github.com/tendermint/tm-db/pull/98) `NewDB` now returns an error instead of panicing (@erikgrinaker)
 
-- `Iterator.Close()` may now error (@erikgrinaker)
+- [\#96](https://github.com/tendermint/tm-db/pull/96) `Batch.Set()`, `Delete()`, and `Close()` may now error (@erikgrinaker)
 
-- Many iterator panics are now exposed via `Error()` instead (@erikgrinaker)
+- [\#97](https://github.com/tendermint/tm-db/pull/97) `Iterator.Close()` may now error (@erikgrinaker)
 
-- `RemoteDB` iterators are now correctly primed with the first item when created, without calling
-  `Next()` (@erikgrinaker)
+- [\#97](https://github.com/tendermint/tm-db/pull/97) Many iterator panics are now exposed via `Error()` instead (@erikgrinaker)
 
-- The `SetDeleter` interface has been removed (@erikgrinaker)
+- [\#96](https://github.com/tendermint/tm-db/pull/96) The `SetDeleter` interface has been removed (@erikgrinaker)
+
+### Bug Fixes
+
+- [\#97](https://github.com/tendermint/tm-db/pull/97) `RemoteDB` iterators are now correctly primed with the first item when created, without calling `Next()` (@erikgrinaker)
 
 ## 0.5.1
 

backend_test.go

@@ -37,7 +37,8 @@ func testBackendGetSetDelete(t *testing.T, backend BackendType) {
 	// Default
 	dirname, err := ioutil.TempDir("", fmt.Sprintf("test_backend_%s_", backend))
 	require.Nil(t, err)
-	db := NewDB("testdb", backend, dirname)
+	db, err := NewDB("testdb", backend, dirname)
+	require.NoError(t, err)
 	defer cleanupDBDir(dirname, "testdb")
 
 	// A nonexistent key should return nil, even if the key is empty
@@ -212,7 +213,8 @@ func TestBackendsNilKeys(t *testing.T) {
 
 func TestGoLevelDBBackend(t *testing.T) {
 	name := fmt.Sprintf("test_%x", randStr(12))
-	db := NewDB(name, GoLevelDBBackend, "")
+	db, err := NewDB(name, GoLevelDBBackend, "")
+	require.NoError(t, err)
 	defer cleanupDBDir("", name)
 
 	_, ok := db.(*GoLevelDB)
@@ -231,7 +233,8 @@ func TestDBIterator(t *testing.T) {
 func testDBIterator(t *testing.T, backend BackendType) {
 	name := fmt.Sprintf("test_%x", randStr(12))
 	dir := os.TempDir()
-	db := NewDB(name, backend, dir)
+	db, err := NewDB(name, backend, dir)
+	require.NoError(t, err)
 	defer cleanupDBDir(dir, name)
 
 	for i := 0; i < 10; i++ {
@@ -366,10 +369,11 @@ func testDBIterator(t *testing.T, backend BackendType) {
 func testDBIteratorBlankKey(t *testing.T, backend BackendType) {
 	name := fmt.Sprintf("test_%x", randStr(12))
 	dir := os.TempDir()
-	db := NewDB(name, backend, dir)
+	db, err := NewDB(name, backend, dir)
+	require.NoError(t, err)
 	defer cleanupDBDir(dir, name)
 
-	err := db.Set([]byte(""), []byte{0})
+	err = db.Set([]byte(""), []byte{0})
 	require.NoError(t, err)
 	err = db.Set([]byte("a"), []byte{1})
 	require.NoError(t, err)
@@ -444,7 +448,8 @@ func TestDBBatch(t *testing.T) {
 func testDBBatch(t *testing.T, backend BackendType) {
 	name := fmt.Sprintf("test_%x", randStr(12))
 	dir := os.TempDir()
-	db := NewDB(name, backend, dir)
+	db, err := NewDB(name, backend, dir)
+	require.NoError(t, err)
 	defer cleanupDBDir(dir, name)
 
 	// create a new batch, and some items - they should not be visible until we write
@@ -454,7 +459,7 @@ func testDBBatch(t *testing.T, backend BackendType) {
 	require.NoError(t, batch.Set([]byte("c"), []byte{3}))
 	assertKeyValues(t, db, map[string][]byte{})
 
-	err := batch.Write()
+	err = batch.Write()
 	require.NoError(t, err)
 	assertKeyValues(t, db, map[string][]byte{"a": {1}, "b": {2}, "c": {3}})
 

boltdb.go

@@ -3,11 +3,11 @@
 package db
 
 import (
+	"errors"
 	"fmt"
 	"os"
 	"path/filepath"
 
-	"github.com/pkg/errors"
 	"go.etcd.io/bbolt"
 )
 

cleveldb_test.go

@@ -10,6 +10,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func BenchmarkRandomReadsWrites2(b *testing.B) {
@@ -83,7 +84,8 @@ func TestCLevelDBBackend(t *testing.T) {
 	// Can't use "" (current directory) or "./" here because levigo.Open returns:
 	// "Error initializing DB: IO error: test_XXX.db: Invalid argument"
 	dir := os.TempDir()
-	db := NewDB(name, CLevelDBBackend, dir)
+	db, err := NewDB(name, CLevelDBBackend, dir)
+	require.NoError(t, err)
 	defer cleanupDBDir(dir, name)
 
 	_, ok := db.(*CLevelDB)
@@ -93,7 +95,8 @@ func TestCLevelDBBackend(t *testing.T) {
 func TestCLevelDBStats(t *testing.T) {
 	name := fmt.Sprintf("test_%x", randStr(12))
 	dir := os.TempDir()
-	db := NewDB(name, CLevelDBBackend, dir)
+	db, err := NewDB(name, CLevelDBBackend, dir)
+	require.NoError(t, err)
 	defer cleanupDBDir(dir, name)
 
 	assert.NotEmpty(t, db.Stats())

common_test.go

@@ -68,8 +68,10 @@ func checkValuePanics(t *testing.T, itr Iterator) {
 
 func newTempDB(t *testing.T, backend BackendType) (db DB, dbDir string) {
 	dirname, err := ioutil.TempDir("", "db_common_test")
-	require.Nil(t, err)
-	return NewDB("testdb", backend, dirname), dirname
+	require.NoError(t, err)
+	db, err = NewDB("testdb", backend, dirname)
+	require.NoError(t, err)
+	return db, dirname
 }
 
 func benchmarkRangeScans(b *testing.B, db DB, dbSize int64) {

db.go

@@ -48,24 +48,20 @@ func registerDBCreator(backend BackendType, creator dbCreator, force bool) {
 }
 
 // NewDB creates a new database of type backend with the given name.
-// NOTE: function panics if:
-//   - backend is unknown (not registered)
-//   - creator function, provided during registration, returns error
-func NewDB(name string, backend BackendType, dir string) DB {
+func NewDB(name string, backend BackendType, dir string) (DB, error) {
 	dbCreator, ok := backends[backend]
 	if !ok {
-		keys := make([]string, len(backends))
-		i := 0
+		keys := make([]string, 0, len(backends))
 		for k := range backends {
-			keys[i] = string(k)
-			i++
+			keys = append(keys, string(k))
 		}
-		panic(fmt.Sprintf("Unknown db_backend %s, expected either %s", backend, strings.Join(keys, " or ")))
+		return nil, fmt.Errorf("unknown db_backend %s, expected one of %v",
+			backend, strings.Join(keys, ","))
 	}
 
 	db, err := dbCreator(name, dir)
 	if err != nil {
-		panic(fmt.Sprintf("Error initializing DB: %v", err))
+		return nil, fmt.Errorf("failed to initialize database: %w", err)
 	}
-	return db
+	return db, nil
 }

go.mod

@@ -10,7 +10,6 @@ require (
 	github.com/gogo/protobuf v1.3.1
 	github.com/google/btree v1.0.0
 	github.com/jmhodges/levigo v1.0.0
-	github.com/pkg/errors v0.9.1
 	github.com/stretchr/testify v1.5.1
 	github.com/syndtr/goleveldb v1.0.1-0.20190923125748-758128399b1d
 	github.com/tecbot/gorocksdb v0.0.0-20191217155057-f0fad39f321c

go.sum

@@ -45,8 +45,6 @@ github.com/onsi/ginkgo v1.7.0 h1:WSHQ+IS43OoUrWtD1/bbclrwK8TTH5hzp+umCiuxHgs=
 github.com/onsi/ginkgo v1.7.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE=
 github.com/onsi/gomega v1.4.3 h1:RE1xgDvH7imwFD45h+u2SgIfERHlS2yNG4DObb5BSKU=
 github.com/onsi/gomega v1.4.3/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1CpauHY=
-github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
-github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
 github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA=
@@ -100,10 +98,6 @@ google.golang.org/genproto v0.0.0-20190819201941-24fa4b261c55/go.mod h1:DMBHOl98
 google.golang.org/grpc v1.19.0/go.mod h1:mqu4LbDTu4XGKhr4mRzUsmM4RtVoemTSY81AxZiDr8c=
 google.golang.org/grpc v1.23.0/go.mod h1:Y5yQAOtifL1yxbo5wqy6BxZv8vAUGQwXBOALyacEbxg=
 google.golang.org/grpc v1.25.1/go.mod h1:c3i+UQWmh7LiEpx4sFZnkU36qjEYZ0imhYfXVyQciAY=
-google.golang.org/grpc v1.28.1 h1:C1QC6KzgSiLyBabDi87BbjaGreoRgGUF5nOyvfrAZ1k=
-google.golang.org/grpc v1.28.1/go.mod h1:rpkK4SK4GF4Ach/+MFLZUBavHOvF2JJB5uozKKal+60=
-google.golang.org/grpc v1.29.0 h1:2pJjwYOdkZ9HlN4sWRYBg9ttH5bCOlsueaM+b/oYjwo=
-google.golang.org/grpc v1.29.0/go.mod h1:itym6AZVZYACWQqET3MqgPpjcuV5QH3BxFS3IjizoKk=
 google.golang.org/grpc v1.29.1 h1:EC2SB8S04d2r73uptxphDSUG+kTKVgjRPF+N3xpxRB4=
 google.golang.org/grpc v1.29.1/go.mod h1:itym6AZVZYACWQqET3MqgPpjcuV5QH3BxFS3IjizoKk=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=

memdb_batch.go

@@ -1,6 +1,6 @@
 package db
 
-import "github.com/pkg/errors"
+import "fmt"
 
 // memDBBatch operations
 type opType int
@@ -65,7 +65,7 @@ func (b *memDBBatch) Write() error {
 		case opTypeDelete:
 			b.db.delete(op.key)
 		default:
-			return errors.Errorf("unknown operation type %v (%v)", op.opType, op)
+			return fmt.Errorf("unknown operation type %v (%v)", op.opType, op)
 		}
 	}
 

remotedb/batch.go

@@ -1,7 +1,8 @@
 package remotedb
 
 import (
-	"github.com/pkg/errors"
+	"errors"
+	"fmt"
 
 	db "github.com/tendermint/tm-db"
 	protodb "github.com/tendermint/tm-db/remotedb/proto"
@@ -56,7 +57,7 @@ func (b *batch) Write() error {
 	}
 	_, err := b.db.dc.BatchWrite(b.db.ctx, &protodb.Batch{Ops: b.ops})
 	if err != nil {
-		return errors.Errorf("remoteDB.BatchWrite: %v", err)
+		return fmt.Errorf("remoteDB.BatchWrite: %w", err)
 	}
 	// Make sure batch cannot be used afterwards. Callers should still call Close(), for errors.
 	b.Close()
@@ -70,7 +71,7 @@ func (b *batch) WriteSync() error {
 	}
 	_, err := b.db.dc.BatchWriteSync(b.db.ctx, &protodb.Batch{Ops: b.ops})
 	if err != nil {
-		return errors.Errorf("RemoteDB.BatchWriteSync: %v", err)
+		return fmt.Errorf("RemoteDB.BatchWriteSync: %w", err)
 	}
 	// Make sure batch cannot be used afterwards. Callers should still call Close(), for errors.
 	return b.Close()

remotedb/grpcdb/server.go

@@ -63,7 +63,11 @@ func (s *server) Init(ctx context.Context, in *protodb.Init) (*protodb.Entity, e
 	s.mu.Lock()
 	defer s.mu.Unlock()
 
-	s.db = db.NewDB(in.Name, db.BackendType(in.Type), in.Dir)
+	var err error
+	s.db, err = db.NewDB(in.Name, db.BackendType(in.Type), in.Dir)
+	if err != nil {
+		return nil, err
+	}
 	return &protodb.Entity{CreatedAt: time.Now().Unix()}, nil
 }
 

remotedb/remotedb.go

@@ -2,10 +2,9 @@ package remotedb
 
 import (
 	"context"
+	"errors"
 	"fmt"
 
-	"github.com/pkg/errors"
-
 	db "github.com/tendermint/tm-db"
 	"github.com/tendermint/tm-db/remotedb/grpcdb"
 	protodb "github.com/tendermint/tm-db/remotedb/proto"
@@ -47,36 +46,36 @@ func (rd *RemoteDB) Close() error {
 
 func (rd *RemoteDB) Delete(key []byte) error {
 	if _, err := rd.dc.Delete(rd.ctx, &protodb.Entity{Key: key}); err != nil {
-		return errors.Errorf("remoteDB.Delete: %v", err)
+		return fmt.Errorf("remoteDB.Delete: %w", err)
 	}
 	return nil
 }
 
 func (rd *RemoteDB) DeleteSync(key []byte) error {
 	if _, err := rd.dc.DeleteSync(rd.ctx, &protodb.Entity{Key: key}); err != nil {
-		return errors.Errorf("remoteDB.DeleteSync: %v", err)
+		return fmt.Errorf("remoteDB.DeleteSync: %w", err)
 	}
 	return nil
 }
 
 func (rd *RemoteDB) Set(key, value []byte) error {
 	if _, err := rd.dc.Set(rd.ctx, &protodb.Entity{Key: key, Value: value}); err != nil {
-		return errors.Errorf("remoteDB.Set: %v", err)
+		return fmt.Errorf("remoteDB.Set: %w", err)
 	}
 	return nil
 }
 
 func (rd *RemoteDB) SetSync(key, value []byte) error {
 	if _, err := rd.dc.SetSync(rd.ctx, &protodb.Entity{Key: key, Value: value}); err != nil {
-		return errors.Errorf("remoteDB.SetSync: %v", err)
+		return fmt.Errorf("remoteDB.SetSync: %w", err)
 	}
 	return nil
 }
 
 func (rd *RemoteDB) Get(key []byte) ([]byte, error) {
 	res, err := rd.dc.Get(rd.ctx, &protodb.Entity{Key: key})
 	if err != nil {
-		return nil, errors.Errorf("remoteDB.Get error: %v", err)
+		return nil, fmt.Errorf("remoteDB.Get error: %w", err)
 	}
 	return res.Value, nil
 }
@@ -109,10 +108,7 @@ func (rd *RemoteDB) Print() error {
 
 func (rd *RemoteDB) Stats() map[string]string {
 	stats, err := rd.dc.Stats(rd.ctx, &protodb.Nothing{})
-	if err != nil {
-		panic(fmt.Sprintf("RemoteDB.Stats error: %v", err))
-	}
-	if stats == nil {
+	if err != nil || stats == nil {
 		return nil
 	}
 	return stats.Data

rocksdb_test.go

@@ -8,12 +8,14 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestRocksDBBackend(t *testing.T) {
 	name := fmt.Sprintf("test_%x", randStr(12))
 	dir := os.TempDir()
-	db := NewDB(name, RocksDBBackend, dir)
+	db, err := NewDB(name, RocksDBBackend, dir)
+	require.NoError(t, err)
 	defer cleanupDBDir(dir, name)
 
 	_, ok := db.(*RocksDB)
@@ -23,7 +25,8 @@ func TestRocksDBBackend(t *testing.T) {
 func TestRocksDBStats(t *testing.T) {
 	name := fmt.Sprintf("test_%x", randStr(12))
 	dir := os.TempDir()
-	db := NewDB(name, RocksDBBackend, dir)
+	db, err := NewDB(name, RocksDBBackend, dir)
+	require.NoError(t, err)
 	defer cleanupDBDir(dir, name)
 
 	assert.NotEmpty(t, db.Stats())