[NOD-993] Get rid of redundant error types + Use %+v when printing startup errors (#719)

* [NOD-993] Use %+v when printing errors

* [NOD-993] Get rid of AssertError

* [NOD-993] Made ruleError use github.com/pkg/errors

* [NOD-993] remove redundant TODO

* [NOD-993] remove redundant Comment

* [NOD-993] Removed DeploymentError
This commit is contained in:
Svarog 2020-05-13 17:27:53 +03:00 committed by GitHub
parent 35b943e04f
commit 378f0b659a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 47 additions and 110 deletions

View File

@ -7,15 +7,15 @@ package blockdag
import (
"compress/bzip2"
"encoding/binary"
"github.com/pkg/errors"
"io"
"os"
"path/filepath"
"reflect"
"strings"
"testing"
"time"
"github.com/pkg/errors"
"github.com/kaspanet/kaspad/dagconfig"
"github.com/kaspanet/kaspad/util"
"github.com/kaspanet/kaspad/util/daghash"
@ -144,29 +144,26 @@ func addNodeAsChildToParents(node *blockNode) {
// same type (either both nil or both of type RuleError) and their error codes
// match when not nil.
func checkRuleError(gotErr, wantErr error) error {
// Ensure the error code is of the expected type and the error
// code matches the value specified in the test instance.
if reflect.TypeOf(gotErr) != reflect.TypeOf(wantErr) {
return errors.Errorf("wrong error - got %T (%[1]v), want %T",
gotErr, wantErr)
}
if gotErr == nil {
if wantErr == nil && gotErr == nil {
return nil
}
// Ensure the want error type is a script error.
werr, ok := wantErr.(RuleError)
if !ok {
return errors.Errorf("unexpected test error type %T", wantErr)
var gotRuleErr RuleError
if ok := errors.As(gotErr, &gotRuleErr); !ok {
return errors.Errorf("gotErr expected to be RuleError, but got %+v instead", gotErr)
}
var wantRuleErr RuleError
if ok := errors.As(wantErr, &wantRuleErr); !ok {
return errors.Errorf("wantErr expected to be RuleError, but got %+v instead", wantErr)
}
// Ensure the error codes match. It's safe to use a raw type assert
// here since the code above already proved they are the same type and
// the want error is a script error.
gotErrorCode := gotErr.(RuleError).ErrorCode
if gotErrorCode != werr.ErrorCode {
if gotRuleErr.ErrorCode != wantRuleErr.ErrorCode {
return errors.Errorf("mismatched error code - got %v (%v), want %v",
gotErrorCode, gotErr, werr.ErrorCode)
gotRuleErr.ErrorCode, gotErr, wantRuleErr.ErrorCode)
}
return nil

View File

@ -6,12 +6,13 @@ package blockdag
import (
"fmt"
"github.com/kaspanet/kaspad/dbaccess"
"math"
"sort"
"sync"
"time"
"github.com/kaspanet/kaspad/dbaccess"
"github.com/pkg/errors"
"github.com/kaspanet/kaspad/util/subnetworkid"
@ -2082,10 +2083,10 @@ type Config struct {
func New(config *Config) (*BlockDAG, error) {
// Enforce required config fields.
if config.DAGParams == nil {
return nil, AssertError("BlockDAG.New DAG parameters nil")
return nil, errors.New("BlockDAG.New DAG parameters nil")
}
if config.TimeSource == nil {
return nil, AssertError("BlockDAG.New timesource is nil")
return nil, errors.New("BlockDAG.New timesource is nil")
}
params := config.DAGParams

View File

@ -223,7 +223,7 @@ func (dag *BlockDAG) initDAGState() error {
log.Debugf("Applying the loaded utxoCollection to the virtual block...")
dag.virtual.utxoSet, err = newFullUTXOSetFromUTXOCollection(fullUTXOCollection)
if err != nil {
return AssertError(fmt.Sprintf("Error loading UTXOSet: %s", err))
return errors.Wrap(err, "Error loading UTXOSet")
}
log.Debugf("Applying the stored tips to the virtual block...")
@ -289,14 +289,14 @@ func (dag *BlockDAG) initBlockIndex() (unprocessedBlockNodes []*blockNode, err e
if dag.blockCount == 0 {
if !node.hash.IsEqual(dag.dagParams.GenesisHash) {
return nil, AssertError(fmt.Sprintf("Expected "+
return nil, errors.Errorf("Expected "+
"first entry in block index to be genesis block, "+
"found %s", node.hash))
"found %s", node.hash)
}
} else {
if len(node.parents) == 0 {
return nil, AssertError(fmt.Sprintf("block %s "+
"has no parents but it's not the genesis block", node.hash))
return nil, errors.Errorf("block %s "+
"has no parents but it's not the genesis block", node.hash)
}
}
@ -350,8 +350,8 @@ func (dag *BlockDAG) initVirtualBlockTips(state *dagState) error {
for _, tipHash := range state.TipHashes {
tip := dag.index.LookupNode(tipHash)
if tip == nil {
return AssertError(fmt.Sprintf("cannot find "+
"DAG tip %s in block index", state.TipHashes))
return errors.Errorf("cannot find "+
"DAG tip %s in block index", state.TipHashes)
}
tips.add(tip)
}
@ -365,12 +365,12 @@ func (dag *BlockDAG) processUnprocessedBlockNodes(unprocessedBlockNodes []*block
// doesn't, the database has certainly been corrupted.
blockExists, err := dbaccess.HasBlock(dbaccess.NoTx(), node.hash)
if err != nil {
return AssertError(fmt.Sprintf("HasBlock "+
"for block %s failed: %s", node.hash, err))
return errors.Wrapf(err, "HasBlock "+
"for block %s failed: %s", node.hash, err)
}
if !blockExists {
return AssertError(fmt.Sprintf("block %s "+
"exists in block index but not in block db", node.hash))
return errors.Errorf("block %s "+
"exists in block index but not in block db", node.hash)
}
// Attempt to accept the block.
@ -388,14 +388,14 @@ func (dag *BlockDAG) processUnprocessedBlockNodes(unprocessedBlockNodes []*block
// If the block is an orphan or is delayed then it couldn't have
// possibly been written to the block index in the first place.
if isOrphan {
return AssertError(fmt.Sprintf("Block %s, which was not "+
return errors.Errorf("Block %s, which was not "+
"previously processed, turned out to be an orphan, which is "+
"impossible.", node.hash))
"impossible.", node.hash)
}
if isDelayed {
return AssertError(fmt.Sprintf("Block %s, which was not "+
return errors.Errorf("Block %s, which was not "+
"previously processed, turned out to be delayed, which is "+
"impossible.", node.hash))
"impossible.", node.hash)
}
}
return nil
@ -428,8 +428,8 @@ func (dag *BlockDAG) deserializeBlockNode(blockRow []byte) (*blockNode, error) {
for _, hash := range header.ParentHashes {
parent := dag.index.LookupNode(hash)
if parent == nil {
return nil, AssertError(fmt.Sprintf("deserializeBlockNode: Could "+
"not find parent %s for block %s", hash, header.BlockHash()))
return nil, errors.Errorf("deserializeBlockNode: Could "+
"not find parent %s for block %s", hash, header.BlockHash())
}
node.parents.add(parent)
}

View File

@ -6,28 +6,10 @@ package blockdag
import (
"fmt"
"github.com/pkg/errors"
)
// DeploymentError identifies an error that indicates a deployment ID was
// specified that does not exist.
type DeploymentError uint32
// Error returns the assertion error as a human-readable string and satisfies
// the error interface.
func (e DeploymentError) Error() string {
return fmt.Sprintf("deployment ID %d does not exist", uint32(e))
}
// AssertError identifies an error that indicates an internal code consistency
// issue and should be treated as a critical and unrecoverable error.
type AssertError string
// Error returns the assertion error as a human-readable string and satisfies
// the error interface.
func (e AssertError) Error() string {
return "assertion failed: " + string(e)
}
// ErrorCode identifies a kind of error.
type ErrorCode int
@ -294,7 +276,6 @@ func (e RuleError) Error() string {
return e.Description
}
// ruleError creates an RuleError given a set of arguments.
func ruleError(c ErrorCode, desc string) RuleError {
return RuleError{ErrorCode: c, Description: desc}
func ruleError(c ErrorCode, desc string) error {
return errors.WithStack(RuleError{ErrorCode: c, Description: desc})
}

View File

@ -5,7 +5,6 @@
package blockdag
import (
"fmt"
"testing"
)
@ -99,46 +98,3 @@ func TestRuleError(t *testing.T) {
}
}
}
// TestDeploymentError tests the stringized output for the DeploymentError type.
func TestDeploymentError(t *testing.T) {
t.Parallel()
tests := []struct {
in DeploymentError
want string
}{
{
DeploymentError(0),
"deployment ID 0 does not exist",
},
{
DeploymentError(10),
"deployment ID 10 does not exist",
},
{
DeploymentError(123),
"deployment ID 123 does not exist",
},
}
t.Logf("Running %d tests", len(tests))
for i, test := range tests {
result := test.in.Error()
if result != test.want {
t.Errorf("Error #%d\n got: %s want: %s", i, result,
test.want)
continue
}
}
}
func TestAssertError(t *testing.T) {
message := "abc 123"
err := AssertError(message)
expectedMessage := fmt.Sprintf("assertion failed: %s", message)
if expectedMessage != err.Error() {
t.Errorf("Unexpected AssertError message. "+
"Got: %s, want: %s", err.Error(), expectedMessage)
}
}

View File

@ -8,6 +8,7 @@ import (
"fmt"
"github.com/kaspanet/kaspad/util/daghash"
"github.com/pkg/errors"
)
// ThresholdState define the various threshold states used when voting on
@ -177,9 +178,9 @@ func (dag *BlockDAG) thresholdState(prevNode *blockNode, checker thresholdCondit
var ok bool
state, ok = cache.Lookup(prevNode.hash)
if !ok {
return ThresholdFailed, AssertError(fmt.Sprintf(
return ThresholdFailed, errors.Errorf(
"thresholdState: cache lookup failed for %s",
prevNode.hash))
prevNode.hash)
}
}
@ -297,7 +298,7 @@ func (dag *BlockDAG) IsDeploymentActive(deploymentID uint32) (bool, error) {
// This function MUST be called with the DAG state lock held (for writes).
func (dag *BlockDAG) deploymentState(prevNode *blockNode, deploymentID uint32) (ThresholdState, error) {
if deploymentID > uint32(len(dag.dagParams.Deployments)) {
return ThresholdFailed, DeploymentError(deploymentID)
return ThresholdFailed, errors.Errorf("deployment ID %d does not exist", deploymentID)
}
deployment := &dag.dagParams.Deployments[deploymentID]

View File

@ -5,12 +5,13 @@
package blockdag
import (
"github.com/pkg/errors"
"math"
"path/filepath"
"testing"
"time"
"github.com/pkg/errors"
"github.com/kaspanet/kaspad/dagconfig"
"github.com/kaspanet/kaspad/util"
"github.com/kaspanet/kaspad/util/daghash"

View File

@ -6,7 +6,6 @@ package main
import (
"fmt"
"github.com/kaspanet/kaspad/dbaccess"
_ "net/http/pprof"
"os"
"path/filepath"
@ -15,6 +14,8 @@ import (
"runtime/pprof"
"strings"
"github.com/kaspanet/kaspad/dbaccess"
"github.com/kaspanet/kaspad/blockdag/indexers"
"github.com/kaspanet/kaspad/config"
"github.com/kaspanet/kaspad/limits"
@ -137,8 +138,7 @@ func kaspadMain(serverChan chan<- *server.Server) error {
server, err := server.NewServer(cfg.Listeners, config.ActiveConfig().NetParams(),
interrupt)
if err != nil {
// TODO: this logging could do with some beautifying.
kasdLog.Errorf("Unable to start server on %s: %s",
kasdLog.Errorf("Unable to start server on %s: %+v",
strings.Join(cfg.Listeners, ", "), err)
return err
}