Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error handling improvement for logger and cmd packages #350

Merged
merged 1 commit into from
Sep 13, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 9 additions & 8 deletions cmd/check_construction.go
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
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
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
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
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
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
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