Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

feat: add verbose error messaging#3053

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
sreya merged 6 commits intomainfromjon/fixerrors
Jul 20, 2022
Merged
Show file tree
Hide file tree
Changes fromall commits
Commits
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletionscli/cliflag/cliflag.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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)
Expand DownExpand Up@@ -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)
Expand Down
4 changes: 3 additions & 1 deletioncli/login.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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())
Expand Down
36 changes: 26 additions & 10 deletionscli/root.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,7 +4,6 @@ import (
"fmt"
"net/url"
"os"
"strconv"
"strings"
"text/template"
"time"
Expand DownExpand Up@@ -40,11 +39,12 @@ 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"
envNoVersionCheck = "CODER_NO_VERSION_WARNING"
)

var (
Expand All@@ -58,8 +58,6 @@ func init() {
}

func Root() *cobra.Command {
var varSuppressVersion bool

cmd := &cobra.Command{
Use: "coder",
SilenceErrors: true,
Expand All@@ -68,7 +66,7 @@ func Root() *cobra.Command {
`,
PersistentPreRun: func(cmd *cobra.Command, args []string) {
err := func() error {
ifvarSuppressVersion {
ifcliflag.IsSetBool(cmd, varNoVersionCheck) {
return nil
}

Expand DownExpand Up@@ -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)
Expand All@@ -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
}
Expand DownExpand Up@@ -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 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Happened to notice this function still uses hard-coded version ofvarNoVersionCheck and also has a suppression check (which we're doing in the pre-run as well), do we want to keep both?

Copy link
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Good catch, we don'tneed to keep both checks but I kept the check at the top of the the pre-run to avoid the unnecessary I/O of instantiating a client that we immediately throw away when someone passes the disable-warning flag.

flag := cmd.Flag("no-version-warning")
if suppress, _ := strconv.ParseBool(flag.Value.String()); suppress {
if cliflag.IsSetBool(cmd, varNoVersionCheck) {
return nil
}

Expand Down
101 changes: 97 additions & 4 deletionscli/root_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,22 +4,115 @@ 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()

cmd, _ := clitest.New(t, "delete")
t.Run("OK", func(t *testing.T) {
t.Parallel()

cmd, err := cmd.ExecuteC()
errStr := cli.FormatCobraError(err, cmd)
require.Contains(t, errStr, "Run 'coder delete --help' for usage.")
cmd, _ := clitest.New(t, "delete")

cmd, err := cmd.ExecuteC()
errStr := cli.FormatCobraError(err, cmd)
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) {
Expand Down
4 changes: 4 additions & 0 deletionscodersdk/client.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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 != "" {
Expand Down
[8]ページ先頭
©2009-2025 Movatter.jp