feat: add verbose error messaging (#3053)

This commit is contained in:
Jon Ayers
2022-07-20 15:17:51 -05:00
committed by GitHub
parent 4a7d067c6c
commit 62e685669f
5 changed files with 170 additions and 15 deletions

View File

@ -17,9 +17,33 @@ import (
"strings"
"time"
"github.com/spf13/cobra"
"github.com/spf13/pflag"
)
// IsSetBool returns the value of the boolean flag if it is set.
// It returns false if the flag isn't set or if any error occurs attempting
// to parse the value of the flag.
func IsSetBool(cmd *cobra.Command, name string) bool {
val, ok := IsSet(cmd, name)
if !ok {
return false
}
b, err := strconv.ParseBool(val)
return err == nil && b
}
// IsSet returns the string value of the flag and whether it was set.
func IsSet(cmd *cobra.Command, name string) (string, bool) {
flag := cmd.Flag(name)
if flag == nil {
return "", false
}
return flag.Value.String(), flag.Changed
}
// String sets a string flag on the given flag set.
func String(flagset *pflag.FlagSet, name, shorthand, env, def, usage string) {
v, ok := os.LookupEnv(env)
@ -67,6 +91,22 @@ func Uint8VarP(flagset *pflag.FlagSet, ptr *uint8, name string, shorthand string
flagset.Uint8VarP(ptr, name, shorthand, uint8(vi64), fmtUsage(usage, env))
}
func Bool(flagset *pflag.FlagSet, name, shorthand, env string, def bool, usage string) {
val, ok := os.LookupEnv(env)
if !ok || val == "" {
flagset.BoolP(name, shorthand, def, fmtUsage(usage, env))
return
}
valb, err := strconv.ParseBool(val)
if err != nil {
flagset.BoolP(name, shorthand, def, fmtUsage(usage, env))
return
}
flagset.BoolP(name, shorthand, valb, fmtUsage(usage, env))
}
// BoolVarP sets a bool flag on the given flag set.
func BoolVarP(flagset *pflag.FlagSet, ptr *bool, name string, shorthand string, env string, def bool, usage string) {
val, ok := os.LookupEnv(env)

View File

@ -73,7 +73,9 @@ func login() *cobra.Command {
// on a very old client.
err = checkVersions(cmd, client)
if err != nil {
return xerrors.Errorf("check versions: %w", err)
// Checking versions isn't a fatal error so we print a warning
// and proceed.
_, _ = fmt.Fprintln(cmd.ErrOrStderr(), cliui.Styles.Warn.Render(err.Error()))
}
hasInitialUser, err := client.HasFirstUser(cmd.Context())

View File

@ -4,7 +4,6 @@ import (
"fmt"
"net/url"
"os"
"strconv"
"strings"
"text/template"
"time"
@ -40,10 +39,11 @@ const (
varAgentURL = "agent-url"
varGlobalConfig = "global-config"
varNoOpen = "no-open"
varNoVersionCheck = "no-version-warning"
varForceTty = "force-tty"
varVerbose = "verbose"
notLoggedInMessage = "You are not logged in. Try logging in using 'coder login <url>'."
noVersionCheckFlag = "no-version-warning"
envNoVersionCheck = "CODER_NO_VERSION_WARNING"
)
@ -58,8 +58,6 @@ func init() {
}
func Root() *cobra.Command {
var varSuppressVersion bool
cmd := &cobra.Command{
Use: "coder",
SilenceErrors: true,
@ -68,7 +66,7 @@ func Root() *cobra.Command {
`,
PersistentPreRun: func(cmd *cobra.Command, args []string) {
err := func() error {
if varSuppressVersion {
if cliflag.IsSetBool(cmd, varNoVersionCheck) {
return nil
}
@ -141,7 +139,7 @@ func Root() *cobra.Command {
cmd.SetUsageTemplate(usageTemplate())
cmd.PersistentFlags().String(varURL, "", "Specify the URL to your deployment.")
cliflag.BoolVarP(cmd.PersistentFlags(), &varSuppressVersion, noVersionCheckFlag, "", envNoVersionCheck, false, "Suppress warning when client and server versions do not match.")
cliflag.Bool(cmd.PersistentFlags(), varNoVersionCheck, "", envNoVersionCheck, false, "Suppress warning when client and server versions do not match.")
cliflag.String(cmd.PersistentFlags(), varToken, "", envSessionToken, "", fmt.Sprintf("Specify an authentication token. For security reasons setting %s is preferred.", envSessionToken))
cliflag.String(cmd.PersistentFlags(), varAgentToken, "", "CODER_AGENT_TOKEN", "", "Specify an agent authentication token.")
_ = cmd.PersistentFlags().MarkHidden(varAgentToken)
@ -152,6 +150,7 @@ func Root() *cobra.Command {
_ = cmd.PersistentFlags().MarkHidden(varForceTty)
cmd.PersistentFlags().Bool(varNoOpen, false, "Block automatically opening URLs in the browser.")
_ = cmd.PersistentFlags().MarkHidden(varNoOpen)
cliflag.Bool(cmd.PersistentFlags(), varVerbose, "v", "CODER_VERBOSE", false, "Enable verbose output")
return cmd
}
@ -427,12 +426,29 @@ func formatExamples(examples ...example) string {
// FormatCobraError colorizes and adds "--help" docs to cobra commands.
func FormatCobraError(err error, cmd *cobra.Command) string {
helpErrMsg := fmt.Sprintf("Run '%s --help' for usage.", cmd.CommandPath())
return cliui.Styles.Error.Render(err.Error() + "\n" + helpErrMsg)
var (
httpErr *codersdk.Error
output strings.Builder
)
if xerrors.As(err, &httpErr) {
_, _ = fmt.Fprintln(&output, httpErr.Friendly())
}
// If the httpErr is nil then we just have a regular error in which
// case we want to print out what's happening.
if httpErr == nil || cliflag.IsSetBool(cmd, varVerbose) {
_, _ = fmt.Fprintln(&output, err.Error())
}
_, _ = fmt.Fprint(&output, helpErrMsg)
return cliui.Styles.Error.Render(output.String())
}
func checkVersions(cmd *cobra.Command, client *codersdk.Client) error {
flag := cmd.Flag("no-version-warning")
if suppress, _ := strconv.ParseBool(flag.Value.String()); suppress {
if cliflag.IsSetBool(cmd, varNoVersionCheck) {
return nil
}

View File

@ -4,17 +4,23 @@ import (
"bytes"
"testing"
"github.com/spf13/cobra"
"github.com/stretchr/testify/require"
"golang.org/x/xerrors"
"github.com/coder/coder/buildinfo"
"github.com/coder/coder/cli"
"github.com/coder/coder/cli/clitest"
"github.com/coder/coder/codersdk"
)
func TestRoot(t *testing.T) {
t.Run("FormatCobraError", func(t *testing.T) {
t.Parallel()
t.Run("OK", func(t *testing.T) {
t.Parallel()
cmd, _ := clitest.New(t, "delete")
cmd, err := cmd.ExecuteC()
@ -22,6 +28,93 @@ func TestRoot(t *testing.T) {
require.Contains(t, errStr, "Run 'coder delete --help' for usage.")
})
t.Run("Verbose", func(t *testing.T) {
t.Parallel()
// Test that the verbose error is masked without verbose flag.
t.Run("NoVerboseAPIError", func(t *testing.T) {
t.Parallel()
cmd, _ := clitest.New(t)
cmd.RunE = func(cmd *cobra.Command, args []string) error {
var err error = &codersdk.Error{
Response: codersdk.Response{
Message: "This is a message.",
},
Helper: "Try this instead.",
}
err = xerrors.Errorf("wrap me: %w", err)
return err
}
cmd, err := cmd.ExecuteC()
errStr := cli.FormatCobraError(err, cmd)
require.Contains(t, errStr, "This is a message. Try this instead.")
require.NotContains(t, errStr, err.Error())
})
// Assert that a regular error is not masked when verbose is not
// specified.
t.Run("NoVerboseRegularError", func(t *testing.T) {
t.Parallel()
cmd, _ := clitest.New(t)
cmd.RunE = func(cmd *cobra.Command, args []string) error {
return xerrors.Errorf("this is a non-codersdk error: %w", xerrors.Errorf("a wrapped error"))
}
cmd, err := cmd.ExecuteC()
errStr := cli.FormatCobraError(err, cmd)
require.Contains(t, errStr, err.Error())
})
// Test that both the friendly error and the verbose error are
// displayed when verbose is passed.
t.Run("APIError", func(t *testing.T) {
t.Parallel()
cmd, _ := clitest.New(t, "--verbose")
cmd.RunE = func(cmd *cobra.Command, args []string) error {
var err error = &codersdk.Error{
Response: codersdk.Response{
Message: "This is a message.",
},
Helper: "Try this instead.",
}
err = xerrors.Errorf("wrap me: %w", err)
return err
}
cmd, err := cmd.ExecuteC()
errStr := cli.FormatCobraError(err, cmd)
require.Contains(t, errStr, "This is a message. Try this instead.")
require.Contains(t, errStr, err.Error())
})
// Assert that a regular error is not masked when verbose specified.
t.Run("RegularError", func(t *testing.T) {
t.Parallel()
cmd, _ := clitest.New(t, "--verbose")
cmd.RunE = func(cmd *cobra.Command, args []string) error {
return xerrors.Errorf("this is a non-codersdk error: %w", xerrors.Errorf("a wrapped error"))
}
cmd, err := cmd.ExecuteC()
errStr := cli.FormatCobraError(err, cmd)
require.Contains(t, errStr, err.Error())
})
})
})
t.Run("Version", func(t *testing.T) {
t.Parallel()

View File

@ -185,6 +185,10 @@ func (e *Error) StatusCode() int {
return e.statusCode
}
func (e *Error) Friendly() string {
return fmt.Sprintf("%s. %s", strings.TrimSuffix(e.Message, "."), e.Helper)
}
func (e *Error) Error() string {
var builder strings.Builder
if e.method != "" && e.url != "" {