Skip to content

Commit

Permalink
chore(cli): improve cli errors (#2177)
Browse files Browse the repository at this point in the history
Improve cli errors

issue: none
  • Loading branch information
corverroos authored Oct 15, 2024
1 parent 66fa167 commit a8d6e2e
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 32 deletions.
22 changes: 10 additions & 12 deletions cli/cmd/omni/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,24 +15,22 @@ import (

func main() {
cmd := clicmd.New()
libcmd.WrapRunE(cmd, func(ctx context.Context, err error) {
if cliErr := new(clicmd.CliError); errors.As(err, &cliErr) {
// Easy on the eyes CLI error message with suggestions.
log.Error(ctx, "❌ "+cliErr.Error(), nil)
} else {
log.Error(ctx, "❌ Error", err)
}
})

fig := figure.NewFigure("omni", "", true)
cmd.SetHelpTemplate(fig.String() + "\n" + cmd.HelpTemplate())

libcmd.SilenceErrUsage(cmd)

ctx := log.WithCLILogger(context.Background())

err := cmd.ExecuteContext(ctx)
if err == nil {
return
}
cliErr := new(clicmd.CliError)
if errors.As(err, &cliErr) {
log.Error(ctx, "❌ "+cliErr.Error(), nil)
} else {
log.Error(ctx, "❌ Fatal error", err)
if err != nil {
os.Exit(1)
}

os.Exit(1)
}
3 changes: 2 additions & 1 deletion halo/cmd/cmd_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ func TestInvalidCmds(t *testing.T) {
{
Name: "no args",
Args: []string{},
ErrContains: "no sub-command specified, see --help",
ErrContains: "no sub-command specified",
},
{
Name: "invalid args",
Expand All @@ -194,6 +194,7 @@ func TestInvalidCmds(t *testing.T) {
t.Run(test.Name, func(t *testing.T) {
t.Parallel()
rootCmd := libcmd.NewRootCmd("halo", "", New())
libcmd.WrapRunE(rootCmd, func(ctx context.Context, err error) {})
rootCmd.SetArgs(test.Args)
err := rootCmd.Execute()
require.Error(t, err)
Expand Down
1 change: 0 additions & 1 deletion halo/cmd/testdata/TestCLIReference_halo.golden
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
Halo is a consensus client implementation for the Omni Protocol

Usage:
halo [flags]
halo [command]

Available Commands:
Expand Down
4 changes: 3 additions & 1 deletion lib/buildinfo/buildinfo.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func NewVersionCmd() *cobra.Command {
Use: "version",
Short: "Print the version information of this binary",
Args: cobra.NoArgs,
Run: func(cmd *cobra.Command, _ []string) {
RunE: func(cmd *cobra.Command, _ []string) error {
commit, timestamp := get()

var sb strings.Builder
Expand All @@ -62,6 +62,8 @@ func NewVersionCmd() *cobra.Command {
_, _ = sb.WriteString("\n")

cmd.Printf(sb.String()) //nolint:govet // Not a problem

return nil
},
}
}
Expand Down
37 changes: 21 additions & 16 deletions lib/cmd/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ import (
// libcmd.Main(appcmd.New())
// }
func Main(cmd *cobra.Command) {
wrapRunE(cmd)
WrapRunE(cmd, func(ctx context.Context, err error) {
log.Error(ctx, "!! Fatal error occurred, app died !!", err)
})

ctx, cancel := signal.NotifyContext(context.Background(), syscall.SIGINT, syscall.SIGTERM)
err := cmd.ExecuteContext(ctx)
Expand All @@ -39,23 +41,30 @@ func Main(cmd *cobra.Command) {
}
}

// wrapRunE wraps all (nested) RunE functions adding omni logging (stack traces + structured errors).
// WrapRunE wraps all (nested) RunE functions adding omni logging (stack traces + structured errors).
// Done here, so cobra command/flag errors are cli style (usage + simple errors).
func wrapRunE(cmd *cobra.Command) {
runE := cmd.RunE
cmd.RunE = func(cmd *cobra.Command, args []string) error {
err := runE(cmd, args)
if err != nil {
cmd.SilenceUsage = true
cmd.SilenceErrors = true
log.Error(cmd.Context(), "!! Fatal error occurred, app died !!", err)
func WrapRunE(cmd *cobra.Command, printFatal func(ctx context.Context, err error)) {
if cmd.RunE == nil {
// Cobra style error with usage if no sub-command specified.
cmd.RunE = func(*cobra.Command, []string) error {
return errors.New("no sub-command specified")
}
} else {
runE := cmd.RunE
cmd.RunE = func(cmd *cobra.Command, args []string) error {
err := runE(cmd, args)
if err != nil {
cmd.SilenceUsage = true
cmd.SilenceErrors = true
printFatal(cmd.Context(), err)
}

return err
return err
}
}

for _, subCmd := range cmd.Commands() {
wrapRunE(subCmd)
WrapRunE(subCmd, printFatal)
}
}

Expand All @@ -71,10 +80,6 @@ func NewRootCmd(appName string, appDescription string, subCmds ...*cobra.Command
// So they get proper toml+env config.
return initializeConfig(appName, cmd)
},
RunE: func(*cobra.Command, []string) error {
// Callers should either add sub-commands or override RunE.
return errors.New("no sub-command specified, see --help")
},
}

root.AddCommand(subCmds...)
Expand Down
2 changes: 1 addition & 1 deletion lib/merkle/msg_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ func TestMsgTree(t *testing.T) {

// Ensure msg.LogIndex is increasing
for i := 1; i < len(msgs); i++ {
msgs[i].LogIndex = msgs[i-1].LogIndex + uint64(rand.Intn(1000))
msgs[i].LogIndex = msgs[i-1].LogIndex + 1 + uint64(rand.Intn(1000))
}

tree, err := xchain.NewMsgTree(msgs)
Expand Down

0 comments on commit a8d6e2e

Please sign in to comment.