Skip to content

Commit c9a4642

Browse files
EmyrkEmyrk
authored
chore: Update BE http errors to be ui friendly (coder#1994)
* chore: More UI friendly errors Mainly capitlization + messages prefix error
1 parent 847e2b1 commit c9a4642

42 files changed

Lines changed: 544 additions & 317 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎cli/autostart.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ func autostartShow() *cobra.Command {
5555
validSchedule, err := schedule.Weekly(*workspace.AutostartSchedule)
5656
if err != nil {
5757
// This should never happen.
58-
_, _ = fmt.Fprintf(cmd.OutOrStdout(), "invalid autostart schedule %q for workspace %s: %s\n", *workspace.AutostartSchedule, workspace.Name, err.Error())
58+
_, _ = fmt.Fprintf(cmd.OutOrStdout(), "Invalid autostart schedule %q for workspace %s: %s\n", *workspace.AutostartSchedule, workspace.Name, err.Error())
5959
return nil
6060
}
6161

‎cli/autostart_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ func TestAutostart(t *testing.T) {
108108
clitest.SetupConfig(t, client, root)
109109

110110
err := cmd.Execute()
111-
require.ErrorContains(t, err, "status code 403: forbidden", "unexpected error")
111+
require.ErrorContains(t, err, "status code 403: Forbidden", "unexpected error")
112112
})
113113

114114
t.Run("Disable_NotFound", func(t *testing.T) {
@@ -125,7 +125,7 @@ func TestAutostart(t *testing.T) {
125125
clitest.SetupConfig(t, client, root)
126126

127127
err := cmd.Execute()
128-
require.ErrorContains(t, err, "status code 403: forbidden", "unexpected error")
128+
require.ErrorContains(t, err, "status code 403: Forbidden", "unexpected error")
129129
})
130130

131131
t.Run("Enable_DefaultSchedule", func(t *testing.T) {

‎cli/ttl_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ func TestTTL(t *testing.T) {
149149
clitest.SetupConfig(t, client, root)
150150

151151
err := cmd.Execute()
152-
require.ErrorContains(t, err, "status code 403: forbidden", "unexpected error")
152+
require.ErrorContains(t, err, "status code 403: Forbidden", "unexpected error")
153153
})
154154

155155
t.Run("Unset_NotFound", func(t *testing.T) {
@@ -166,6 +166,6 @@ func TestTTL(t *testing.T) {
166166
clitest.SetupConfig(t, client, root)
167167

168168
err := cmd.Execute()
169-
require.ErrorContains(t, err, "status code 403: forbidden", "unexpected error")
169+
require.ErrorContains(t, err, "status code 403: Forbidden", "unexpected error")
170170
})
171171
}

‎coderd/authorize.go‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,7 @@ func (api *API) Authorize(rw http.ResponseWriter, r *http.Request, action rbac.A
2121
roles := httpmw.AuthorizationUserRoles(r)
2222
err := api.Authorizer.ByRoleName(r.Context(), roles.ID.String(), roles.Roles, action, object.RBACObject())
2323
if err != nil {
24-
httpapi.Write(rw, http.StatusForbidden, httpapi.Response{
25-
Message: err.Error(),
26-
})
24+
httpapi.Forbidden(rw)
2725

2826
// Log the errors for debugging
2927
internalError := new(rbac.UnauthorizedError)

‎coderd/csp.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@ func (api *API) logReportCSPViolations(rw http.ResponseWriter, r *http.Request)
2323
if err != nil {
2424
api.Logger.Warn(ctx, "csp violation", slog.Error(err))
2525
httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{
26-
Message: "failed to read body",
26+
Message: "Failed to read body, invalid json",
27+
Detail: err.Error(),
2728
})
2829
return
2930
}

‎coderd/files.go‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ func (api *API) postFile(rw http.ResponseWriter, r *http.Request) {
3232
case "application/x-tar":
3333
default:
3434
httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{
35-
Message: fmt.Sprintf("unsupported content type: %s", contentType),
35+
Message: fmt.Sprintf("Unsupported content type header %q", contentType),
3636
})
3737
return
3838
}
@@ -41,7 +41,8 @@ func (api *API) postFile(rw http.ResponseWriter, r *http.Request) {
4141
data, err := io.ReadAll(r.Body)
4242
if err != nil {
4343
httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{
44-
Message: fmt.Sprintf("read file: %s", err),
44+
Message: "Failed to read file from request",
45+
Detail: err.Error(),
4546
})
4647
return
4748
}
@@ -64,7 +65,8 @@ func (api *API) postFile(rw http.ResponseWriter, r *http.Request) {
6465
})
6566
if err != nil {
6667
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
67-
Message: fmt.Sprintf("insert file: %s", err),
68+
Message: "Internal error saving file",
69+
Detail: err.Error(),
6870
})
6971
return
7072
}
@@ -78,7 +80,7 @@ func (api *API) fileByHash(rw http.ResponseWriter, r *http.Request) {
7880
hash := chi.URLParam(r, "hash")
7981
if hash == "" {
8082
httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{
81-
Message: "hash must be provided",
83+
Message: "File hash must be provided in url",
8284
})
8385
return
8486
}
@@ -89,7 +91,8 @@ func (api *API) fileByHash(rw http.ResponseWriter, r *http.Request) {
8991
}
9092
if err != nil {
9193
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
92-
Message: fmt.Sprintf("get file: %s", err),
94+
Message: "Internal error fetching file",
95+
Detail: err.Error(),
9396
})
9497
return
9598
}

‎coderd/gitsshkey.go‎

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package coderd
22

33
import (
4-
"fmt"
54
"net/http"
65

76
"github.com/coder/coder/coderd/database"
@@ -22,7 +21,8 @@ func (api *API) regenerateGitSSHKey(rw http.ResponseWriter, r *http.Request) {
2221
privateKey, publicKey, err := gitsshkey.Generate(api.SSHKeygenAlgorithm)
2322
if err != nil {
2423
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
25-
Message: fmt.Sprintf("regenerate key pair: %s", err),
24+
Message: "Internal error generating a new SSH keypair",
25+
Detail: err.Error(),
2626
})
2727
return
2828
}
@@ -35,15 +35,17 @@ func (api *API) regenerateGitSSHKey(rw http.ResponseWriter, r *http.Request) {
3535
})
3636
if err != nil {
3737
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
38-
Message: fmt.Sprintf("update git SSH key: %s", err),
38+
Message: "Internal error updating user's git SSH key",
39+
Detail: err.Error(),
3940
})
4041
return
4142
}
4243

4344
newKey, err := api.Database.GetGitSSHKey(r.Context(), user.ID)
4445
if err != nil {
4546
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
46-
Message: fmt.Sprintf("get git SSH key: %s", err),
47+
Message: "Internal error fetching user's git SSH key",
48+
Detail: err.Error(),
4749
})
4850
return
4951
}
@@ -67,7 +69,8 @@ func (api *API) gitSSHKey(rw http.ResponseWriter, r *http.Request) {
6769
gitSSHKey, err := api.Database.GetGitSSHKey(r.Context(), user.ID)
6870
if err != nil {
6971
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
70-
Message: fmt.Sprintf("update git SSH key: %s", err),
72+
Message: "Internal error fetching user's SSH key",
73+
Detail: err.Error(),
7174
})
7275
return
7376
}
@@ -86,31 +89,35 @@ func (api *API) agentGitSSHKey(rw http.ResponseWriter, r *http.Request) {
8689
resource, err := api.Database.GetWorkspaceResourceByID(r.Context(), agent.ResourceID)
8790
if err != nil {
8891
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
89-
Message: fmt.Sprintf("getting workspace resources: %s", err),
92+
Message: "Internal error fetching workspace resource",
93+
Detail: err.Error(),
9094
})
9195
return
9296
}
9397

9498
job, err := api.Database.GetWorkspaceBuildByJobID(r.Context(), resource.JobID)
9599
if err != nil {
96100
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
97-
Message: fmt.Sprintf("getting workspace build: %s", err),
101+
Message: "Internal error fetching workspace build",
102+
Detail: err.Error(),
98103
})
99104
return
100105
}
101106

102107
workspace, err := api.Database.GetWorkspaceByID(r.Context(), job.WorkspaceID)
103108
if err != nil {
104109
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
105-
Message: fmt.Sprintf("getting workspace: %s", err),
110+
Message: "Internal error fetching workspace",
111+
Detail: err.Error(),
106112
})
107113
return
108114
}
109115

110116
gitSSHKey, err := api.Database.GetGitSSHKey(r.Context(), workspace.OwnerID)
111117
if err != nil {
112118
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
113-
Message: fmt.Sprintf("getting git SSH key: %s", err),
119+
Message: "Internal error fetching git SSH key",
120+
Detail: err.Error(),
114121
})
115122
return
116123
}

‎coderd/httpapi/httpapi.go‎

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,22 @@ func init() {
5252

5353
// Response represents a generic HTTP response.
5454
type Response struct {
55-
Message string `json:"message" validate:"required"`
56-
Errors []Error `json:"errors,omitempty" validate:"required"`
55+
// Message is an actionable message that depicts actions the request took.
56+
// These messages should be fully formed sentences with proper punctuation.
57+
// Examples:
58+
// - "A user has been created."
59+
// - "Failed to create a user."
60+
Message string `json:"message"`
61+
// Detail is a debug message that provides further insight into why the
62+
// action failed. This information can be technical and a regular golang
63+
// err.Error() text.
64+
// - "database: too many open connections"
65+
// - "stat: too many open files"
66+
Detail string `json:"detail"`
67+
// Validations are form field-specific friendly error messages. They will be
68+
// shown on a form field in the UI. These can also be used to add additional
69+
// context if there is a set of errors in the primary 'Message'.
70+
Validations []Error `json:"errors,omitempty"`
5771
}
5872

5973
// Error represents a scoped error to a user input.
@@ -64,7 +78,7 @@ type Error struct {
6478

6579
func Forbidden(rw http.ResponseWriter) {
6680
Write(rw, http.StatusForbidden, Response{
67-
Message: "forbidden",
81+
Message: "Forbidden",
6882
})
6983
}
7084

@@ -93,7 +107,8 @@ func Read(rw http.ResponseWriter, r *http.Request, value interface{}) bool {
93107
err := json.NewDecoder(r.Body).Decode(value)
94108
if err != nil {
95109
Write(rw, http.StatusBadRequest, Response{
96-
Message: fmt.Sprintf("read body: %s", err.Error()),
110+
Message: "Request body must be valid JSON",
111+
Detail: err.Error(),
97112
})
98113
return false
99114
}
@@ -108,14 +123,15 @@ func Read(rw http.ResponseWriter, r *http.Request, value interface{}) bool {
108123
})
109124
}
110125
Write(rw, http.StatusBadRequest, Response{
111-
Message: "Validation failed",
112-
Errors: apiErrors,
126+
Message: "Validation failed",
127+
Validations: apiErrors,
113128
})
114129
return false
115130
}
116131
if err != nil {
117132
Write(rw, http.StatusInternalServerError, Response{
118-
Message: fmt.Sprintf("validation: %s", err.Error()),
133+
Message: "Internal error validating request body payload",
134+
Detail: err.Error(),
119135
})
120136
return false
121137
}

‎coderd/httpapi/httpapi_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,9 +74,9 @@ func TestRead(t *testing.T) {
7474
var v httpapi.Response
7575
err := json.NewDecoder(rw.Body).Decode(&v)
7676
require.NoError(t, err)
77-
require.Len(t, v.Errors, 1)
78-
require.Equal(t, "value", v.Errors[0].Field)
79-
require.Equal(t, "Validation failed for tag \"required\" with value: \"\"", v.Errors[0].Detail)
77+
require.Len(t, v.Validations, 1)
78+
require.Equal(t, "value", v.Validations[0].Field)
79+
require.Equal(t, "Validation failed for tag \"required\" with value: \"\"", v.Validations[0].Detail)
8080
})
8181
}
8282

‎coderd/httpmw/apikey.go‎

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -65,15 +65,15 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs) func(http.Handler) h
6565
}
6666
if cookieValue == "" {
6767
httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
68-
Message: fmt.Sprintf("%q cookie or query parameter must be provided", SessionTokenKey),
68+
Message: fmt.Sprintf("Cookie %q or query parameter must be provided", SessionTokenKey),
6969
})
7070
return
7171
}
7272
parts := strings.Split(cookieValue, "-")
7373
// APIKeys are formatted: ID-SECRET
7474
if len(parts) != 2 {
7575
httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
76-
Message: fmt.Sprintf("invalid %q cookie api key format", SessionTokenKey),
76+
Message: fmt.Sprintf("Invalid %q cookie API key format", SessionTokenKey),
7777
})
7878
return
7979
}
@@ -82,26 +82,27 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs) func(http.Handler) h
8282
// Ensuring key lengths are valid.
8383
if len(keyID) != 10 {
8484
httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
85-
Message: fmt.Sprintf("invalid %q cookie api key id", SessionTokenKey),
85+
Message: fmt.Sprintf("Invalid %q cookie API key id", SessionTokenKey),
8686
})
8787
return
8888
}
8989
if len(keySecret) != 22 {
9090
httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
91-
Message: fmt.Sprintf("invalid %q cookie api key secret", SessionTokenKey),
91+
Message: fmt.Sprintf("Invalid %q cookie API key secret", SessionTokenKey),
9292
})
9393
return
9494
}
9595
key, err := db.GetAPIKeyByID(r.Context(), keyID)
9696
if err != nil {
9797
if errors.Is(err, sql.ErrNoRows) {
9898
httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
99-
Message: "api key is invalid",
99+
Message: "API key is invalid",
100100
})
101101
return
102102
}
103103
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
104-
Message: fmt.Sprintf("get api key by id: %s", err.Error()),
104+
Message: "Internal error fetching API key by id",
105+
Detail: err.Error(),
105106
})
106107
return
107108
}
@@ -110,7 +111,7 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs) func(http.Handler) h
110111
// Checking to see if the secret is valid.
111112
if subtle.ConstantTimeCompare(key.HashedSecret, hashed[:]) != 1 {
112113
httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
113-
Message: "api key secret is invalid",
114+
Message: "API key secret is invalid",
114115
})
115116
return
116117
}
@@ -127,7 +128,7 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs) func(http.Handler) h
127128
oauthConfig = oauth.Github
128129
default:
129130
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
130-
Message: fmt.Sprintf("unexpected authentication type %q", key.LoginType),
131+
Message: fmt.Sprintf("Unexpected authentication type %q", key.LoginType),
131132
})
132133
return
133134
}
@@ -139,7 +140,8 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs) func(http.Handler) h
139140
}).Token()
140141
if err != nil {
141142
httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
142-
Message: fmt.Sprintf("couldn't refresh expired oauth token: %s", err.Error()),
143+
Message: "Could not refresh expired Oauth token",
144+
Detail: err.Error(),
143145
})
144146
return
145147
}
@@ -154,7 +156,7 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs) func(http.Handler) h
154156
// Checking if the key is expired.
155157
if key.ExpiresAt.Before(now) {
156158
httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
157-
Message: fmt.Sprintf("api key expired at %q", key.ExpiresAt.String()),
159+
Message: fmt.Sprintf("API key expired at %q", key.ExpiresAt.String()),
158160
})
159161
return
160162
}
@@ -182,7 +184,7 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs) func(http.Handler) h
182184
})
183185
if err != nil {
184186
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
185-
Message: fmt.Sprintf("api key couldn't update: %s", err.Error()),
187+
Message: fmt.Sprintf("API key couldn't update: %s", err.Error()),
186188
})
187189
return
188190
}
@@ -194,14 +196,15 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs) func(http.Handler) h
194196
roles, err := db.GetAuthorizationUserRoles(r.Context(), key.UserID)
195197
if err != nil {
196198
httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
197-
Message: "roles not found",
199+
Message: "Internal error fetching user's roles",
200+
Detail: err.Error(),
198201
})
199202
return
200203
}
201204

202205
if roles.Status != database.UserStatusActive {
203206
httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
204-
Message: fmt.Sprintf("user is not active (status = %q), contact an admin to reactivate your account", roles.Status),
207+
Message: fmt.Sprintf("User is not active (status = %q). Contact an admin to reactivate your account.", roles.Status),
205208
})
206209
return
207210
}

0 commit comments

Comments
 (0)