Skip to content

Commit 62e6856

Browse files
sreyasreya
authored
feat: add verbose error messaging (#3053)
1 parent 4a7d067 commit 62e6856

File tree

5 files changed

+170
-15
lines changed

5 files changed

+170
-15
lines changed

‎cli/cliflag/cliflag.go

+40
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,33 @@ import (
1717
"strings"
1818
"time"
1919

20+
"github.com/spf13/cobra"
2021
"github.com/spf13/pflag"
2122
)
2223

24+
// IsSetBool returns the value of the boolean flag if it is set.
25+
// It returns false if the flag isn't set or if any error occurs attempting
26+
// to parse the value of the flag.
27+
func IsSetBool(cmd *cobra.Command, name string) bool {
28+
val, ok := IsSet(cmd, name)
29+
if !ok {
30+
return false
31+
}
32+
33+
b, err := strconv.ParseBool(val)
34+
return err == nil && b
35+
}
36+
37+
// IsSet returns the string value of the flag and whether it was set.
38+
func IsSet(cmd *cobra.Command, name string) (string, bool) {
39+
flag := cmd.Flag(name)
40+
if flag == nil {
41+
return "", false
42+
}
43+
44+
return flag.Value.String(), flag.Changed
45+
}
46+
2347
// String sets a string flag on the given flag set.
2448
func String(flagset *pflag.FlagSet, name, shorthand, env, def, usage string) {
2549
v, ok := os.LookupEnv(env)
@@ -67,6 +91,22 @@ func Uint8VarP(flagset *pflag.FlagSet, ptr *uint8, name string, shorthand string
6791
flagset.Uint8VarP(ptr, name, shorthand, uint8(vi64), fmtUsage(usage, env))
6892
}
6993

94+
func Bool(flagset *pflag.FlagSet, name, shorthand, env string, def bool, usage string) {
95+
val, ok := os.LookupEnv(env)
96+
if !ok || val == "" {
97+
flagset.BoolP(name, shorthand, def, fmtUsage(usage, env))
98+
return
99+
}
100+
101+
valb, err := strconv.ParseBool(val)
102+
if err != nil {
103+
flagset.BoolP(name, shorthand, def, fmtUsage(usage, env))
104+
return
105+
}
106+
107+
flagset.BoolP(name, shorthand, valb, fmtUsage(usage, env))
108+
}
109+
70110
// BoolVarP sets a bool flag on the given flag set.
71111
func BoolVarP(flagset *pflag.FlagSet, ptr *bool, name string, shorthand string, env string, def bool, usage string) {
72112
val, ok := os.LookupEnv(env)

‎cli/login.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,9 @@ func login() *cobra.Command {
7373
// on a very old client.
7474
err = checkVersions(cmd, client)
7575
if err != nil {
76-
return xerrors.Errorf("check versions: %w", err)
76+
// Checking versions isn't a fatal error so we print a warning
77+
// and proceed.
78+
_, _ = fmt.Fprintln(cmd.ErrOrStderr(), cliui.Styles.Warn.Render(err.Error()))
7779
}
7880

7981
hasInitialUser, err := client.HasFirstUser(cmd.Context())

‎cli/root.go

+26-10
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"fmt"
55
"net/url"
66
"os"
7-
"strconv"
87
"strings"
98
"text/template"
109
"time"
@@ -40,11 +39,12 @@ const (
4039
varAgentURL = "agent-url"
4140
varGlobalConfig = "global-config"
4241
varNoOpen = "no-open"
42+
varNoVersionCheck = "no-version-warning"
4343
varForceTty = "force-tty"
44+
varVerbose = "verbose"
4445
notLoggedInMessage = "You are not logged in. Try logging in using 'coder login <url>'."
4546

46-
noVersionCheckFlag = "no-version-warning"
47-
envNoVersionCheck = "CODER_NO_VERSION_WARNING"
47+
envNoVersionCheck = "CODER_NO_VERSION_WARNING"
4848
)
4949

5050
var (
@@ -58,8 +58,6 @@ func init() {
5858
}
5959

6060
func Root() *cobra.Command {
61-
var varSuppressVersion bool
62-
6361
cmd := &cobra.Command{
6462
Use: "coder",
6563
SilenceErrors: true,
@@ -68,7 +66,7 @@ func Root() *cobra.Command {
6866
`,
6967
PersistentPreRun: func(cmd *cobra.Command, args []string) {
7068
err := func() error {
71-
if varSuppressVersion {
69+
if cliflag.IsSetBool(cmd, varNoVersionCheck) {
7270
return nil
7371
}
7472

@@ -141,7 +139,7 @@ func Root() *cobra.Command {
141139
cmd.SetUsageTemplate(usageTemplate())
142140

143141
cmd.PersistentFlags().String(varURL, "", "Specify the URL to your deployment.")
144-
cliflag.BoolVarP(cmd.PersistentFlags(), &varSuppressVersion, noVersionCheckFlag, "", envNoVersionCheck, false, "Suppress warning when client and server versions do not match.")
142+
cliflag.Bool(cmd.PersistentFlags(), varNoVersionCheck, "", envNoVersionCheck, false, "Suppress warning when client and server versions do not match.")
145143
cliflag.String(cmd.PersistentFlags(), varToken, "", envSessionToken, "", fmt.Sprintf("Specify an authentication token. For security reasons setting %s is preferred.", envSessionToken))
146144
cliflag.String(cmd.PersistentFlags(), varAgentToken, "", "CODER_AGENT_TOKEN", "", "Specify an agent authentication token.")
147145
_ = cmd.PersistentFlags().MarkHidden(varAgentToken)
@@ -152,6 +150,7 @@ func Root() *cobra.Command {
152150
_ = cmd.PersistentFlags().MarkHidden(varForceTty)
153151
cmd.PersistentFlags().Bool(varNoOpen, false, "Block automatically opening URLs in the browser.")
154152
_ = cmd.PersistentFlags().MarkHidden(varNoOpen)
153+
cliflag.Bool(cmd.PersistentFlags(), varVerbose, "v", "CODER_VERBOSE", false, "Enable verbose output")
155154

156155
return cmd
157156
}
@@ -427,12 +426,29 @@ func formatExamples(examples ...example) string {
427426
// FormatCobraError colorizes and adds "--help" docs to cobra commands.
428427
func FormatCobraError(err error, cmd *cobra.Command) string {
429428
helpErrMsg := fmt.Sprintf("Run '%s --help' for usage.", cmd.CommandPath())
430-
return cliui.Styles.Error.Render(err.Error() + "\n" + helpErrMsg)
429+
430+
var (
431+
httpErr *codersdk.Error
432+
output strings.Builder
433+
)
434+
435+
if xerrors.As(err, &httpErr) {
436+
_, _ = fmt.Fprintln(&output, httpErr.Friendly())
437+
}
438+
439+
// If the httpErr is nil then we just have a regular error in which
440+
// case we want to print out what's happening.
441+
if httpErr == nil || cliflag.IsSetBool(cmd, varVerbose) {
442+
_, _ = fmt.Fprintln(&output, err.Error())
443+
}
444+
445+
_, _ = fmt.Fprint(&output, helpErrMsg)
446+
447+
return cliui.Styles.Error.Render(output.String())
431448
}
432449

433450
func checkVersions(cmd *cobra.Command, client *codersdk.Client) error {
434-
flag := cmd.Flag("no-version-warning")
435-
if suppress, _ := strconv.ParseBool(flag.Value.String()); suppress {
451+
if cliflag.IsSetBool(cmd, varNoVersionCheck) {
436452
return nil
437453
}
438454

‎cli/root_test.go

+97-4
Original file line numberDiff line numberDiff line change
@@ -4,22 +4,115 @@ import (
44
"bytes"
55
"testing"
66

7+
"github.com/spf13/cobra"
78
"github.com/stretchr/testify/require"
9+
"golang.org/x/xerrors"
810

911
"github.com/coder/coder/buildinfo"
1012
"github.com/coder/coder/cli"
1113
"github.com/coder/coder/cli/clitest"
14+
"github.com/coder/coder/codersdk"
1215
)
1316

1417
func TestRoot(t *testing.T) {
1518
t.Run("FormatCobraError", func(t *testing.T) {
1619
t.Parallel()
1720

18-
cmd, _ := clitest.New(t, "delete")
21+
t.Run("OK", func(t *testing.T) {
22+
t.Parallel()
1923

20-
cmd, err := cmd.ExecuteC()
21-
errStr := cli.FormatCobraError(err, cmd)
22-
require.Contains(t, errStr, "Run 'coder delete --help' for usage.")
24+
cmd, _ := clitest.New(t, "delete")
25+
26+
cmd, err := cmd.ExecuteC()
27+
errStr := cli.FormatCobraError(err, cmd)
28+
require.Contains(t, errStr, "Run 'coder delete --help' for usage.")
29+
})
30+
31+
t.Run("Verbose", func(t *testing.T) {
32+
t.Parallel()
33+
34+
// Test that the verbose error is masked without verbose flag.
35+
t.Run("NoVerboseAPIError", func(t *testing.T) {
36+
t.Parallel()
37+
38+
cmd, _ := clitest.New(t)
39+
40+
cmd.RunE = func(cmd *cobra.Command, args []string) error {
41+
var err error = &codersdk.Error{
42+
Response: codersdk.Response{
43+
Message: "This is a message.",
44+
},
45+
Helper: "Try this instead.",
46+
}
47+
48+
err = xerrors.Errorf("wrap me: %w", err)
49+
50+
return err
51+
}
52+
53+
cmd, err := cmd.ExecuteC()
54+
errStr := cli.FormatCobraError(err, cmd)
55+
require.Contains(t, errStr, "This is a message. Try this instead.")
56+
require.NotContains(t, errStr, err.Error())
57+
})
58+
59+
// Assert that a regular error is not masked when verbose is not
60+
// specified.
61+
t.Run("NoVerboseRegularError", func(t *testing.T) {
62+
t.Parallel()
63+
64+
cmd, _ := clitest.New(t)
65+
66+
cmd.RunE = func(cmd *cobra.Command, args []string) error {
67+
return xerrors.Errorf("this is a non-codersdk error: %w", xerrors.Errorf("a wrapped error"))
68+
}
69+
70+
cmd, err := cmd.ExecuteC()
71+
errStr := cli.FormatCobraError(err, cmd)
72+
require.Contains(t, errStr, err.Error())
73+
})
74+
75+
// Test that both the friendly error and the verbose error are
76+
// displayed when verbose is passed.
77+
t.Run("APIError", func(t *testing.T) {
78+
t.Parallel()
79+
80+
cmd, _ := clitest.New(t, "--verbose")
81+
82+
cmd.RunE = func(cmd *cobra.Command, args []string) error {
83+
var err error = &codersdk.Error{
84+
Response: codersdk.Response{
85+
Message: "This is a message.",
86+
},
87+
Helper: "Try this instead.",
88+
}
89+
90+
err = xerrors.Errorf("wrap me: %w", err)
91+
92+
return err
93+
}
94+
95+
cmd, err := cmd.ExecuteC()
96+
errStr := cli.FormatCobraError(err, cmd)
97+
require.Contains(t, errStr, "This is a message. Try this instead.")
98+
require.Contains(t, errStr, err.Error())
99+
})
100+
101+
// Assert that a regular error is not masked when verbose specified.
102+
t.Run("RegularError", func(t *testing.T) {
103+
t.Parallel()
104+
105+
cmd, _ := clitest.New(t, "--verbose")
106+
107+
cmd.RunE = func(cmd *cobra.Command, args []string) error {
108+
return xerrors.Errorf("this is a non-codersdk error: %w", xerrors.Errorf("a wrapped error"))
109+
}
110+
111+
cmd, err := cmd.ExecuteC()
112+
errStr := cli.FormatCobraError(err, cmd)
113+
require.Contains(t, errStr, err.Error())
114+
})
115+
})
23116
})
24117

25118
t.Run("Version", func(t *testing.T) {

‎codersdk/client.go

+4
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,10 @@ func (e *Error) StatusCode() int {
185185
return e.statusCode
186186
}
187187

188+
func (e *Error) Friendly() string {
189+
return fmt.Sprintf("%s. %s", strings.TrimSuffix(e.Message, "."), e.Helper)
190+
}
191+
188192
func (e *Error) Error() string {
189193
var builder strings.Builder
190194
if e.method != "" && e.url != "" {

0 commit comments

Comments
 (0)