chore: refactor panics into error handling (#6345)

## Motivation Updated some places where `log.Panic` is used and replaced it by returning the error, callers already handle them.
spacemeshos · Sep 24, 2024 · 94db9f8 · 94db9f8
1 parent 0e84c0a
commit 94db9f8
Show file tree

Hide file tree

Showing 19 changed files with 118 additions and 58 deletions.
diff --git a/activation/e2e/activation_test.go b/activation/e2e/activation_test.go
@@ -117,7 +117,8 @@ func Test_BuilderWithMultipleClients(t *testing.T) {
 	)
 	certClient := activation.NewCertifierClient(db, localDB, logger.Named("certifier"))
 	certifier := activation.NewCertifier(localDB, logger, certClient)
-	poetDb := activation.NewPoetDb(db, logger.Named("poetDb"))
+	poetDb, err := activation.NewPoetDb(db, logger.Named("poetDb"))
+	require.NoError(t, err)
 	client, err := poetProver.Client(poetDb, poetCfg, logger, activation.WithCertifier(certifier))
 	require.NoError(t, err)
 

diff --git a/activation/e2e/atx_merge_test.go b/activation/e2e/atx_merge_test.go
@@ -220,7 +220,8 @@ func Test_MarryAndMerge(t *testing.T) {
 	verifier, err := activation.NewPostVerifier(cfg, logger, activation.WithVerifyingOpts(verifyingOpts))
 	require.NoError(t, err)
 	t.Cleanup(func() { assert.NoError(t, verifier.Close()) })
-	poetDb := activation.NewPoetDb(db, logger.Named("poetDb"))
+	poetDb, err := activation.NewPoetDb(db, logger.Named("poetDb"))
+	require.NoError(t, err)
 	validator := activation.NewValidator(db, poetDb, cfg, opts.Scrypt, verifier)
 
 	eg, ctx := errgroup.WithContext(context.Background())

diff --git a/activation/e2e/builds_atx_v2_test.go b/activation/e2e/builds_atx_v2_test.go
@@ -65,7 +65,8 @@ func TestBuilder_SwitchesToBuildV2(t *testing.T) {
 
 	initPost(t, cfg, opts, sig, goldenATX, grpcCfg, svc)
 
-	poetDb := activation.NewPoetDb(db, logger.Named("poetDb"))
+	poetDb, err := activation.NewPoetDb(db, logger.Named("poetDb"))
+	require.NoError(t, err)
 	verifier, err := activation.NewPostVerifier(cfg, logger.Named("verifier"))
 	require.NoError(t, err)
 	t.Cleanup(func() { assert.NoError(t, verifier.Close()) })

diff --git a/activation/e2e/checkpoint_merged_test.go b/activation/e2e/checkpoint_merged_test.go
@@ -57,7 +57,8 @@ func Test_CheckpointAfterMerge(t *testing.T) {
 	verifier, err := activation.NewPostVerifier(cfg, logger, activation.WithVerifyingOpts(verifyingOpts))
 	require.NoError(t, err)
 	t.Cleanup(func() { assert.NoError(t, verifier.Close()) })
-	poetDb := activation.NewPoetDb(db, logger.Named("poetDb"))
+	poetDb, err := activation.NewPoetDb(db, logger.Named("poetDb"))
+	require.NoError(t, err)
 	validator := activation.NewValidator(db, poetDb, cfg, opts.Scrypt, verifier)
 
 	eg, ctx := errgroup.WithContext(context.Background())
@@ -280,7 +281,8 @@ func Test_CheckpointAfterMerge(t *testing.T) {
 	require.Equal(t, marriageATX.ID(), *checkpointedMerged.MarriageATX)
 
 	// 4. Spawn new ATX handler and builder using the new DB
-	poetDb = activation.NewPoetDb(newDB, logger.Named("poetDb"))
+	poetDb, err = activation.NewPoetDb(newDB, logger.Named("poetDb"))
+	require.NoError(t, err)
 	cdb = datastore.NewCachedDB(newDB, logger)
 
 	poetSvc = activation.NewPoetServiceWithClient(poetDb, client, poetCfg, logger)

diff --git a/activation/e2e/checkpoint_test.go b/activation/e2e/checkpoint_test.go
@@ -58,7 +58,8 @@ func TestCheckpoint_PublishingSoloATXs(t *testing.T) {
 	initPost(t, cfg, opts, sig, goldenATX, grpcCfg, svc)
 	syncer := syncedSyncer(t)
 
-	poetDb := activation.NewPoetDb(db, logger.Named("poetDb"))
+	poetDb, err := activation.NewPoetDb(db, logger.Named("poetDb"))
+	require.NoError(t, err)
 	verifier, err := activation.NewPostVerifier(cfg, logger.Named("verifier"))
 	require.NoError(t, err)
 	t.Cleanup(func() { assert.NoError(t, verifier.Close()) })
@@ -186,7 +187,8 @@ func TestCheckpoint_PublishingSoloATXs(t *testing.T) {
 	defer newDB.Close()
 
 	// 3. Spawn new ATX handler and builder using the new DB
-	poetDb = activation.NewPoetDb(newDB, logger.Named("poetDb"))
+	poetDb, err = activation.NewPoetDb(newDB, logger.Named("poetDb"))
+	require.NoError(t, err)
 	cdb = datastore.NewCachedDB(newDB, logger)
 	atxdata, err = atxsdata.Warm(newDB, 1, logger)
 	poetService = activation.NewPoetServiceWithClient(poetDb, client, poetCfg, logger)

diff --git a/activation/e2e/nipost_test.go b/activation/e2e/nipost_test.go
@@ -188,7 +188,8 @@ func TestNIPostBuilderWithClients(t *testing.T) {
 	require.NoError(t, err)
 	t.Cleanup(func() { assert.NoError(t, verifier.Close()) })
 
-	poetDb := activation.NewPoetDb(db, logger.Named("poetDb"))
+	poetDb, err := activation.NewPoetDb(db, logger.Named("poetDb"))
+	require.NoError(t, err)
 
 	postClient, err := svc.Client(sig.NodeID())
 	require.NoError(t, err)
@@ -271,7 +272,8 @@ func Test_NIPostBuilderWithMultipleClients(t *testing.T) {
 		GracePeriod: epoch / 4,
 	}
 
-	poetDb := activation.NewPoetDb(db, logger.Named("poetDb"))
+	poetDb, err := activation.NewPoetDb(db, logger.Named("poetDb"))
+	require.NoError(t, err)
 	client := ae2e.NewTestPoetClient(len(signers), poetCfg)
 	poetService := activation.NewPoetServiceWithClient(poetDb, client, poetCfg, logger)
 

diff --git a/activation/e2e/validation_test.go b/activation/e2e/validation_test.go
@@ -50,7 +50,8 @@ func TestValidator_Validate(t *testing.T) {
 		GracePeriod: epoch / 4,
 	}
 
-	poetDb := activation.NewPoetDb(statesql.InMemory(), logger.Named("poetDb"))
+	poetDb, err := activation.NewPoetDb(statesql.InMemory(), logger.Named("poetDb"))
+	require.NoError(t, err)
 	client := ae2e.NewTestPoetClient(1, poetCfg)
 	poetService := activation.NewPoetServiceWithClient(poetDb, client, poetCfg, logger)
 

diff --git a/activation/poet_client_test.go b/activation/poet_client_test.go
@@ -530,7 +530,8 @@ func TestPoetService_CachesCertifierInfo(t *testing.T) {
 
 			cfg := DefaultPoetConfig()
 			cfg.InfoCacheTTL = tc.ttl
-			db := NewPoetDb(statesql.InMemory(), zaptest.NewLogger(t))
+			db, err := NewPoetDb(statesql.InMemory(), zaptest.NewLogger(t))
+			require.NoError(t, err)
 
 			url := &url.URL{Host: "certifier.hello"}
 			pubkey := []byte("pubkey")

diff --git a/activation/poetdb.go b/activation/poetdb.go
@@ -44,7 +44,7 @@ type PoetDb struct {
 }
 
 // NewPoetDb returns a new PoET handler.
-func NewPoetDb(db sql.StateDatabase, log *zap.Logger, opts ...PoetDbOption) *PoetDb {
+func NewPoetDb(db sql.StateDatabase, log *zap.Logger, opts ...PoetDbOption) (*PoetDb, error) {
 	options := PoetDbOptions{
 		// in last epochs there are 45 proofs per epoch, with each of them nearly 140KB
 		// 200 is set not to keep multiple epochs, but to account for unexpected growth
@@ -60,13 +60,13 @@ func NewPoetDb(db sql.StateDatabase, log *zap.Logger, opts ...PoetDbOption) *Poe
 	}
 	poetProofsLru, err := lru.New[types.PoetProofRef, *types.PoetProofMessage](options.cacheSize)
 	if err != nil {
-		log.Panic("failed to create PoET proofs LRU cache", zap.Error(err))
+		return nil, fmt.Errorf("create lru: %w", err)
 	}
 	return &PoetDb{
 		sqlDB:         db,
 		poetProofsLru: poetProofsLru,
 		logger:        log,
-	}
+	}, nil
 }
 
 // HasProof returns true if the database contains a proof with the given reference, or false otherwise.

diff --git a/activation/poetdb_test.go b/activation/poetdb_test.go
@@ -63,7 +63,8 @@ func getPoetProof(t *testing.T) types.PoetProofMessage {
 func TestPoetDbHappyFlow(t *testing.T) {
 	r := require.New(t)
 	msg := getPoetProof(t)
-	poetDb := NewPoetDb(statesql.InMemory(), zaptest.NewLogger(t))
+	poetDb, err := NewPoetDb(statesql.InMemory(), zaptest.NewLogger(t))
+	r.NoError(err)
 
 	r.NoError(poetDb.Validate(msg.Statement[:], msg.PoetProof, msg.PoetServiceID, msg.RoundID, types.EmptyEdSignature))
 	ref, err := msg.Ref()
@@ -83,10 +84,11 @@ func TestPoetDbHappyFlow(t *testing.T) {
 func TestPoetDbInvalidPoetProof(t *testing.T) {
 	r := require.New(t)
 	msg := getPoetProof(t)
-	poetDb := NewPoetDb(statesql.InMemory(), zaptest.NewLogger(t))
+	poetDb, err := NewPoetDb(statesql.InMemory(), zaptest.NewLogger(t))
+	r.NoError(err)
 	msg.PoetProof.Root = []byte("some other root")
 
-	err := poetDb.Validate(msg.Statement[:], msg.PoetProof, msg.PoetServiceID, msg.RoundID, types.EmptyEdSignature)
+	err = poetDb.Validate(msg.Statement[:], msg.PoetProof, msg.PoetServiceID, msg.RoundID, types.EmptyEdSignature)
 	r.EqualError(
 		err,
 		fmt.Sprintf(
@@ -99,10 +101,11 @@ func TestPoetDbInvalidPoetProof(t *testing.T) {
 func TestPoetDbInvalidPoetStatement(t *testing.T) {
 	r := require.New(t)
 	msg := getPoetProof(t)
-	poetDb := NewPoetDb(statesql.InMemory(), zaptest.NewLogger(t))
+	poetDb, err := NewPoetDb(statesql.InMemory(), zaptest.NewLogger(t))
+	r.NoError(err)
 	msg.Statement = types.CalcHash32([]byte("some other statement"))
 
-	err := poetDb.Validate(msg.Statement[:], msg.PoetProof, msg.PoetServiceID, msg.RoundID, types.EmptyEdSignature)
+	err = poetDb.Validate(msg.Statement[:], msg.PoetProof, msg.PoetServiceID, msg.RoundID, types.EmptyEdSignature)
 	r.EqualError(
 		err,
 		fmt.Sprintf(
@@ -115,9 +118,10 @@ func TestPoetDbInvalidPoetStatement(t *testing.T) {
 func TestPoetDbNonExistingKeys(t *testing.T) {
 	r := require.New(t)
 	msg := getPoetProof(t)
-	poetDb := NewPoetDb(statesql.InMemory(), zaptest.NewLogger(t))
+	poetDb, err := NewPoetDb(statesql.InMemory(), zaptest.NewLogger(t))
+	r.NoError(err)
 
-	_, err := poetDb.GetProofRef(msg.PoetServiceID, "0")
+	_, err = poetDb.GetProofRef(msg.PoetServiceID, "0")
 	r.EqualError(
 		err,
 		fmt.Sprintf(

diff --git a/fetch/cache.go b/fetch/cache.go
@@ -1,6 +1,7 @@
 package fetch
 
 import (
+	"fmt"
 	"math/rand/v2"
 	"sync"
 
@@ -9,7 +10,6 @@ import (
 
 	"github.com/spacemeshos/go-spacemesh/common/types"
 	"github.com/spacemeshos/go-spacemesh/datastore"
-	"github.com/spacemeshos/go-spacemesh/log"
 	"github.com/spacemeshos/go-spacemesh/p2p"
 )
 
@@ -24,12 +24,12 @@ type HashPeersCache struct {
 type HashPeers map[p2p.Peer]struct{}
 
 // NewHashPeersCache creates a new hash-to-peers cache.
-func NewHashPeersCache(size int) *HashPeersCache {
+func NewHashPeersCache(size int) (*HashPeersCache, error) {
 	cache, err := lru.New[types.Hash32, HashPeers](size)
 	if err != nil {
-		log.Panic("could not initialize cache ", err)
+		return nil, fmt.Errorf("create lru: %w", err)
 	}
-	return &HashPeersCache{Cache: cache}
+	return &HashPeersCache{Cache: cache}, nil
 }
 
 // get returns peers for a given hash (non-thread-safe).

diff --git a/fetch/cache_test.go b/fetch/cache_test.go
@@ -25,7 +25,8 @@ func getCachedEntry(cache *HashPeersCache, hash types.Hash32) (HashPeers, bool)
 func TestAdd(t *testing.T) {
 	t.Parallel()
 	t.Run("1Hash3Peers", func(t *testing.T) {
-		cache := NewHashPeersCache(10)
+		cache, err := NewHashPeersCache(10)
+		require.NoError(t, err)
 		hash := types.RandomHash()
 		peer1 := p2p.Peer("test_peer_1")
 		peer2 := p2p.Peer("test_peer_2")
@@ -49,7 +50,8 @@ func TestAdd(t *testing.T) {
 		require.Len(t, hashPeers, 3)
 	})
 	t.Run("2Hashes1Peer", func(t *testing.T) {
-		cache := NewHashPeersCache(10)
+		cache, err := NewHashPeersCache(10)
+		require.NoError(t, err)
 		hash1 := types.RandomHash()
 		hash2 := types.RandomHash()
 		peer := p2p.Peer("test_peer")
@@ -74,7 +76,8 @@ func TestAdd(t *testing.T) {
 func TestGetRandom(t *testing.T) {
 	t.Parallel()
 	t.Run("no hash peers", func(t *testing.T) {
-		cache := NewHashPeersCache(10)
+		cache, err := NewHashPeersCache(10)
+		require.NoError(t, err)
 		hash := types.RandomHash()
 		var seed [32]byte
 		binary.LittleEndian.PutUint64(seed[:], uint64(time.Now().UnixNano()))
@@ -83,7 +86,8 @@ func TestGetRandom(t *testing.T) {
 		require.Empty(t, peers)
 	})
 	t.Run("1Hash3Peers", func(t *testing.T) {
-		cache := NewHashPeersCache(10)
+		cache, err := NewHashPeersCache(10)
+		require.NoError(t, err)
 		hash := types.RandomHash()
 		peer1 := p2p.Peer("test_peer_1")
 		peer2 := p2p.Peer("test_peer_2")
@@ -110,7 +114,8 @@ func TestGetRandom(t *testing.T) {
 		require.ElementsMatch(t, []p2p.Peer{peer1, peer2, peer3}, peers)
 	})
 	t.Run("2Hashes1Peer", func(t *testing.T) {
-		cache := NewHashPeersCache(10)
+		cache, err := NewHashPeersCache(10)
+		require.NoError(t, err)
 		hash1 := types.RandomHash()
 		hash2 := types.RandomHash()
 		peer := p2p.Peer("test_peer")
@@ -138,7 +143,8 @@ func TestGetRandom(t *testing.T) {
 func TestRegisterPeerHashes(t *testing.T) {
 	t.Parallel()
 	t.Run("1Hash2Peers", func(t *testing.T) {
-		cache := NewHashPeersCache(10)
+		cache, err := NewHashPeersCache(10)
+		require.NoError(t, err)
 		hash1 := types.RandomHash()
 		hash2 := types.RandomHash()
 		hash3 := types.RandomHash()
@@ -154,7 +160,8 @@ func TestRegisterPeerHashes(t *testing.T) {
 }
 
 func TestRace(t *testing.T) {
-	cache := NewHashPeersCache(10)
+	cache, err := NewHashPeersCache(10)
+	require.NoError(t, err)
 	hash := types.RandomHash()
 	peer1 := p2p.Peer("test_peer_1")
 	peer2 := p2p.Peer("test_peer_2")

diff --git a/fetch/fetch.go b/fetch/fetch.go
@@ -5,6 +5,7 @@ import (
 	"context"
 	"encoding/binary"
 	"errors"
+	"fmt"
 	"io"
 	"math/rand/v2"
 	"sync"
@@ -268,9 +269,14 @@ func NewFetch(
 	proposals *store.Store,
 	host *p2p.Host,
 	opts ...Option,
-) *Fetch {
+) (*Fetch, error) {
 	bs := datastore.NewBlobStore(cdb, proposals)
 
+	hashPeerCache, err := NewHashPeersCache(cacheSize)
+	if err != nil {
+		// this should never happen, since cacheSize is a constant, but just in case
+		panic(fmt.Errorf("create hash peer cache: %w", err))
+	}
 	f := &Fetch{
 		cfg:         DefaultConfig(),
 		logger:      zap.NewNop(),
@@ -279,7 +285,7 @@ func NewFetch(
 		servers:     map[string]requester{},
 		unprocessed: make(map[types.Hash32]*request),
 		ongoing:     make(map[types.Hash32]*request),
-		hashToPeers: NewHashPeersCache(cacheSize),
+		hashToPeers: hashPeerCache,
 	}
 	for _, opt := range opts {
 		opt(f)
@@ -341,7 +347,7 @@ func NewFetch(
 		f.registerServer(host, lyrDataProtocol, server.WrapHandler(h.handleLayerDataReq))
 		f.registerServer(host, OpnProtocol, server.WrapHandler(h.handleLayerOpinionsReq2))
 	}
-	return f
+	return f, nil
 }
 
 func (f *Fetch) registerServer(