From 4a78bade6d6a21ff8402d61dc48bc534a4f02e14 Mon Sep 17 00:00:00 2001 From: Abhineet Jain Date: Mon, 23 May 2022 14:51:49 -0400 Subject: [PATCH] bug: Cleaner error message for non logged-in users (#1670) * add helper text to unauthorized error messages * fix lint error, add unit tests * fix test name * fix test name * fix lint errors in test * add unauthorized test for templates create * remove unnecessary variable * remove Error struct, change error message * change [url] to --- cli/root.go | 10 ++++++++ cli/userlist_test.go | 53 +++++++++++++++++++++++++++++----------- coderd/templates_test.go | 14 +++++++++++ codersdk/client.go | 16 ++++++++++++ 4 files changed, 79 insertions(+), 14 deletions(-) diff --git a/cli/root.go b/cli/root.go index 909176f48b..7398986608 100644 --- a/cli/root.go +++ b/cli/root.go @@ -7,6 +7,8 @@ import ( "strings" "time" + "golang.org/x/xerrors" + "github.com/kirsle/configdir" "github.com/mattn/go-isatty" "github.com/spf13/cobra" @@ -113,6 +115,10 @@ func createClient(cmd *cobra.Command) (*codersdk.Client, error) { if err != nil || rawURL == "" { rawURL, err = root.URL().Read() if err != nil { + // If the configuration files are absent, the user is logged out + if os.IsNotExist(err) { + return nil, xerrors.New("You are not logged in. Try logging in using 'coder login '.") + } return nil, err } } @@ -124,6 +130,10 @@ func createClient(cmd *cobra.Command) (*codersdk.Client, error) { if err != nil || token == "" { token, err = root.Session().Read() if err != nil { + // If the configuration files are absent, the user is logged out + if os.IsNotExist(err) { + return nil, xerrors.New("You are not logged in. Try logging in using 'coder login '.") + } return nil, err } } diff --git a/cli/userlist_test.go b/cli/userlist_test.go index 76f9488ec9..8529f6980f 100644 --- a/cli/userlist_test.go +++ b/cli/userlist_test.go @@ -7,22 +7,47 @@ import ( "github.com/coder/coder/cli/clitest" "github.com/coder/coder/coderd/coderdtest" + "github.com/coder/coder/codersdk" "github.com/coder/coder/pty/ptytest" ) func TestUserList(t *testing.T) { - t.Parallel() - client := coderdtest.New(t, nil) - coderdtest.CreateFirstUser(t, client) - cmd, root := clitest.New(t, "users", "list") - clitest.SetupConfig(t, client, root) - pty := ptytest.New(t) - cmd.SetIn(pty.Input()) - cmd.SetOut(pty.Output()) - errC := make(chan error) - go func() { - errC <- cmd.Execute() - }() - require.NoError(t, <-errC) - pty.ExpectMatch("coder.com") + t.Run("List", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, nil) + coderdtest.CreateFirstUser(t, client) + cmd, root := clitest.New(t, "users", "list") + clitest.SetupConfig(t, client, root) + pty := ptytest.New(t) + cmd.SetIn(pty.Input()) + cmd.SetOut(pty.Output()) + errC := make(chan error) + go func() { + errC <- cmd.Execute() + }() + require.NoError(t, <-errC) + pty.ExpectMatch("coder.com") + }) + t.Run("NoURLFileErrorHasHelperText", func(t *testing.T) { + t.Parallel() + + cmd, _ := clitest.New(t, "users", "list") + + _, err := cmd.ExecuteC() + + require.Contains(t, err.Error(), "Try logging in using 'coder login '.") + }) + t.Run("SessionAuthErrorHasHelperText", func(t *testing.T) { + t.Parallel() + + client := coderdtest.New(t, nil) + cmd, root := clitest.New(t, "users", "list") + clitest.SetupConfig(t, client, root) + + _, err := cmd.ExecuteC() + + var apiErr *codersdk.Error + require.ErrorAs(t, err, &apiErr) + require.Contains(t, err.Error(), "Try logging in using 'coder login '.") + }) } diff --git a/coderd/templates_test.go b/coderd/templates_test.go index 24c4012039..fa7eb0c02b 100644 --- a/coderd/templates_test.go +++ b/coderd/templates_test.go @@ -59,6 +59,20 @@ func TestPostTemplateByOrganization(t *testing.T) { require.Equal(t, http.StatusConflict, apiErr.StatusCode()) }) + t.Run("Unauthorized", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, nil) + _, err := client.CreateTemplate(context.Background(), uuid.New(), codersdk.CreateTemplateRequest{ + Name: "test", + VersionID: uuid.New(), + }) + + var apiErr *codersdk.Error + require.ErrorAs(t, err, &apiErr) + require.Equal(t, http.StatusUnauthorized, apiErr.StatusCode()) + require.Contains(t, err.Error(), "Try logging in using 'coder login '.") + }) + t.Run("NoVersion", func(t *testing.T) { t.Parallel() client := coderdtest.New(t, nil) diff --git a/codersdk/client.go b/codersdk/client.go index 0367c912a5..2735c1900a 100644 --- a/codersdk/client.go +++ b/codersdk/client.go @@ -125,6 +125,14 @@ func (c *Client) dialWebsocket(ctx context.Context, path string) (*websocket.Con // wraps it in a codersdk.Error type for easy marshaling. func readBodyAsError(res *http.Response) error { contentType := res.Header.Get("Content-Type") + + var helper string + if res.StatusCode == http.StatusUnauthorized { + // 401 means the user is not logged in + // 403 would mean that the user is not authorized + helper = "Try logging in using 'coder login '." + } + if strings.HasPrefix(contentType, "text/plain") { resp, err := io.ReadAll(res.Body) if err != nil { @@ -135,6 +143,7 @@ func readBodyAsError(res *http.Response) error { Response: httpapi.Response{ Message: string(resp), }, + Helper: helper, } } @@ -146,6 +155,7 @@ func readBodyAsError(res *http.Response) error { // If no body is sent, we'll just provide the status code. return &Error{ statusCode: res.StatusCode, + Helper: helper, } } return xerrors.Errorf("decode body: %w", err) @@ -153,6 +163,7 @@ func readBodyAsError(res *http.Response) error { return &Error{ Response: m, statusCode: res.StatusCode, + Helper: helper, } } @@ -162,6 +173,8 @@ type Error struct { httpapi.Response statusCode int + + Helper string } func (e *Error) StatusCode() int { @@ -171,6 +184,9 @@ func (e *Error) StatusCode() int { func (e *Error) Error() string { var builder strings.Builder _, _ = fmt.Fprintf(&builder, "status code %d: %s", e.statusCode, e.Message) + if e.Helper != "" { + _, _ = fmt.Fprintf(&builder, ": %s", e.Helper) + } for _, err := range e.Errors { _, _ = fmt.Fprintf(&builder, "\n\t%s: %s", err.Field, err.Detail) }