Skip to content

Commit

Permalink
Merge pull request #350 from GeekArthur/errorHandling2
Browse files Browse the repository at this point in the history 
Error handling improvement for logger and cmd packages
  • Loading branch information
@GeekArthur
GeekArthur committed Sep 13, 2022
2 parents a2da69f + 1913ad2 commit e7a9ebd
Show file tree
Hide file tree
Showing 15 changed files with 154 additions and 150 deletions.
17 changes: 9 additions & 8 deletions cmd/check_construction.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,15 @@ package cmd
import (
"context"
"fmt"
"github.com/coinbase/rosetta-cli/pkg/errors"
"time"

cliErrs "github.com/coinbase/rosetta-cli/pkg/errors"

"github.com/coinbase/rosetta-cli/pkg/results"
"github.com/coinbase/rosetta-cli/pkg/tester"

"github.com/coinbase/rosetta-sdk-go/fetcher"
"github.com/coinbase/rosetta-sdk-go/types"
"github.com/coinbase/rosetta-sdk-go/utils"
"github.com/spf13/cobra"
"golang.org/x/sync/errgroup"
Expand Down Expand Up @@ -59,7 +61,7 @@ func runCheckConstructionCmd(_ *cobra.Command, _ []string) error {
Config,
nil,
nil,
errors.ErrConstructionConfigMissing,
cliErrs.ErrConstructionConfigMissing,
)
}

Expand Down Expand Up @@ -88,7 +90,7 @@ func runCheckConstructionCmd(_ *cobra.Command, _ []string) error {
Config,
nil,
nil,
fmt.Errorf("%w: unable to initialize asserter", fetchErr.Err),
fmt.Errorf("unable to initialize asserter for fetcher: %w", fetchErr.Err),
)
}

Expand All @@ -99,7 +101,7 @@ func runCheckConstructionCmd(_ *cobra.Command, _ []string) error {
Config,
nil,
nil,
fmt.Errorf("%w: unable to confirm network is supported", err),
fmt.Errorf("unable to confirm network %s is supported: %w", types.PrintStruct(Config.Network), err),
)
}

Expand All @@ -112,7 +114,7 @@ func runCheckConstructionCmd(_ *cobra.Command, _ []string) error {
Config,
nil,
nil,
err,
fmt.Errorf("network options don't match asserter configuration file %s: %w", asserterConfigurationFile, err),
)
}
}
Expand All @@ -130,18 +132,17 @@ func runCheckConstructionCmd(_ *cobra.Command, _ []string) error {
Config,
nil,
nil,
fmt.Errorf("%w: unable to initialize construction tester", err),
fmt.Errorf("unable to initialize construction tester: %w", err),
)
}

defer constructionTester.CloseDatabase(ctx)

if err := constructionTester.PerformBroadcasts(ctx); err != nil {
return results.ExitConstruction(
Config,
nil,
nil,
fmt.Errorf("%w: unable to perform broadcasts", err),
fmt.Errorf("unable to perform broadcasts: %w", err),
)
}

Expand Down
18 changes: 12 additions & 6 deletions cmd/check_data.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@ package cmd
import (
"context"
"fmt"
"github.com/coinbase/rosetta-cli/pkg/errors"
"time"

"github.com/coinbase/rosetta-cli/pkg/results"
"github.com/coinbase/rosetta-cli/pkg/tester"
"github.com/coinbase/rosetta-sdk-go/fetcher"
"github.com/coinbase/rosetta-sdk-go/types"
"github.com/coinbase/rosetta-sdk-go/utils"
"github.com/spf13/cobra"
"golang.org/x/sync/errgroup"
Expand Down Expand Up @@ -96,7 +96,7 @@ func runCheckDataCmd(_ *cobra.Command, _ []string) error {
Config,
nil,
nil,
fmt.Errorf("%w: unable to initialize asserter", fetchErr.Err),
fmt.Errorf("unable to initialize asserter for fetcher: %w", fetchErr.Err),
"",
"",
)
Expand All @@ -109,7 +109,7 @@ func runCheckDataCmd(_ *cobra.Command, _ []string) error {
Config,
nil,
nil,
fmt.Errorf("%w: unable to confirm network", err),
fmt.Errorf("unable to confirm network %s is supported: %w", types.PrintStruct(Config.Network), err),
"",
"",
)
Expand All @@ -124,7 +124,7 @@ func runCheckDataCmd(_ *cobra.Command, _ []string) error {
Config,
nil,
nil,
err,
fmt.Errorf("network options don't match asserter configuration file %s: %w", asserterConfigurationFile, err),
"",
"",
)
Expand All @@ -141,9 +141,15 @@ func runCheckDataCmd(_ *cobra.Command, _ []string) error {
nil, // only populated when doing recursive search
&SignalReceived,
)

if err != nil {
return fmt.Errorf("%s:%s", errors.ErrInitDataTester, err)
return results.ExitData(
Config,
nil,
nil,
fmt.Errorf("unable to initialize data tester: %w", err),
"",
"",
)
}
defer dataTester.CloseDatabase(ctx)

Expand Down
40 changes: 21 additions & 19 deletions cmd/check_spec.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ import (
"github.com/coinbase/rosetta-sdk-go/fetcher"
"github.com/coinbase/rosetta-sdk-go/types"
"github.com/spf13/cobra"

cliErrs "github.com/coinbase/rosetta-cli/pkg/errors"
)

var (
Expand All @@ -32,7 +34,7 @@ var (
Long: `Check:spec checks whether a Rosetta implementation satisfies either Coinbase-specific requirements or
minimum requirements specified in rosetta-api.org.
By default, check:spec will verify only Coinbase spec requirements. To verifiy the minimum requirements as well,
By default, check:spec will verify only Coinbase spec requirements. To verify the minimum requirements as well,
add the --all flag to the check:spec command:
rosetta-cli check:spec --all --configuration-file [filepath]
Expand Down Expand Up @@ -60,7 +62,7 @@ type checkSpec struct {

func newCheckSpec(ctx context.Context) (*checkSpec, error) {
if Config.Construction == nil {
return nil, fmt.Errorf("%v", errRosettaConfigNoConstruction)
return nil, cliErrs.ErrConstructionConfigMissing
}

onlineFetcherOpts := []fetcher.Option{
Expand Down Expand Up @@ -97,7 +99,7 @@ func newCheckSpec(ctx context.Context) (*checkSpec, error) {
Config,
nil,
nil,
fmt.Errorf("%v: unable to initialize asserter for online node fetcher", fetchErr.Err),
fmt.Errorf("unable to initialize asserter for online fetcher: %w", fetchErr.Err),
"",
"",
)
Expand Down Expand Up @@ -132,7 +134,7 @@ func (cs *checkSpec) networkOptions(ctx context.Context) checkSpecOutput {
// This is an endpoint for offline mode
_, err := cs.offlineFetcher.NetworkOptionsRetry(ctx, Config.Network, nil)
if err != nil {
printError("%v: unable to fetch network options\n", err.Err)
printError("unable to fetch network options: %v\n", err.Err)
markAllValidationsFailed(output)
return output
}
Expand Down Expand Up @@ -169,7 +171,7 @@ func (cs *checkSpec) networkList(ctx context.Context) checkSpecOutput {

// endpoint for offline mode
if err != nil {
printError("%v: unable to fetch network list", err.Err)
printError("unable to fetch network list: %v\n", err.Err)
markAllValidationsFailed(output)
return output
}
Expand Down Expand Up @@ -211,12 +213,12 @@ func (cs *checkSpec) accountCoins(ctx context.Context) checkSpecOutput {
if isUTXO() {
acct, _, currencies, err := cs.getAccount(ctx)
if err != nil {
printError("%v: unable to get an account\n", err)
printError("unable to get an account: %v\n", err)
markAllValidationsFailed(output)
return output
}
if err != nil {
printError("%v\n", errAccountNullPointer)
if acct == nil {
printError("%v\n", cliErrs.ErrAccountNullPointer)
markAllValidationsFailed(output)
return output
}
Expand All @@ -228,7 +230,7 @@ func (cs *checkSpec) accountCoins(ctx context.Context) checkSpecOutput {
false,
currencies)
if fetchErr != nil {
printError("%v: unable to get coins for account: %v\n", fetchErr.Err, *acct)
printError("unable to get coins for account %s: %v\n", types.PrintStruct(acct), fetchErr.Err)
markAllValidationsFailed(output)
return output
}
Expand Down Expand Up @@ -261,7 +263,7 @@ func (cs *checkSpec) block(ctx context.Context) checkSpecOutput {

res, fetchErr := cs.onlineFetcher.NetworkStatusRetry(ctx, Config.Network, nil)
if fetchErr != nil {
printError("%v: unable to get network status\n", fetchErr.Err)
printError("unable to get network status: %v\n", fetchErr.Err)
markAllValidationsFailed(output)
return output
}
Expand All @@ -278,15 +280,15 @@ func (cs *checkSpec) block(ctx context.Context) checkSpecOutput {
}
b, fetchErr := cs.onlineFetcher.BlockRetry(ctx, Config.Network, &blockID)
if fetchErr != nil {
printError("%v: unable to fetch block %v\n", fetchErr.Err, blockID)
printError("unable to fetch block %s: %v\n", types.PrintStruct(blockID), fetchErr.Err)
markAllValidationsFailed(output)
return output
}

if block == nil {
block = b
} else if !isEqual(types.Hash(*block), types.Hash(*b)) {
printError("%v\n", errBlockNotIdempotent)
printError("%v\n", cliErrs.ErrBlockNotIdempotent)
setValidationStatusFailed(output, idempotent)
}
}
Expand All @@ -295,24 +297,24 @@ func (cs *checkSpec) block(ctx context.Context) checkSpecOutput {
// fetch the tip block again
res, fetchErr = cs.onlineFetcher.NetworkStatusRetry(ctx, Config.Network, nil)
if fetchErr != nil {
printError("%v: unable to get network status\n", fetchErr.Err)
printError("unable to get network status: %v\n", fetchErr.Err)
setValidationStatusFailed(output, defaultTip)
return output
}
tip := res.CurrentBlockIdentifier

// tip shoud be returned if block_identifier is not specified
// tip should be returned if block_identifier is not specified
emptyBlockID := &types.PartialBlockIdentifier{}
block, fetchErr := cs.onlineFetcher.BlockRetry(ctx, Config.Network, emptyBlockID)
if fetchErr != nil {
printError("%v: unable to fetch tip block\n", fetchErr.Err)
printError("unable to fetch tip block: %v\n", fetchErr.Err)
setValidationStatusFailed(output, defaultTip)
return output
}

// block index returned from /block should be >= the index returned by /network/status
if isNegative(block.BlockIdentifier.Index - tip.Index) {
printError("%v\n", errBlockTip)
printError("%v\n", cliErrs.ErrBlockTip)
setValidationStatusFailed(output, defaultTip)
}

Expand Down Expand Up @@ -383,7 +385,7 @@ func (cs *checkSpec) getAccount(ctx context.Context) (
error) {
res, err := cs.onlineFetcher.NetworkStatusRetry(ctx, Config.Network, nil)
if err != nil {
return nil, nil, nil, fmt.Errorf("%v: unable to get network status", err.Err)
return nil, nil, nil, fmt.Errorf("unable to get network status of network %s: %w", types.PrintStruct(Config.Network), err.Err)
}

var acct *types.AccountIdentifier
Expand All @@ -399,7 +401,7 @@ func (cs *checkSpec) getAccount(ctx context.Context) (

block, err := cs.onlineFetcher.BlockRetry(ctx, Config.Network, blockID)
if err != nil {
return nil, nil, nil, fmt.Errorf("%v: unable to fetch block at index: %v", err.Err, i)
return nil, nil, nil, fmt.Errorf("unable to fetch block at index %d: %w", i, err.Err)
}

// looking for an account in block transactions
Expand All @@ -425,7 +427,7 @@ func runCheckSpecCmd(_ *cobra.Command, _ []string) error {
ctx := context.Background()
cs, err := newCheckSpec(ctx)
if err != nil {
return fmt.Errorf("%v: unable to create checkSpec object with online URL", err)
return fmt.Errorf("unable to create checkSpec object with online URL: %w", err)
}

output := []checkSpecOutput{}
Expand Down
14 changes: 3 additions & 11 deletions cmd/check_spec_utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,21 +15,13 @@
package cmd

import (
"errors"
"fmt"
"strconv"

"github.com/coinbase/rosetta-sdk-go/fetcher"
"github.com/fatih/color"
)

var (
errErrorEmptyMessage = errors.New("Error object can't have empty message")
errErrorNegativeCode = errors.New("Error object can't have negative code")
errAccountNullPointer = errors.New("Null pointer to Account object")
errBlockNotIdempotent = errors.New("Multiple calls with the same hash don't return the same block")
errBlockTip = errors.New("Unspecified block_identifier doesn't give the tip block")
errRosettaConfigNoConstruction = errors.New("No construction element in Rosetta config")
cliErrs "github.com/coinbase/rosetta-cli/pkg/errors"
)

type checkSpecAPI string
Expand Down Expand Up @@ -112,12 +104,12 @@ func setValidationStatusFailed(output checkSpecOutput, req checkSpecRequirement)
func validateErrorObject(err *fetcher.Error, output checkSpecOutput) {
if err != nil {
if err.ClientErr != nil && isNegative(int64(err.ClientErr.Code)) {
printError("%v\n", errErrorNegativeCode)
printError("%v\n", cliErrs.ErrErrorNegativeCode)
setValidationStatusFailed(output, errorCode)
}

if err.ClientErr != nil && isEmpty(err.ClientErr.Message) {
printError("%v\n", errErrorEmptyMessage)
printError("%v\n", cliErrs.ErrErrorEmptyMessage)
setValidationStatusFailed(output, errorMessage)
}
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/configuration_create.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ var (

func runConfigurationCreateCmd(cmd *cobra.Command, args []string) error {
if err := utils.SerializeAndWrite(args[0], configuration.DefaultConfiguration()); err != nil {
return fmt.Errorf("%w: unable to save configuration file to %s", err, args[0])
return fmt.Errorf("unable to save configuration file to %s: %w", args[0], err)
}

return nil
Expand Down
2 changes: 1 addition & 1 deletion cmd/configuration_validate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ var (
func runConfigurationValidateCmd(cmd *cobra.Command, args []string) error {
_, err := configuration.LoadConfiguration(Context, args[0])
if err != nil {
return fmt.Errorf("%w: configuration validation failed %s", err, args[0])
return fmt.Errorf("configuration validation failed %s: %w", args[0], err)
}

color.Green("Configuration file validated!")
Expand Down
10 changes: 5 additions & 5 deletions cmd/root.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,12 +103,12 @@ var (
// rootPreRun is executed before the root command runs and sets up cpu
// profiling.
//
// Bassed on https://golang.org/pkg/runtime/pprof/#hdr-Profiling_a_Go_program
// Based on https://golang.org/pkg/runtime/pprof/#hdr-Profiling_a_Go_program
func rootPreRun(*cobra.Command, []string) error {
if cpuProfile != "" {
f, err := os.Create(path.Clean(cpuProfile))
if err != nil {
return fmt.Errorf("%w: unable to create CPU profile file", err)
return fmt.Errorf("unable to create CPU profile file: %w", err)
}
if err := pprof.StartCPUProfile(f); err != nil {
if err := f.Close(); err != nil {
Expand All @@ -129,7 +129,7 @@ func rootPreRun(*cobra.Command, []string) error {
runtime.SetBlockProfileRate(1)
f, err := os.Create(path.Clean(blockProfile))
if err != nil {
return fmt.Errorf("%w: unable to create block profile file", err)
return fmt.Errorf("unable to create block profile file: %w", err)
}

p := pprof.Lookup("block")
Expand Down Expand Up @@ -355,7 +355,7 @@ func initConfig() {
}

if err != nil {
log.Fatalf("%s: unable to load configuration", err.Error())
log.Fatalf("unable to load configuration: %s", err.Error())
}

// Override node url in configuration file when it's explicitly set via CLI
Expand Down Expand Up @@ -407,7 +407,7 @@ func ensureDataDirectoryExists() {
if len(Config.DataDirectory) == 0 {
tmpDir, err := utils.CreateTempDir()
if err != nil {
log.Fatalf("%s: unable to create temporary directory", err.Error())
log.Fatalf("unable to create temporary directory: %s", err.Error())
}

Config.DataDirectory = tmpDir
Expand Down
Loading

0 comments on commit e7a9ebd

Please sign in to comment.