[block-store] return ErrNotFound instead of default error type

Currently, the committer sends NotFound grpc error code for block APIs
by parsing the error message. As this is fragile, this commit adds
ErrNotFound error so that the committer can use errors.Is().

Signed-off-by: Senthilnathan <cendhu@gmail.com>

Author: cendhu

common/ledger/blkstorage/blockindex.go

@@ -14,7 +14,7 @@ import (
 
 	"github.com/hyperledger/fabric-protos-go-apiv2/common"
 	"github.com/hyperledger/fabric-protos-go-apiv2/peer"
-	"github.com/pkg/errors"
+	"github.com/cockroachdb/errors"
 	"google.golang.org/protobuf/encoding/protowire"
 	"google.golang.org/protobuf/proto"
 
@@ -40,6 +40,7 @@ var (
 	indexSavePointKey              = []byte(indexSavePointKeyStr)
 	errIndexSavePointKeyNotPresent = errors.New("NoBlockIndexed")
 	errNilValue                    = errors.New("")
+	ErrNotFound                    = errors.New("not found in index")
 	importTxIDsBatchSize           = uint64(10000) // txID is 64 bytes, so batch size roughly translates to 640KB
 )
 
@@ -157,7 +158,7 @@ func (index *blockIndex) getBlockLocByHash(blockHash []byte) (*fileLocPointer, e
 		return nil, err
 	}
 	if b == nil {
-		return nil, errors.Errorf("no such block hash [%x] in index", blockHash)
+		return nil, errors.Wrapf(ErrNotFound, "block hash [%x]", blockHash)
 	}
 	blkLoc := &fileLocPointer{}
 	if err := blkLoc.unmarshal(b); err != nil {
@@ -175,7 +176,7 @@ func (index *blockIndex) getBlockLocByBlockNum(blockNum uint64) (*fileLocPointer
 		return nil, err
 	}
 	if b == nil {
-		return nil, errors.Errorf("no such block number [%d] in index", blockNum)
+		return nil, errors.Wrapf(ErrNotFound, "block number [%d]", blockNum)
 	}
 	blkLoc := &fileLocPointer{}
 	if err := blkLoc.unmarshal(b); err != nil {
@@ -250,7 +251,7 @@ func (index *blockIndex) getTxIDVal(txID string) (*TxIDIndexValue, uint64, error
 		return nil, 0, errors.Wrapf(err, "error while trying to retrieve transaction info by TXID [%s]", txID)
 	}
 	if !present {
-		return nil, 0, errors.Errorf("no such transaction ID [%s] in index", txID)
+		return nil, 0, errors.Wrapf(ErrNotFound, "transaction ID [%s]", txID)
 	}
 	valBytes := itr.Value()
 	if len(valBytes) == 0 {
@@ -276,7 +277,7 @@ func (index *blockIndex) getTXLocByBlockNumTranNum(blockNum uint64, tranNum uint
 		return nil, err
 	}
 	if b == nil {
-		return nil, errors.Errorf("no such blockNumber, transactionNumber <%d, %d> in index", blockNum, tranNum)
+		return nil, errors.Wrapf(ErrNotFound, "blockNumber, transactionNumber <%d, %d>", blockNum, tranNum)
 	}
 	txFLP := &fileLocPointer{}
 	if err := txFLP.unmarshal(b); err != nil {

common/ledger/blkstorage/blockindex_test.go

@@ -67,7 +67,7 @@ func testBlockIndexSync(t *testing.T, numBlocks int, numBlocksToIndex int, syncB
 		// Before, we test for index sync-up, verify that the last set of blocks not indexed in the original index
 		for i := numBlocksToIndex + 1; i <= numBlocks; i++ {
 			_, err := blkfileMgr.retrieveBlockByNumber(uint64(i))
-			require.EqualError(t, err, fmt.Sprintf("no such block number [%d] in index", i))
+			requireNotFoundError(t, err, fmt.Sprintf("block number [%d]", i))
 		}
 
 		// perform index sync

common/ledger/blkstorage/blockstore_provider_test.go

@@ -143,24 +143,24 @@ func checkBlocks(t *testing.T, expectedBlocks []*common.Block, store *BlockStore
 func checkWithWrongInputs(t *testing.T, store *BlockStore, numBlocks int) {
 	block, err := store.RetrieveBlockByHash([]byte("non-existent-hash"))
 	require.Nil(t, block)
-	require.EqualError(t, err, fmt.Sprintf("no such block hash [%x] in index", []byte("non-existent-hash")))
+	requireNotFoundError(t, err, "block hash")
 
 	block, err = store.RetrieveBlockByTxID("non-existent-txid")
 	require.Nil(t, block)
-	require.EqualError(t, err, "no such transaction ID [non-existent-txid] in index")
+	requireNotFoundError(t, err, "transaction ID [non-existent-txid]")
 
 	tx, err := store.RetrieveTxByID("non-existent-txid")
 	require.Nil(t, tx)
-	require.EqualError(t, err, "no such transaction ID [non-existent-txid] in index")
+	requireNotFoundError(t, err, "transaction ID [non-existent-txid]")
 
 	tx, err = store.RetrieveTxByBlockNumTranNum(uint64(numBlocks+1), uint64(0))
 	require.Nil(t, tx)
-	require.EqualError(t, err, fmt.Sprintf("no such blockNumber, transactionNumber <%d, 0> in index", numBlocks+1))
+	requireNotFoundError(t, err, fmt.Sprintf("blockNumber, transactionNumber <%d, 0>", numBlocks+1))
 
 	txCode, blkNum, err := store.RetrieveTxValidationCodeByTxID("non-existent-txid")
 	require.Equal(t, peer.TxValidationCode(-1), txCode)
 	require.Equal(t, uint64(0), blkNum)
-	require.EqualError(t, err, "no such transaction ID [non-existent-txid] in index")
+	requireNotFoundError(t, err, "transaction ID [non-existent-txid]")
 }
 
 func TestBlockStoreProvider(t *testing.T) {

common/ledger/blkstorage/pkg_test.go

@@ -113,7 +113,7 @@ func (w *testBlockfileMgrWrapper) testGetBlockByHashNotIndexed(blocks []*common.
 	for _, block := range blocks {
 		hash := protoutil.BlockHeaderHash(block.Header)
 		_, err := w.blockfileMgr.retrieveBlockByHash(hash)
-		require.EqualError(w.t, err, fmt.Sprintf("no such block hash [%x] in index", hash))
+		requireNotFoundError(w.t, err, "block hash")
 	}
 }
 
@@ -123,7 +123,7 @@ func (w *testBlockfileMgrWrapper) testGetBlockByTxIDNotIndexed(blocks []*common.
 			txID, err := protoutil.GetOrComputeTxIDFromEnvelope(txEnv)
 			require.NoError(w.t, err)
 			_, err = w.blockfileMgr.retrieveBlockByTxID(txID)
-			require.EqualError(w.t, err, fmt.Sprintf("no such transaction ID [%s] in index", txID))
+			requireNotFoundError(w.t, err, fmt.Sprintf("transaction ID [%s]", txID))
 		}
 	}
 }

common/ledger/blkstorage/reset_test.go

@@ -236,7 +236,7 @@ func assertBlockStorePostReset(t *testing.T, store *BlockStore, originallyCommit
 	require.Equal(t, originallyCommittedBlocks[0], blk)
 
 	_, err = store.RetrieveBlockByNumber(1)
-	require.EqualError(t, err, "no such block number [1] in index")
+	requireNotFoundError(t, err, "block number [1]")
 
 	err = store.AddBlock(originallyCommittedBlocks[0])
 	require.EqualError(t, err, "block number should have been 1 but was 0")

common/ledger/blkstorage/snapshot_test.go

@@ -285,7 +285,7 @@ func TestImportFromSnapshot(t *testing.T) {
 
 			// before, we test for index sync-up, verify that the last set of blocks not indexed in the original index
 			_, err := blkfileMgr.retrieveBlockByNumber(block.Header.Number)
-			require.EqualError(t, err, fmt.Sprintf("no such block number [%d] in index", block.Header.Number))
+			requireNotFoundError(t, err, fmt.Sprintf("block number [%d]", block.Header.Number))
 
 			// close and open should be able to sync-up the index
 			closeBlockStore()
@@ -472,7 +472,7 @@ func verifyQueriesOnBlocksPriorToSnapshot(
 		require.EqualError(t, err, expectedErrStr)
 
 		_, err = bootstrappedBlockStore.RetrieveBlockByHash(blockHash)
-		require.EqualError(t, err, fmt.Sprintf("no such block hash [%x] in index", blockHash))
+		requireNotFoundError(t, err, "block hash")
 	}
 
 	bootstrappingSnapshotHeight := uint64(len(blocksDetailsBeforeSnapshot))

common/ledger/blkstorage/test_utils.go

@@ -0,0 +1,21 @@
+/*
+Copyright IBM Corp. All Rights Reserved.
+
+SPDX-License-Identifier: Apache-2.0
+*/
+
+package blkstorage
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+// requireNotFoundError is a test helper that verifies an error is ErrNotFound
+// and contains the expected context string
+func requireNotFoundError(t testing.TB, err error, expectedContext string) {
+	t.Helper()
+	require.ErrorIs(t, err, ErrNotFound)
+	require.Contains(t, err.Error(), expectedContext)
+}