Skip to content

Commit 4a78bad

Browse files
AbhineetJainAbhineetJain
authored
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 <url>
1 parent c543fca commit 4a78bad

File tree

4 files changed

+79
-14
lines changed

4 files changed

+79
-14
lines changed

‎cli/root.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ import (
77
"strings"
88
"time"
99

10+
"golang.org/x/xerrors"
11+
1012
"github.com/kirsle/configdir"
1113
"github.com/mattn/go-isatty"
1214
"github.com/spf13/cobra"
@@ -113,6 +115,10 @@ func createClient(cmd *cobra.Command) (*codersdk.Client, error) {
113115
if err != nil || rawURL == "" {
114116
rawURL, err = root.URL().Read()
115117
if err != nil {
118+
// If the configuration files are absent, the user is logged out
119+
if os.IsNotExist(err) {
120+
return nil, xerrors.New("You are not logged in. Try logging in using 'coder login <url>'.")
121+
}
116122
return nil, err
117123
}
118124
}
@@ -124,6 +130,10 @@ func createClient(cmd *cobra.Command) (*codersdk.Client, error) {
124130
if err != nil || token == "" {
125131
token, err = root.Session().Read()
126132
if err != nil {
133+
// If the configuration files are absent, the user is logged out
134+
if os.IsNotExist(err) {
135+
return nil, xerrors.New("You are not logged in. Try logging in using 'coder login <url>'.")
136+
}
127137
return nil, err
128138
}
129139
}

‎cli/userlist_test.go

Lines changed: 39 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -7,22 +7,47 @@ import (
77

88
"github.com/coder/coder/cli/clitest"
99
"github.com/coder/coder/coderd/coderdtest"
10+
"github.com/coder/coder/codersdk"
1011
"github.com/coder/coder/pty/ptytest"
1112
)
1213

1314
func TestUserList(t *testing.T) {
14-
t.Parallel()
15-
client := coderdtest.New(t, nil)
16-
coderdtest.CreateFirstUser(t, client)
17-
cmd, root := clitest.New(t, "users", "list")
18-
clitest.SetupConfig(t, client, root)
19-
pty := ptytest.New(t)
20-
cmd.SetIn(pty.Input())
21-
cmd.SetOut(pty.Output())
22-
errC := make(chan error)
23-
go func() {
24-
errC <- cmd.Execute()
25-
}()
26-
require.NoError(t, <-errC)
27-
pty.ExpectMatch("coder.com")
15+
t.Run("List", func(t *testing.T) {
16+
t.Parallel()
17+
client := coderdtest.New(t, nil)
18+
coderdtest.CreateFirstUser(t, client)
19+
cmd, root := clitest.New(t, "users", "list")
20+
clitest.SetupConfig(t, client, root)
21+
pty := ptytest.New(t)
22+
cmd.SetIn(pty.Input())
23+
cmd.SetOut(pty.Output())
24+
errC := make(chan error)
25+
go func() {
26+
errC <- cmd.Execute()
27+
}()
28+
require.NoError(t, <-errC)
29+
pty.ExpectMatch("coder.com")
30+
})
31+
t.Run("NoURLFileErrorHasHelperText", func(t *testing.T) {
32+
t.Parallel()
33+
34+
cmd, _ := clitest.New(t, "users", "list")
35+
36+
_, err := cmd.ExecuteC()
37+
38+
require.Contains(t, err.Error(), "Try logging in using 'coder login <url>'.")
39+
})
40+
t.Run("SessionAuthErrorHasHelperText", func(t *testing.T) {
41+
t.Parallel()
42+
43+
client := coderdtest.New(t, nil)
44+
cmd, root := clitest.New(t, "users", "list")
45+
clitest.SetupConfig(t, client, root)
46+
47+
_, err := cmd.ExecuteC()
48+
49+
var apiErr *codersdk.Error
50+
require.ErrorAs(t, err, &apiErr)
51+
require.Contains(t, err.Error(), "Try logging in using 'coder login <url>'.")
52+
})
2853
}

‎coderd/templates_test.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,20 @@ func TestPostTemplateByOrganization(t *testing.T) {
5959
require.Equal(t, http.StatusConflict, apiErr.StatusCode())
6060
})
6161

62+
t.Run("Unauthorized", func(t *testing.T) {
63+
t.Parallel()
64+
client := coderdtest.New(t, nil)
65+
_, err := client.CreateTemplate(context.Background(), uuid.New(), codersdk.CreateTemplateRequest{
66+
Name: "test",
67+
VersionID: uuid.New(),
68+
})
69+
70+
var apiErr *codersdk.Error
71+
require.ErrorAs(t, err, &apiErr)
72+
require.Equal(t, http.StatusUnauthorized, apiErr.StatusCode())
73+
require.Contains(t, err.Error(), "Try logging in using 'coder login <url>'.")
74+
})
75+
6276
t.Run("NoVersion", func(t *testing.T) {
6377
t.Parallel()
6478
client := coderdtest.New(t, nil)

‎codersdk/client.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,14 @@ func (c *Client) dialWebsocket(ctx context.Context, path string) (*websocket.Con
125125
// wraps it in a codersdk.Error type for easy marshaling.
126126
func readBodyAsError(res *http.Response) error {
127127
contentType := res.Header.Get("Content-Type")
128+
129+
var helper string
130+
if res.StatusCode == http.StatusUnauthorized {
131+
// 401 means the user is not logged in
132+
// 403 would mean that the user is not authorized
133+
helper = "Try logging in using 'coder login <url>'."
134+
}
135+
128136
if strings.HasPrefix(contentType, "text/plain") {
129137
resp, err := io.ReadAll(res.Body)
130138
if err != nil {
@@ -135,6 +143,7 @@ func readBodyAsError(res *http.Response) error {
135143
Response: httpapi.Response{
136144
Message: string(resp),
137145
},
146+
Helper: helper,
138147
}
139148
}
140149

@@ -146,13 +155,15 @@ func readBodyAsError(res *http.Response) error {
146155
// If no body is sent, we'll just provide the status code.
147156
return &Error{
148157
statusCode: res.StatusCode,
158+
Helper: helper,
149159
}
150160
}
151161
return xerrors.Errorf("decode body: %w", err)
152162
}
153163
return &Error{
154164
Response: m,
155165
statusCode: res.StatusCode,
166+
Helper: helper,
156167
}
157168
}
158169

@@ -162,6 +173,8 @@ type Error struct {
162173
httpapi.Response
163174

164175
statusCode int
176+
177+
Helper string
165178
}
166179

167180
func (e *Error) StatusCode() int {
@@ -171,6 +184,9 @@ func (e *Error) StatusCode() int {
171184
func (e *Error) Error() string {
172185
var builder strings.Builder
173186
_, _ = fmt.Fprintf(&builder, "status code %d: %s", e.statusCode, e.Message)
187+
if e.Helper != "" {
188+
_, _ = fmt.Fprintf(&builder, ": %s", e.Helper)
189+
}
174190
for _, err := range e.Errors {
175191
_, _ = fmt.Fprintf(&builder, "\n\t%s: %s", err.Field, err.Detail)
176192
}

0 commit comments

Comments
 (0)