fix(cli): show error/hide help for unsupported subcommands (#10760) (#12624)

Fixes #10760 

The coder CLI quietly accepts any subcommand arguments and silently swallows them.

Currently:
```sh
❯ coder | head -n5
coder v2.3.3+e491217

USAGE:
  coder [global-flags] <subcommand>
```

```sh
❯ coder idontexist | head -n5
coder v2.3.3+e491217

USAGE:
  coder [global-flags] <subcommand>
```

Now help output will not be show when there is an unknown subcommand error. Instead users will be given the command for the help output.

```sh
❯ coder idontexist
Encountered an error running "coder", see "coder --help" for more information 
error: unrecognized subcommand "idontexist"
```
```sh
❯ coder iexistbut idontexist
Encountered an error running "coder iexistbut", see "coder iexistbut --help" for more information 
error: unrecognized subcommand "idontexist"
```

Also this stuff: `Encountered an error running "coder iexistbut"... ` gets written to `os.Stdout` in `prettyErrorFormatter{w: os.Stderr, verbose: r.verbose}`, not sure how to test that output.
This commit is contained in:
elasticspoon 2024-03-17 23:17:43 -04:00 committed by GitHub
parent c189cc93e4
commit 5011edc292
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 69 additions and 16 deletions

View File

@ -4,7 +4,9 @@ import (
"bufio"
_ "embed"
"fmt"
"os"
"regexp"
"slices"
"sort"
"strings"
"text/tabwriter"
@ -320,6 +322,25 @@ var usageWantsArgRe = regexp.MustCompile(`<.*>`)
// output for a given command.
func helpFn() serpent.HandlerFunc {
return func(inv *serpent.Invocation) error {
// Check for invalid subcommands before printing help.
if len(inv.Args) > 0 && !usageWantsArgRe.MatchString(inv.Command.Use) {
_, _ = fmt.Fprintf(inv.Stderr, "---\nerror: unrecognized subcommand %q\n", inv.Args[0])
}
if len(inv.Args) > 0 {
// Return an error so that exit status is non-zero when
// a subcommand is not found.
err := xerrors.Errorf("unrecognized subcommand %q", strings.Join(inv.Args, " "))
if slices.Contains(os.Args, "--help") {
// Subcommand error is not wrapped in RunCommandErr if command
// is invoked with --help with no HelpHandler
return &serpent.RunCommandError{
Cmd: inv.Command,
Err: err,
}
}
return err
}
// We use stdout for help and not stderr since there's no straightforward
// way to distinguish between a user error and a help request.
//
@ -340,9 +361,6 @@ func helpFn() serpent.HandlerFunc {
if err != nil {
return err
}
if len(inv.Args) > 0 && !usageWantsArgRe.MatchString(inv.Command.Use) {
_, _ = fmt.Fprintf(inv.Stderr, "---\nerror: unknown subcommand %q\n", inv.Args[0])
}
return nil
}
}

View File

@ -69,10 +69,6 @@ func (r *RootCmd) portForward() *serpent.Command {
return xerrors.Errorf("parse port-forward specs: %w", err)
}
if len(specs) == 0 {
err = inv.Command.HelpHandler(inv)
if err != nil {
return xerrors.Errorf("generate help output: %w", err)
}
return xerrors.New("no port-forwards requested")
}

View File

@ -35,15 +35,10 @@ func TestPortForward_None(t *testing.T) {
inv, root := clitest.New(t, "port-forward", "blah")
clitest.SetupConfig(t, member, root)
pty := ptytest.New(t).Attach(inv)
inv.Stderr = pty.Output()
err := inv.Run()
require.Error(t, err)
require.ErrorContains(t, err, "no port-forwards")
// Check that the help was printed.
pty.ExpectMatch("port-forward <workspace>")
}
func TestPortForward(t *testing.T) {

View File

@ -1196,10 +1196,13 @@ func formatMultiError(from string, multi []error, opts *formatOpts) string {
// formatter and add it to cliHumanFormatError function.
func formatRunCommandError(err *serpent.RunCommandError, opts *formatOpts) string {
var str strings.Builder
_, _ = str.WriteString(pretty.Sprint(headLineStyle(), fmt.Sprintf("Encountered an error running %q", err.Cmd.FullName())))
_, _ = str.WriteString(pretty.Sprint(headLineStyle(),
fmt.Sprintf(
`Encountered an error running %q, see "%s --help" for more information`,
err.Cmd.FullName(), err.Cmd.FullName())))
_, _ = str.WriteString(pretty.Sprint(headLineStyle(), "\nerror: "))
msgString, special := cliHumanFormatError("", err.Err, opts)
_, _ = str.WriteString("\n")
if special {
_, _ = str.WriteString(msgString)
} else {

View File

@ -60,6 +60,49 @@ func TestCommandHelp(t *testing.T) {
func TestRoot(t *testing.T) {
t.Parallel()
t.Run("MissingRootCommand", func(t *testing.T) {
t.Parallel()
out := new(bytes.Buffer)
inv, _ := clitest.New(t, "idontexist")
inv.Stdout = out
err := inv.Run()
assert.ErrorContains(t, err,
`unrecognized subcommand "idontexist"`)
require.Empty(t, out.String())
})
t.Run("MissingSubcommand", func(t *testing.T) {
t.Parallel()
out := new(bytes.Buffer)
inv, _ := clitest.New(t, "server", "idontexist")
inv.Stdout = out
err := inv.Run()
// subcommand error only when command has subcommands
assert.ErrorContains(t, err,
`unrecognized subcommand "idontexist"`)
require.Empty(t, out.String())
})
t.Run("BadSubcommandArgs", func(t *testing.T) {
t.Parallel()
out := new(bytes.Buffer)
inv, _ := clitest.New(t, "list", "idontexist")
inv.Stdout = out
err := inv.Run()
assert.ErrorContains(t, err,
`wanted no args but got 1 [idontexist]`)
require.Empty(t, out.String())
})
t.Run("Version", func(t *testing.T) {
t.Parallel()

2
go.sum
View File

@ -218,8 +218,6 @@ github.com/coder/pretty v0.0.0-20230908205945-e89ba86370e0 h1:3A0ES21Ke+FxEM8CXx
github.com/coder/pretty v0.0.0-20230908205945-e89ba86370e0/go.mod h1:5UuS2Ts+nTToAMeOjNlnHFkPahrtDkmpydBen/3wgZc=
github.com/coder/retry v1.5.1 h1:iWu8YnD8YqHs3XwqrqsjoBTAVqT9ml6z9ViJ2wlMiqc=
github.com/coder/retry v1.5.1/go.mod h1:blHMk9vs6LkoRT9ZHyuZo360cufXEhrxqvEzeMtRGoY=
github.com/coder/serpent v0.4.0 h1:L/itwnCxfhLutxQ2mScP3tH1ro8z8+Kc/iKKyZZxEMk=
github.com/coder/serpent v0.4.0/go.mod h1:Wud83ikZF/ulbdMcEMAwqvkEIQx7+l47+ef69M/arAA=
github.com/coder/serpent v0.4.1-0.20240315163851-a0148c87ea3f h1:nqJ/Mvm+nLI22n5BIYhvSmTZ6CD+MRo/aGVZwVQgr1k=
github.com/coder/serpent v0.4.1-0.20240315163851-a0148c87ea3f/go.mod h1:REkJ5ZFHQUWFTPLExhXYZ1CaHFjxvGNRlLXLdsI08YA=
github.com/coder/ssh v0.0.0-20231128192721-70855dedb788 h1:YoUSJ19E8AtuUFVYBpXuOD6a/zVP3rcxezNsoDseTUw=