Skip to content

Commit 446a9c0

Browse files
elasticspoonelasticspoon
committed
fix(cli): show error instead of help for unsupported subcommands (#10760)
The coder CLI quietly accepts any subcommand arguments and silently swallows them. Currently: ❯ coder idontexist | head -n5 coder v2.3.3+e491217 USAGE: coder [global-flags] <subcommand> Now help output will not be show but users will be given the command for the help output. ❯ coder idontexist Encountered an error running "coder", see "coder --help" for more information error: unrecognized subcommand "idontexist" ❯ coder iexistbut idontexist Encountered an error running "coder iexistbut", see "coder iexistbut --help" for more information unrecognized subcommand "idontexist"
1 parent b4c0fa8 commit 446a9c0

File tree

6 files changed

+69
-16
lines changed

6 files changed

+69
-16
lines changed

‎cli/help.go

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@ import (
44
"bufio"
55
_ "embed"
66
"fmt"
7+
"os"
78
"regexp"
9+
"slices"
810
"sort"
911
"strings"
1012
"text/tabwriter"
@@ -320,6 +322,25 @@ var usageWantsArgRe = regexp.MustCompile(`<.*>`)
320322
// output for a given command.
321323
func helpFn() serpent.HandlerFunc {
322324
return func(inv *serpent.Invocation) error {
325+
// Check for invalid subcommands before printing help.
326+
if len(inv.Args) > 0 && !usageWantsArgRe.MatchString(inv.Command.Use) {
327+
_, _ = fmt.Fprintf(inv.Stderr, "---\nerror: unrecognized subcommand %q\n", inv.Args[0])
328+
}
329+
if len(inv.Args) > 0 {
330+
// Return an error so that exit status is non-zero when
331+
// a subcommand is not found.
332+
err := xerrors.Errorf("unrecognized subcommand %q", strings.Join(inv.Args, " "))
333+
if slices.Contains(os.Args, "--help") {
334+
// Subcommand error is not wrapped in RunCommandErr if command
335+
// is invoked with --help with no HelpHandler
336+
return &serpent.RunCommandError{
337+
Cmd: inv.Command,
338+
Err: err,
339+
}
340+
}
341+
return err
342+
}
343+
323344
// We use stdout for help and not stderr since there's no straightforward
324345
// way to distinguish between a user error and a help request.
325346
//
@@ -340,9 +361,6 @@ func helpFn() serpent.HandlerFunc {
340361
if err != nil {
341362
return err
342363
}
343-
if len(inv.Args) > 0 && !usageWantsArgRe.MatchString(inv.Command.Use) {
344-
_, _ = fmt.Fprintf(inv.Stderr, "---\nerror: unknown subcommand %q\n", inv.Args[0])
345-
}
346364
return nil
347365
}
348366
}

‎cli/portforward.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -69,10 +69,6 @@ func (r *RootCmd) portForward() *serpent.Command {
6969
return xerrors.Errorf("parse port-forward specs: %w", err)
7070
}
7171
if len(specs) == 0 {
72-
err = inv.Command.HelpHandler(inv)
73-
if err != nil {
74-
return xerrors.Errorf("generate help output: %w", err)
75-
}
7672
return xerrors.New("no port-forwards requested")
7773
}
7874

‎cli/portforward_test.go

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,15 +35,10 @@ func TestPortForward_None(t *testing.T) {
3535

3636
inv, root := clitest.New(t, "port-forward", "blah")
3737
clitest.SetupConfig(t, member, root)
38-
pty := ptytest.New(t).Attach(inv)
39-
inv.Stderr = pty.Output()
4038

4139
err := inv.Run()
4240
require.Error(t, err)
4341
require.ErrorContains(t, err, "no port-forwards")
44-
45-
// Check that the help was printed.
46-
pty.ExpectMatch("port-forward <workspace>")
4742
}
4843

4944
func TestPortForward(t *testing.T) {

‎cli/root.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1196,10 +1196,13 @@ func formatMultiError(from string, multi []error, opts *formatOpts) string {
11961196
// formatter and add it to cliHumanFormatError function.
11971197
func formatRunCommandError(err *serpent.RunCommandError, opts *formatOpts) string {
11981198
var str strings.Builder
1199-
_, _ = str.WriteString(pretty.Sprint(headLineStyle(), fmt.Sprintf("Encountered an error running %q", err.Cmd.FullName())))
1199+
_, _ = str.WriteString(pretty.Sprint(headLineStyle(),
1200+
fmt.Sprintf(
1201+
`Encountered an error running %q, see "%s --help" for more information`,
1202+
err.Cmd.FullName(), err.Cmd.FullName())))
1203+
_, _ = str.WriteString(pretty.Sprint(headLineStyle(), "\nerror: "))
12001204

12011205
msgString, special := cliHumanFormatError("", err.Err, opts)
1202-
_, _ = str.WriteString("\n")
12031206
if special {
12041207
_, _ = str.WriteString(msgString)
12051208
} else {

‎cli/root_test.go

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,49 @@ func TestCommandHelp(t *testing.T) {
6060

6161
func TestRoot(t *testing.T) {
6262
t.Parallel()
63+
t.Run("MissingRootCommand", func(t *testing.T) {
64+
t.Parallel()
65+
66+
out := new(bytes.Buffer)
67+
68+
inv, _ := clitest.New(t, "idontexist")
69+
inv.Stdout = out
70+
71+
err := inv.Run()
72+
assert.ErrorContains(t, err,
73+
`unrecognized subcommand "idontexist"`)
74+
require.Empty(t, out.String())
75+
})
76+
77+
t.Run("MissingSubcommand", func(t *testing.T) {
78+
t.Parallel()
79+
80+
out := new(bytes.Buffer)
81+
82+
inv, _ := clitest.New(t, "server", "idontexist")
83+
inv.Stdout = out
84+
85+
err := inv.Run()
86+
// subcommand error only when command has subcommands
87+
assert.ErrorContains(t, err,
88+
`unrecognized subcommand "idontexist"`)
89+
require.Empty(t, out.String())
90+
})
91+
92+
t.Run("BadSubcommandArgs", func(t *testing.T) {
93+
t.Parallel()
94+
95+
out := new(bytes.Buffer)
96+
97+
inv, _ := clitest.New(t, "list", "idontexist")
98+
inv.Stdout = out
99+
100+
err := inv.Run()
101+
assert.ErrorContains(t, err,
102+
`wanted no args but got 1 [idontexist]`)
103+
require.Empty(t, out.String())
104+
})
105+
63106
t.Run("Version", func(t *testing.T) {
64107
t.Parallel()
65108

‎go.sum

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -214,8 +214,6 @@ github.com/coder/pretty v0.0.0-20230908205945-e89ba86370e0 h1:3A0ES21Ke+FxEM8CXx
214214
github.com/coder/pretty v0.0.0-20230908205945-e89ba86370e0/go.mod h1:5UuS2Ts+nTToAMeOjNlnHFkPahrtDkmpydBen/3wgZc=
215215
github.com/coder/retry v1.5.1 h1:iWu8YnD8YqHs3XwqrqsjoBTAVqT9ml6z9ViJ2wlMiqc=
216216
github.com/coder/retry v1.5.1/go.mod h1:blHMk9vs6LkoRT9ZHyuZo360cufXEhrxqvEzeMtRGoY=
217-
github.com/coder/serpent v0.4.0 h1:L/itwnCxfhLutxQ2mScP3tH1ro8z8+Kc/iKKyZZxEMk=
218-
github.com/coder/serpent v0.4.0/go.mod h1:Wud83ikZF/ulbdMcEMAwqvkEIQx7+l47+ef69M/arAA=
219217
github.com/coder/serpent v0.4.1-0.20240315163851-a0148c87ea3f h1:nqJ/Mvm+nLI22n5BIYhvSmTZ6CD+MRo/aGVZwVQgr1k=
220218
github.com/coder/serpent v0.4.1-0.20240315163851-a0148c87ea3f/go.mod h1:REkJ5ZFHQUWFTPLExhXYZ1CaHFjxvGNRlLXLdsI08YA=
221219
github.com/coder/ssh v0.0.0-20231128192721-70855dedb788 h1:YoUSJ19E8AtuUFVYBpXuOD6a/zVP3rcxezNsoDseTUw=

0 commit comments

Comments
 (0)