diff --git a/modules/bonfire/README.md b/modules/bonfire/README.md index 8799893f..18992c38 100644 --- a/modules/bonfire/README.md +++ b/modules/bonfire/README.md @@ -46,7 +46,7 @@ Use cases: - Hidden commands (`bonfire.Command.Hidden`) run but stay out of the list unless `list --all`. - JSON catalog: `list --format=json` prints every command, hidden ones flagged, with arguments, options, defaults and examples as a `bonfire.Listing` (`bonfire.Describe` builds it from a command slice). - Documentation coverage: `bonfire.CheckDocs` reports commands lacking a description, long help, examples, or argument and option descriptions. -- Error block and exit codes: `bonfire.Execute` runs the root through fang and returns `bonfire.ExitOK`, `bonfire.ExitFailure`, or `bonfire.ExitUsage` for an unknown command or flag and a wrong number of arguments (`bonfire.UsageError`). Errors print on the error stream as a block (`bonfire.RenderError`): the ERROR badge and the message, the explanation and Try lines of `bonfire.WithHint` (`{bin}` becomes `bin/` or `summer`), cobra's "did you mean" suggestions as Try lines, and "Run with -v for details." or, at `-v`, the wrapped error chain; every message passes through `sunscreen.Scrub` and every line through `bonfire.Sanitize`, which strips control characters. Errors wrapped by `bonfire.Reported` print nothing. An error carrying a code through `bonfire.WithExitCode` exits with that code; summer uses it to exit with the code of an app binary it ran. `bonfire.Fail` prints errors that happen before the root exists. For an application binary (any root other than `summer`) that fails, other than with a usage error, while the working directory has no `config/` directory and a parent directory holds `summer.yaml`, both add the hint "This command runs from the application root." with `cd ; bin/ ` (only the message changes); the values of flags named like a password, secret, token or key are masked as `***` in that line, also a value that starts with `-`. +- Error block and exit codes: `bonfire.Execute` runs the root through fang and returns `bonfire.ExitOK`, `bonfire.ExitFailure`, or `bonfire.ExitUsage` for an unknown command or flag and a wrong number of arguments (`bonfire.UsageError`). Errors print on the error stream as a block (`bonfire.RenderError`): the ERROR badge and the message, the explanation and Try lines of `bonfire.WithHint` (`{bin}` becomes `bin/` or `summer`), for an unknown command (recognised by `bonfire.Execute` from cobra's own error, never from the wording of a command's error) `Command "x" is not defined.` with cobra's "did you mean" suggestions as Try lines, and "Run with -v for details." or, at `-v`, the wrapped error chain; every message passes through `sunscreen.Scrub` and every line through `bonfire.Sanitize`, which strips control characters. Errors wrapped by `bonfire.Reported` print nothing. An error carrying a code through `bonfire.WithExitCode` exits with that code; summer uses it to exit with the code of an app binary it ran. `bonfire.Fail` prints errors that happen before the root exists. For an application binary (any root other than `summer`) that fails, other than with a usage error, while the working directory has no `config/` directory and a parent directory holds `summer.yaml`, both add the hint "This command runs from the application root." with `cd ; bin/ ` (only the message changes); the values of flags named like a password, secret, token or key are masked as `***` in that line, also a value that starts with `-`. - Name checks at boot: two commands with the same name, or a command named `list` or `help`, make `bonfire.NewRoot` fail with `bonfire.ErrDuplicateCommand`; a command flag that reuses a global option's name or shorthand fails with `bonfire.ErrReservedFlag` instead of panicking. - Injectable streams (`bonfire.NewRootIO`, `bonfire.NewOutput`) so commands can be tested against buffers. - In-process calls: `bonfire.Call` runs a named command with arguments against any writer (Laravel `Artisan::call`), and `bonfire.Catalog` holds an application binary's final command list so code outside the Cobra root, such as the conga scheduler, can call any registered command. diff --git a/modules/bonfire/errors.go b/modules/bonfire/errors.go index f43316b3..c3999edf 100644 --- a/modules/bonfire/errors.go +++ b/modules/bonfire/errors.go @@ -12,6 +12,7 @@ import ( "git.golem15.com/golem15/summercms/internal/argmask" "git.golem15.com/golem15/summercms/modules/sunscreen" "github.com/charmbracelet/colorprofile" + "github.com/spf13/cobra" ) // UsageError marks a mistake in how a command was called: an unknown @@ -40,13 +41,39 @@ func Usage(err error) error { return &UsageError{Err: err} } -// isUsage reports a UsageError in the chain or cobra's unknown command error. +// isUsage reports a UsageError in the chain. cobra's unknown command error +// counts only after Execute has converted it (unknownCommand). func isUsage(err error) bool { var ue *UsageError - if errors.As(err, &ue) { - return true + return errors.As(err, &ue) +} + +// unknownCommandError is cobra's unknown command error once Execute has +// confirmed it came from cobra: Error keeps cobra's text, name and +// suggestions are read from it once. +type unknownCommandError struct { + msg string + name string + suggestions []string +} + +func (e *unknownCommandError) Error() string { return e.msg } + +// unknownCommand converts err into a UsageError around an +// unknownCommandError when it is cobra's unknown command error for args: +// root.Find on the same args fails at the root with the same message, so +// cobra rejected the command word and no command ran. Every other error, +// including a command's own error worded the same way, is returned as is. +func unknownCommand(root *cobra.Command, args []string, err error) error { + if err == nil || root == nil { + return err } - return strings.HasPrefix(err.Error(), "unknown command") + cmd, _, ferr := root.Find(args) + if ferr == nil || cmd != root || ferr.Error() != err.Error() { + return err + } + name, suggestions := parseUnknownCommand(err.Error()) + return &UsageError{Err: &unknownCommandError{msg: err.Error(), name: name, suggestions: suggestions}} } // HintError adds an explanation and a command to try to an error. The error @@ -169,9 +196,10 @@ func WithExitCode(err error, code int) error { // and the message, the explanation and the Try lines of a HintError ({bin} // replaced by summer or bin/, for the root named name), then "Run with // -v for details." at verbosity 0 or the wrapped error chain under "Caused -// by:" at -v and above. cobra's unknown command error becomes `Command "x" -// is not defined.` with its suggestions as Try lines, and other usage errors -// suggest the command's help. Every message passes through sunscreen.Scrub, +// by:" at -v and above. An unknown command, as recognised by Execute, +// becomes `Command "x" is not defined.` with cobra's suggestions as Try +// lines, and other usage errors suggest the command's help. A raw cobra +// error passed in directly keeps cobra's text. Every message passes through sunscreen.Scrub, // and every line through Sanitize. // Errors wrapped by Reported print nothing. func RenderError(w io.Writer, name string, err error, g Globals) { @@ -189,12 +217,12 @@ func RenderError(w io.Writer, name string, err error, g Globals) { if errors.As(err, &hint) { explanation, try = hint.Explanation, hint.Try } - if strings.HasPrefix(message, "unknown command") { - cmdName, suggestions := parseUnknownCommand(message) - message = `Command "` + cmdName + `" is not defined.` + var unknown *unknownCommandError + if errors.As(err, &unknown) { + message = `Command "` + unknown.name + `" is not defined.` if try == "" { var lines []string - for _, s := range suggestions { + for _, s := range unknown.suggestions { lines = append(lines, "{bin} "+s) } if len(lines) == 0 { diff --git a/modules/bonfire/errors_test.go b/modules/bonfire/errors_test.go index ccdce45e..8727759f 100644 --- a/modules/bonfire/errors_test.go +++ b/modules/bonfire/errors_test.go @@ -342,3 +342,34 @@ func TestSanitize(t *testing.T) { t.Errorf("Sanitize(%q) = %q, want it unchanged", clean, got) } } + +// TestPluginErrorWordedUnknownCommand (IN-03): only cobra's own unknown +// command error is a usage error. A command whose Run returns an error +// worded the same way exits 1 and is printed verbatim, while a mistyped +// command still exits 2 with `Command "x" is not defined.` and cobra's +// suggestions as Try lines. +func TestPluginErrorWordedUnknownCommand(t *testing.T) { + plainEnv(t) + commands := []Command{ + {Name: "migrate", Description: "Migrate", Run: func(context.Context, Input, Output) error { + return errors.New(`unknown command "x" for "acme"`) + }}, + } + _, errOut, code := runGolden(t, commands, "migrate") + if code != ExitFailure { + t.Errorf("plugin error: exit %d, want %d\n%s", code, ExitFailure, errOut) + } + if !strings.Contains(errOut, `unknown command "x" for "acme"`) || strings.Contains(errOut, "is not defined") { + t.Errorf("plugin error not printed verbatim:\n%s", errOut) + } + + _, errOut, code = runGolden(t, commands, "migrat") + if code != ExitUsage { + t.Errorf("migrat: exit %d, want %d\n%s", code, ExitUsage, errOut) + } + for _, want := range []string{`Command "migrat" is not defined.`, "Try:", "bin/acme migrate"} { + if !strings.Contains(errOut, want) { + t.Errorf("migrat: block lacks %q:\n%s", want, errOut) + } + } +} diff --git a/modules/bonfire/execute.go b/modules/bonfire/execute.go index 977e7016..24ee8462 100644 --- a/modules/bonfire/execute.go +++ b/modules/bonfire/execute.go @@ -54,10 +54,11 @@ func Execute(ctx context.Context, root *cobra.Command, args []string) int { // fang hands over a colorprofile writer built from the // environment only; RenderError unwraps it and applies // --ansi and --no-ansi itself. + err = unknownCommand(root, args, err) RenderError(w, name, wrongDirectory(name, err, args, ""), g) }), ) - return exitCode(err) + return exitCode(unknownCommand(root, args, err)) } // Fail renders err like Execute does and returns ExitFailure (ExitOK for a diff --git a/scripts/check-phase19.8.sh b/scripts/check-phase19.8.sh index 58f83bae..2a9dcd68 100755 --- a/scripts/check-phase19.8.sh +++ b/scripts/check-phase19.8.sh @@ -131,6 +131,8 @@ BONFIRE_TESTS=( TestProcessViewStripsControl TestTableStripsControl # quick 261022-4z8 (re-review fixes): IN-15, WR-09. TestNestedProcessBuildsNoProgram TestPrintAboveAfterCancel + # quick 261028-rqd (info findings): IN-03. + TestPluginErrorWordedUnknownCommand ) DOCSITE_TESTS=( # Plan 04: the docs checker reads past the global options.