Skip to content

Commit

Permalink
Improving the handling and printing of errors
Browse files Browse the repository at this point in the history
  • Loading branch information
Rican7 committed Mar 21, 2024
1 parent 45c7919 commit a2ed216
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 29 deletions.
52 changes: 35 additions & 17 deletions lieut.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ func (a *SingleCommandApp) Run(ctx context.Context, arguments []string) int {
}

if err := a.initialize(); err != nil {
return a.printErr(err, false)
return a.handleError(err)
}

return a.execute(ctx, a.exec, a.flags.Args())
Expand Down Expand Up @@ -260,7 +260,7 @@ func (a *MultiCommandApp) Run(ctx context.Context, arguments []string) int {
}

if err := a.initialize(); err != nil {
return a.printErr(err, false)
return a.handleError(err)
}

return a.execute(ctx, cmd.Executor, cmd.flags.Args())
Expand Down Expand Up @@ -315,14 +315,28 @@ func (a *MultiCommandApp) PrintUsage(commandName string) {

// PrintUsageError prints a standardized usage error to the app's error output.
func (a *SingleCommandApp) PrintUsageError(err error) {
name := a.info.Name
a.printUsageError(name, err)
if err != nil && a.printError(err) {
// Print a spacer line if an error was printed
fmt.Fprintln(a.errOut)
}

a.PrintUsage()

fmt.Fprintf(a.errOut, "\nRun '%s --help' for usage.\n", a.info.Name)
}

// PrintUsageError prints a standardized usage error to the app's error output.
func (a *MultiCommandApp) PrintUsageError(commandName string, err error) {
name := a.fullCommandName(commandName)
a.printUsageError(name, err)

if err != nil && a.printError(err) {
// Print a spacer line if an error was printed
fmt.Fprintln(a.errOut)
}

a.PrintUsage(commandName)

fmt.Fprintf(a.errOut, "\nRun '%s --help' for usage.\n", name)
}

func (a *app) intercept(flagSet *flagSet) bool {
Expand Down Expand Up @@ -366,20 +380,29 @@ func (a *app) execute(ctx context.Context, exec Executor, arguments []string) in

err := exec(ctx, arguments)
if err != nil && !errors.Is(err, context.Canceled) {
return a.printErr(err, true)
return a.handleError(err)
}

return 0
}

func (a *app) printErr(err error, pad bool) int {
msgFmt := "Error: %v\n"
// printError takes an error, prints it with formatting, and then returns
// whether or not any actual error message was printed.
func (a *app) printError(err error) bool {
msg := err.Error()

if pad {
msgFmt = "\n" + msgFmt
if msg == "" {
// Return false to denote that no error was printed
return false
}

fmt.Fprintf(a.errOut, msgFmt, err)
fmt.Fprintf(a.errOut, "Error: %s\n", msg)

return true
}

func (a *app) handleError(err error) int {
a.printError(err)

var statusErr StatusCodeError
if errors.As(err, &statusErr) {
Expand Down Expand Up @@ -414,13 +437,8 @@ func (a *app) printVersion(toErr bool) {
fmt.Fprintf(out, "%s (%s/%s)\n", identifier, runtime.GOOS, runtime.GOARCH)
}

func (a *app) printUsageError(name string, err error) {
fmt.Fprintf(a.errOut, "%s: %v\n", name, err)
fmt.Fprintf(a.errOut, "Run '%s --help' for usage.\n", name)
}

func (a *MultiCommandApp) printUnknownCommand(commandName string) int {
a.printUsageError(a.info.Name, fmt.Errorf("unknown command '%s'", commandName))
a.PrintUsageError("", fmt.Errorf("unknown command '%s'", commandName))

return 1
}
Expand Down
55 changes: 43 additions & 12 deletions lieut_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -508,7 +508,10 @@ func TestSingleCommandApp_PrintUsageError(t *testing.T) {
app := NewSingleCommandApp(testAppInfo, testNoOpExecutor, flagSet, &buf, &buf)

usageErr := errors.New("test usage error")
want := `test: test usage error
want := `Error: test usage error
Usage: test testing
Run 'test --help' for usage.
`

Expand All @@ -517,7 +520,7 @@ Run 'test --help' for usage.
got := buf.String()

if got != want {
t.Errorf("app.PrintVersion gave %q, want %q", got, want)
t.Errorf("app.PrintUsageError gave %q, want %q", got, want)
}
}

Expand All @@ -528,7 +531,10 @@ func TestMultiCommandApp_PrintUsageError(t *testing.T) {
app := NewMultiCommandApp(testAppInfo, flagSet, &buf, &buf)

usageErr := errors.New("test usage error")
want := `test: test usage error
want := `Error: test usage error
Usage: test testing
Run 'test --help' for usage.
`

Expand All @@ -537,7 +543,7 @@ Run 'test --help' for usage.
got := buf.String()

if got != want {
t.Errorf("app.PrintVersion gave %q, want %q", got, want)
t.Errorf("app.PrintUsageError gave %q, want %q", got, want)
}
}

Expand All @@ -551,7 +557,10 @@ func TestMultiCommandApp_PrintUsageError_Command(t *testing.T) {
app.SetCommand(testCommandInfo, nil, nil)

usageErr := errors.New("test usage error")
want := `test testcommand: test usage error
want := `Error: test usage error
Usage: test testcommand [arguments ...]
Run 'test testcommand --help' for usage.
`

Expand All @@ -560,7 +569,7 @@ Run 'test testcommand --help' for usage.
got := buf.String()

if got != want {
t.Errorf("app.PrintVersion gave %q, want %q", got, want)
t.Errorf("app.PrintUsageError gave %q, want %q", got, want)
}
}

Expand Down Expand Up @@ -727,6 +736,17 @@ test vTest (%s/%s)
wantedOut: "",
wantedErrOut: "Error: test init error\n",
},
"initialize returns status code empty error": {
init: func() error {
return ErrWithStatusCode(errors.New(""), 101)
},

args: []string{"test"},

wantedExitCode: 101,
wantedOut: "",
wantedErrOut: "",
},
"execute returns error": {
exec: func(ctx context.Context, arguments []string) error {
return errors.New("test exec error")
Expand All @@ -736,7 +756,7 @@ test vTest (%s/%s)

wantedExitCode: 1,
wantedOut: "",
wantedErrOut: "\nError: test exec error\n",
wantedErrOut: "Error: test exec error\n",
},
"execute returns status code error": {
exec: func(ctx context.Context, arguments []string) error {
Expand All @@ -747,7 +767,18 @@ test vTest (%s/%s)

wantedExitCode: 217,
wantedOut: "",
wantedErrOut: "\nError: test exec error\n",
wantedErrOut: "Error: test exec error\n",
},
"execute returns status code empty error": {
exec: func(ctx context.Context, arguments []string) error {
return ErrWithStatusCode(errors.New(""), 217)
},

args: []string{"test"},

wantedExitCode: 217,
wantedOut: "",
wantedErrOut: "",
},
} {
t.Run(testName, func(t *testing.T) {
Expand Down Expand Up @@ -1014,7 +1045,7 @@ test vTest (%s/%s)

wantedExitCode: 1,
wantedOut: "",
wantedErrOut: "\nError: test exec error\n",
wantedErrOut: "Error: test exec error\n",
},
"execute returns status code error": {
exec: func(ctx context.Context, arguments []string) error {
Expand All @@ -1025,14 +1056,14 @@ test vTest (%s/%s)

wantedExitCode: 217,
wantedOut: "",
wantedErrOut: "\nError: test exec error\n",
wantedErrOut: "Error: test exec error\n",
},
"unknown command": {
args: []string{"thiscommanddoesnotexist"},

wantedExitCode: 1,
wantedOut: "",
wantedErrOut: "test: unknown command 'thiscommanddoesnotexist'\nRun 'test --help' for usage.\n",
wantedErrOut: "Error: unknown command 'thiscommanddoesnotexist'\n\nUsage: test testing\n\nRun 'test --help' for usage.\n",
},
"unknown command is known flag": {
flags: func() Flags {
Expand All @@ -1046,7 +1077,7 @@ test vTest (%s/%s)

wantedExitCode: 1,
wantedOut: "",
wantedErrOut: "test: unknown command '--testflag'\nRun 'test --help' for usage.\n",
wantedErrOut: "Error: unknown command '--testflag'\n\nUsage: test testing\n\nRun 'test --help' for usage.\n",
},
} {
t.Run(testName, func(t *testing.T) {
Expand Down

0 comments on commit a2ed216

Please sign in to comment.