Fix panic in check:configuration command

[t-hale]

Motivation

This PR addresses #269

Solution

Just adds a nil check around https://github.com/coinbase/rosetta-cli/blob/master/pkg/results/construction_results.go#L316

Open questions

I generally don't like for users to see stack traces at all (especially in this case where cobra provides the usage message and a meaningful error). Have you considered making ErrorStackTraceDisabled set to true by default so that the user only sees the usage message, descriptive error and a nonzero return code?

[shrimalmadhur]

@t-hale you will have to gpg sign your commits for me to merge this. Thanks!

[t-hale]

@t-hale you will have to gpg sign your commits for me to merge this. Thanks!

done

[shrimalmadhur]

@t-hale on your open question. By default it is true because this tool is majorly used for testing the rosetta implementation locally. For a user who is testing that, it's good to see the errors right away so that they can fix their implementation quickly. This helps in faster turnaround. We could think of a way where we dump detailed error in a file but till then I think default spitting the stack makes sense. Does it makes sense? lmk your thoughts.

[t-hale]

@t-hale on your open question. By default it is true because this tool is majorly used for testing the rosetta implementation locally. For a user who is testing that, it's good to see the errors right away so that they can fix their implementation quickly. This helps in faster turnaround. We could think of a way where we dump detailed error in a file but till then I think default spitting the stack makes sense. Does it makes sense? lmk your thoughts.

@shrimalmadhur I agree that the errors should be shown quickly and easily for the user to diagnose. In the situation dealing with the check:configuration command, if I run with no arguments:

rosetta-cli check:configuration

I get a substantial amount of output (error message, stack trace, error message, usage and the error message again)

Error: construction configuration is missing
github.com/coinbase/rosetta-cli/pkg/results.ExitConstruction
	/Users/tyler/Documents/github/rosetta-cli/pkg/results/construction_results.go:305
github.com/coinbase/rosetta-cli/cmd.runCheckConstructionCmd
	/Users/tyler/Documents/github/rosetta-cli/cmd/check_construction.go:58
github.com/spf13/cobra.(*Command).execute
	/Users/tyler/go/pkg/mod/github.com/spf13/[email protected]/command.go:856
github.com/spf13/cobra.(*Command).ExecuteC
	/Users/tyler/go/pkg/mod/github.com/spf13/[email protected]/command.go:974
github.com/spf13/cobra.(*Command).Execute
	/Users/tyler/go/pkg/mod/github.com/spf13/[email protected]/command.go:902
github.com/coinbase/rosetta-cli/cmd.Execute
	/Users/tyler/Documents/github/rosetta-cli/cmd/root.go:170
main.main
	/Users/tyler/Documents/github/rosetta-cli/main.go:26
runtime.main
	/usr/local/Cellar/go/1.17.5/libexec/src/runtime/proc.go:255
runtime.goexit
	/usr/local/Cellar/go/1.17.5/libexec/src/runtime/asm_amd64.s:1581

Error: construction configuration is missing
Usage:
  rosetta-cli check:construction [flags]

Flags:
      --asserter-configuration-file string   Check that /network/options matches contents of file at this path
  -h, --help                                 help for check:construction

Global Flags:
      --block-profile string        Save the pprof block profile in the specified file
      --configuration-file string   Configuration file that provides connection and test settings.
                                    If you would like to generate a starter configuration file (populated
                                    with the defaults), run rosetta-cli configuration:create.

                                    Any fields not populated in the configuration file will be populated with
                                    default values.
      --cpu-profile string          Save the pprof cpu profile in the specified file
      --mem-profile string          Save the pprof mem profile in the specified file

Command Failed: construction configuration is missing
exit status 1

This is probably due to my unfamiliarity with the CLI, but it seems like we can cut down on the output logging a bit and use the MarkFlagRequired() function to handle this sooner / automatically within cobra. Are there situations where some of the global flags are required for certain commands, but not for others?

[shrimalmadhur]

@t-hale I think the configuration file (--configuration-file) is definitely required for most of of the commands since it determines the blockchain endpoints and other config. But I agree we could do a better job with having detailed logs in file and have crisp logs to user. I will put this under our charter. Also if you have ideas and some bandwidth, feel free to propose and make changes. We love contributions :)