blockstore: use errors when API contracts are broken

v2/blockstore/readonly.go

@@ -24,7 +24,10 @@ import (
 
 var _ blockstore.Blockstore = (*ReadOnly)(nil)
 
-var errZeroLengthSection = fmt.Errorf("zero-length section not allowed by default; see WithZeroLengthSectionAsEOF option")
+var (
+	errZeroLengthSection = fmt.Errorf("zero-length carv2 section not allowed by default; see WithZeroLengthSectionAsEOF option")
+	errReadOnly          = fmt.Errorf("called write method on a read-only carv2 blockstore")
+)
 
 type (
 	// ReadOnly provides a read-only CAR Block Store.
@@ -176,7 +179,7 @@ func (b *ReadOnly) readBlock(idx int64) (cid.Cid, []byte, error) {
 
 // DeleteBlock is unsupported and always panics.
 func (b *ReadOnly) DeleteBlock(_ cid.Cid) error {
-	panic("called write method on a read-only blockstore")
+	return errReadOnly
 }
 
 // Has indicates if the store contains a block that corresponds to the given key.
@@ -303,12 +306,12 @@ func (b *ReadOnly) GetSize(key cid.Cid) (int, error) {
 
 // Put is not supported and always returns an error.
 func (b *ReadOnly) Put(blocks.Block) error {
-	panic("called write method on a read-only blockstore")
+	return errReadOnly
 }
 
 // PutMany is not supported and always returns an error.
 func (b *ReadOnly) PutMany([]blocks.Block) error {
-	panic("called write method on a read-only blockstore")
+	return errReadOnly
 }
 
 // WithAsyncErrorHandler returns a context with async error handling set to the given errHandler.

v2/blockstore/readonly_test.go

@@ -102,9 +102,9 @@ func TestReadOnly(t *testing.T) {
 				require.Equal(t, wantBlock, gotBlock)
 
 				// Assert write operations panic
-				require.Panics(t, func() { subject.Put(wantBlock) })
-				require.Panics(t, func() { subject.PutMany([]blocks.Block{wantBlock}) })
-				require.Panics(t, func() { subject.DeleteBlock(key) })
+				require.Error(t, subject.Put(wantBlock))
+				require.Error(t, subject.PutMany([]blocks.Block{wantBlock}))
+				require.Error(t, subject.DeleteBlock(key))
 			}
 
 			ctx, cancel := context.WithTimeout(context.Background(), time.Second*2)

v2/blockstore/readwrite.go

@@ -23,6 +23,8 @@ import (
 
 var _ blockstore.Blockstore = (*ReadWrite)(nil)
 
+var errFinalized = fmt.Errorf("cannot use a read-write carv2 blockstore after finalizing")
+
 // ReadWrite implements a blockstore that stores blocks in CARv2 format.
 // Blocks put into the blockstore can be read back once they are successfully written.
 // This implementation is preferable for a write-heavy workload.
@@ -284,22 +286,22 @@ func (b *ReadWrite) unfinalize() error {
 	return err
 }
 
-func (b *ReadWrite) panicIfFinalized() {
-	if b.header.DataSize != 0 {
-		panic("must not use a read-write blockstore after finalizing")
-	}
+func (b *ReadWrite) finalized() bool {
+	return b.header.DataSize != 0
 }
 
 // Put puts a given block to the underlying datastore
 func (b *ReadWrite) Put(blk blocks.Block) error {
-	// PutMany already calls panicIfFinalized.
+	// PutMany already checks b.finalized.
 	return b.PutMany([]blocks.Block{blk})
 }
 
 // PutMany puts a slice of blocks at the same time using batching
 // capabilities of the underlying datastore whenever possible.
 func (b *ReadWrite) PutMany(blks []blocks.Block) error {
-	b.panicIfFinalized()
+	if b.finalized() {
+		return errFinalized
+	}
 
 	b.mu.Lock()
 	defer b.mu.Unlock()
@@ -362,31 +364,33 @@ func (b *ReadWrite) Finalize() error {
 }
 
 func (b *ReadWrite) AllKeysChan(ctx context.Context) (<-chan cid.Cid, error) {
-	b.panicIfFinalized()
+	if b.finalized() {
+		return nil, errFinalized
+	}
 
 	return b.ReadOnly.AllKeysChan(ctx)
 }
 
 func (b *ReadWrite) Has(key cid.Cid) (bool, error) {
-	b.panicIfFinalized()
+	if b.finalized() {
+		return false, errFinalized
+	}
 
 	return b.ReadOnly.Has(key)
 }
 
 func (b *ReadWrite) Get(key cid.Cid) (blocks.Block, error) {
-	b.panicIfFinalized()
+	if b.finalized() {
+		return nil, errFinalized
+	}
 
 	return b.ReadOnly.Get(key)
 }
 
 func (b *ReadWrite) GetSize(key cid.Cid) (int, error) {
-	b.panicIfFinalized()
+	if b.finalized() {
+		return 0, errFinalized
+	}
 
 	return b.ReadOnly.GetSize(key)
 }
-
-func (b *ReadWrite) HashOnRead(h bool) {
-	b.panicIfFinalized()
-
-	b.ReadOnly.HashOnRead(h)
-}

v2/blockstore/readwrite_test.go

@@ -494,18 +494,24 @@ func TestReadWritePanicsOnlyWhenFinalized(t *testing.T) {
 	require.True(t, has)
 
 	subject.HashOnRead(true)
-	// Delete should always panic regardless of finalize
-	require.Panics(t, func() { subject.DeleteBlock(oneTestBlockCid) })
+	// Delete should always error regardless of finalize
+	require.Error(t, subject.DeleteBlock(oneTestBlockCid))
 
 	require.NoError(t, subject.Finalize())
-	require.Panics(t, func() { subject.Get(oneTestBlockCid) })
-	require.Panics(t, func() { subject.GetSize(anotherTestBlockCid) })
-	require.Panics(t, func() { subject.Has(anotherTestBlockCid) })
-	require.Panics(t, func() { subject.HashOnRead(true) })
-	require.Panics(t, func() { subject.Put(oneTestBlockWithCidV1) })
-	require.Panics(t, func() { subject.PutMany([]blocks.Block{anotherTestBlockWithCidV0}) })
-	require.Panics(t, func() { subject.AllKeysChan(context.Background()) })
-	require.Panics(t, func() { subject.DeleteBlock(oneTestBlockCid) })
+	require.Error(t, subject.Finalize())
+
+	_, err = subject.Get(oneTestBlockCid)
+	require.Error(t, err)
+	_, err = subject.GetSize(anotherTestBlockCid)
+	require.Error(t, err)
+	_, err = subject.Has(anotherTestBlockCid)
+	require.Error(t, err)
+
+	require.Error(t, subject.Put(oneTestBlockWithCidV1))
+	require.Error(t, subject.PutMany([]blocks.Block{anotherTestBlockWithCidV0}))
+	_, err = subject.AllKeysChan(context.Background())
+	require.Error(t, err)
+	require.Error(t, subject.DeleteBlock(oneTestBlockCid))
 }
 
 func TestReadWriteWithPaddingWorksAsExpected(t *testing.T) {