coder · Emyrk · Jun 3, 2022 · Jun 2, 2022 · Jun 3, 2022 · Jun 3, 2022
diff --git a/cli/autostart.go b/cli/autostart.go
@@ -55,7 +55,7 @@ func autostartShow() *cobra.Command {
 			validSchedule, err := schedule.Weekly(*workspace.AutostartSchedule)
 			if err != nil {
 				// This should never happen.
-				_, _ = fmt.Fprintf(cmd.OutOrStdout(), "invalid autostart schedule %q for workspace %s: %s\n", *workspace.AutostartSchedule, workspace.Name, err.Error())
+				_, _ = fmt.Fprintf(cmd.OutOrStdout(), "Invalid autostart schedule %q for workspace %s: %s\n", *workspace.AutostartSchedule, workspace.Name, err.Error())
 				return nil
 			}
 

diff --git a/cli/autostart_test.go b/cli/autostart_test.go
@@ -108,7 +108,7 @@ func TestAutostart(t *testing.T) {
 		clitest.SetupConfig(t, client, root)
 
 		err := cmd.Execute()
-		require.ErrorContains(t, err, "status code 403: forbidden", "unexpected error")
+		require.ErrorContains(t, err, "status code 403: Forbidden", "unexpected error")
 	})
 
 	t.Run("Disable_NotFound", func(t *testing.T) {
@@ -125,7 +125,7 @@ func TestAutostart(t *testing.T) {
 		clitest.SetupConfig(t, client, root)
 
 		err := cmd.Execute()
-		require.ErrorContains(t, err, "status code 403: forbidden", "unexpected error")
+		require.ErrorContains(t, err, "status code 403: Forbidden", "unexpected error")
 	})
 
 	t.Run("Enable_DefaultSchedule", func(t *testing.T) {

diff --git a/cli/ttl_test.go b/cli/ttl_test.go
@@ -149,7 +149,7 @@ func TestTTL(t *testing.T) {
 		clitest.SetupConfig(t, client, root)
 
 		err := cmd.Execute()
-		require.ErrorContains(t, err, "status code 403: forbidden", "unexpected error")
+		require.ErrorContains(t, err, "status code 403: Forbidden", "unexpected error")
 	})
 
 	t.Run("Unset_NotFound", func(t *testing.T) {
@@ -166,6 +166,6 @@ func TestTTL(t *testing.T) {
 		clitest.SetupConfig(t, client, root)
 
 		err := cmd.Execute()
-		require.ErrorContains(t, err, "status code 403: forbidden", "unexpected error")
+		require.ErrorContains(t, err, "status code 403: Forbidden", "unexpected error")
 	})
 }
diff --git a/coderd/authorize.go b/coderd/authorize.go
@@ -21,9 +21,7 @@ func (api *API) Authorize(rw http.ResponseWriter, r *http.Request, action rbac.A
 	roles := httpmw.AuthorizationUserRoles(r)
 	err := api.Authorizer.ByRoleName(r.Context(), roles.ID.String(), roles.Roles, action, object.RBACObject())
 	if err != nil {
-		httpapi.Write(rw, http.StatusForbidden, httpapi.Response{
-			Message: err.Error(),
-		})
+		httpapi.Forbidden(rw)
 
 		// Log the errors for debugging
 		internalError := new(rbac.UnauthorizedError)

diff --git a/coderd/csp.go b/coderd/csp.go
@@ -23,7 +23,8 @@ func (api *API) logReportCSPViolations(rw http.ResponseWriter, r *http.Request)
 	if err != nil {
 		api.Logger.Warn(ctx, "csp violation", slog.Error(err))
 		httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{
-			Message: "failed to read body",
+			Message: "Failed to read body, invalid json",
+			Detail:  err.Error(),
 		})
 		return
 	}

diff --git a/coderd/files.go b/coderd/files.go
@@ -32,7 +32,7 @@ func (api *API) postFile(rw http.ResponseWriter, r *http.Request) {
 	case "application/x-tar":
 	default:
 		httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{
-			Message: fmt.Sprintf("unsupported content type: %s", contentType),
+			Message: fmt.Sprintf("Unsupported content type header %q", contentType),
 		})
 		return
 	}
@@ -41,7 +41,8 @@ func (api *API) postFile(rw http.ResponseWriter, r *http.Request) {
 	data, err := io.ReadAll(r.Body)
 	if err != nil {
 		httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{
-			Message: fmt.Sprintf("read file: %s", err),
+			Message: "Failed to read file from request",
+			Detail:  err.Error(),
 		})
 		return
 	}
@@ -64,7 +65,8 @@ func (api *API) postFile(rw http.ResponseWriter, r *http.Request) {
 	})
 	if err != nil {
 		httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
-			Message: fmt.Sprintf("insert file: %s", err),
+			Message: "Internal error saving file",
+			Detail:  err.Error(),
 		})
 		return
 	}
@@ -78,7 +80,7 @@ func (api *API) fileByHash(rw http.ResponseWriter, r *http.Request) {
 	hash := chi.URLParam(r, "hash")
 	if hash == "" {
 		httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{
-			Message: "hash must be provided",
+			Message: "File hash must be provided in url",
 		})
 		return
 	}
@@ -89,7 +91,8 @@ func (api *API) fileByHash(rw http.ResponseWriter, r *http.Request) {
 	}
 	if err != nil {
 		httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
-			Message: fmt.Sprintf("get file: %s", err),
+			Message: "Internal error fetching file",
+			Detail:  err.Error(),
 		})
 		return
 	}

diff --git a/coderd/gitsshkey.go b/coderd/gitsshkey.go
@@ -1,7 +1,6 @@
 package coderd
 
 import (
-	"fmt"
 	"net/http"
 
 	"github.com/coder/coder/coderd/database"
@@ -22,7 +21,8 @@ func (api *API) regenerateGitSSHKey(rw http.ResponseWriter, r *http.Request) {
 	privateKey, publicKey, err := gitsshkey.Generate(api.SSHKeygenAlgorithm)
 	if err != nil {
 		httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
-			Message: fmt.Sprintf("regenerate key pair: %s", err),
+			Message: "Internal error generating a new SSH keypair",
+			Detail:  err.Error(),
 		})
 		return
 	}
@@ -35,15 +35,17 @@ func (api *API) regenerateGitSSHKey(rw http.ResponseWriter, r *http.Request) {
 	})
 	if err != nil {
 		httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
-			Message: fmt.Sprintf("update git SSH key: %s", err),
+			Message: "Internal error updating user's git SSH key",
+			Detail:  err.Error(),
 		})
 		return
 	}
 
 	newKey, err := api.Database.GetGitSSHKey(r.Context(), user.ID)
 	if err != nil {
 		httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
-			Message: fmt.Sprintf("get git SSH key: %s", err),
+			Message: "Internal error fetching user's git SSH key",
+			Detail:  err.Error(),
 		})
 		return
 	}
@@ -67,7 +69,8 @@ func (api *API) gitSSHKey(rw http.ResponseWriter, r *http.Request) {
 	gitSSHKey, err := api.Database.GetGitSSHKey(r.Context(), user.ID)
 	if err != nil {
 		httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
-			Message: fmt.Sprintf("update git SSH key: %s", err),
+			Message: "Internal error fetching user's SSH key",
+			Detail:  err.Error(),
 		})
 		return
 	}
@@ -86,31 +89,35 @@ func (api *API) agentGitSSHKey(rw http.ResponseWriter, r *http.Request) {
 	resource, err := api.Database.GetWorkspaceResourceByID(r.Context(), agent.ResourceID)
 	if err != nil {
 		httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
-			Message: fmt.Sprintf("getting workspace resources: %s", err),
+			Message: "Internal error fetching workspace resource",
+			Detail:  err.Error(),
 		})
 		return
 	}
 
 	job, err := api.Database.GetWorkspaceBuildByJobID(r.Context(), resource.JobID)
 	if err != nil {
 		httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
-			Message: fmt.Sprintf("getting workspace build: %s", err),
+			Message: "Internal error fetching workspace build",
+			Detail:  err.Error(),
 		})
 		return
 	}
 
 	workspace, err := api.Database.GetWorkspaceByID(r.Context(), job.WorkspaceID)
 	if err != nil {
 		httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
-			Message: fmt.Sprintf("getting workspace: %s", err),
+			Message: "Internal error fetching workspace",
+			Detail:  err.Error(),
 		})
 		return
 	}
 
 	gitSSHKey, err := api.Database.GetGitSSHKey(r.Context(), workspace.OwnerID)
 	if err != nil {
 		httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
-			Message: fmt.Sprintf("getting git SSH key: %s", err),
+			Message: "Internal error fetching git SSH key",
+			Detail:  err.Error(),
 		})
 		return
 	}

diff --git a/coderd/httpapi/httpapi.go b/coderd/httpapi/httpapi.go
@@ -52,8 +52,22 @@ func init() {
 
 // Response represents a generic HTTP response.
 type Response struct {
-	Message string  `json:"message" validate:"required"`
-	Errors  []Error `json:"errors,omitempty" validate:"required"`
+	// Message is an actionable message that depicts actions the request took.
+	// These messages should be fully formed sentences with proper punctuation.
+	// Examples:
+	// - "A user has been created."
+	// - "Failed to create a user."
+	Message string `json:"message"`
+	// Detail is a debug message that provides further insight into why the
+	// action failed. This information can be technical and a regular golang
+	// err.Error() text.
+	// - "database: too many open connections"
+	// - "stat: too many open files"
+	Detail string `json:"detail"`
+	// Validations are form field-specific friendly error messages. They will be
+	// shown on a form field in the UI. These can also be used to add additional
+	// context if there is a set of errors in the primary 'Message'.
+	Validations []Error `json:"errors,omitempty"`
 }
 
 // Error represents a scoped error to a user input.
@@ -64,7 +78,7 @@ type Error struct {
 
 func Forbidden(rw http.ResponseWriter) {
 	Write(rw, http.StatusForbidden, Response{
-		Message: "forbidden",
+		Message: "Forbidden",
 	})
 }
 
@@ -93,7 +107,8 @@ func Read(rw http.ResponseWriter, r *http.Request, value interface{}) bool {
 	err := json.NewDecoder(r.Body).Decode(value)
 	if err != nil {
 		Write(rw, http.StatusBadRequest, Response{
-			Message: fmt.Sprintf("read body: %s", err.Error()),
+			Message: "Request body must be valid JSON",
+			Detail:  err.Error(),
 		})
 		return false
 	}
@@ -108,14 +123,15 @@ func Read(rw http.ResponseWriter, r *http.Request, value interface{}) bool {
 			})
 		}
 		Write(rw, http.StatusBadRequest, Response{
-			Message: "Validation failed",
-			Errors:  apiErrors,
+			Message:     "Validation failed",
+			Validations: apiErrors,
 		})
 		return false
 	}
 	if err != nil {
 		Write(rw, http.StatusInternalServerError, Response{
-			Message: fmt.Sprintf("validation: %s", err.Error()),
+			Message: "Internal error validating request body payload",
+			Detail:  err.Error(),
 		})
 		return false
 	}

diff --git a/coderd/httpapi/httpapi_test.go b/coderd/httpapi/httpapi_test.go
@@ -74,9 +74,9 @@ func TestRead(t *testing.T) {
 		var v httpapi.Response
 		err := json.NewDecoder(rw.Body).Decode(&v)
 		require.NoError(t, err)
-		require.Len(t, v.Errors, 1)
-		require.Equal(t, "value", v.Errors[0].Field)
-		require.Equal(t, "Validation failed for tag \"required\" with value: \"\"", v.Errors[0].Detail)
+		require.Len(t, v.Validations, 1)
+		require.Equal(t, "value", v.Validations[0].Field)
+		require.Equal(t, "Validation failed for tag \"required\" with value: \"\"", v.Validations[0].Detail)
 	})
 }
 

diff --git a/coderd/httpmw/apikey.go b/coderd/httpmw/apikey.go
@@ -65,15 +65,15 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs) func(http.Handler) h
 			}
 			if cookieValue == "" {
 				httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
-					Message: fmt.Sprintf("%q cookie or query parameter must be provided", SessionTokenKey),
+					Message: fmt.Sprintf("Cookie %q or query parameter must be provided", SessionTokenKey),
 				})
 				return
 			}
 			parts := strings.Split(cookieValue, "-")
 			// APIKeys are formatted: ID-SECRET
 			if len(parts) != 2 {
 				httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
-					Message: fmt.Sprintf("invalid %q cookie api key format", SessionTokenKey),
+					Message: fmt.Sprintf("Invalid %q cookie API key format", SessionTokenKey),
 				})
 				return
 			}
@@ -82,26 +82,27 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs) func(http.Handler) h
 			// Ensuring key lengths are valid.
 			if len(keyID) != 10 {
 				httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
-					Message: fmt.Sprintf("invalid %q cookie api key id", SessionTokenKey),
+					Message: fmt.Sprintf("Invalid %q cookie API key id", SessionTokenKey),
 				})
 				return
 			}
 			if len(keySecret) != 22 {
 				httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
-					Message: fmt.Sprintf("invalid %q cookie api key secret", SessionTokenKey),
+					Message: fmt.Sprintf("Invalid %q cookie API key secret", SessionTokenKey),
 				})
 				return
 			}
 			key, err := db.GetAPIKeyByID(r.Context(), keyID)
 			if err != nil {
 				if errors.Is(err, sql.ErrNoRows) {
 					httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
-						Message: "api key is invalid",
+						Message: "API key is invalid",
 					})
 					return
 				}
 				httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
-					Message: fmt.Sprintf("get api key by id: %s", err.Error()),
+					Message: "Internal error fetching API key by id",
+					Detail:  err.Error(),
 				})
 				return
 			}
@@ -110,7 +111,7 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs) func(http.Handler) h
 			// Checking to see if the secret is valid.
 			if subtle.ConstantTimeCompare(key.HashedSecret, hashed[:]) != 1 {
 				httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
-					Message: "api key secret is invalid",
+					Message: "API key secret is invalid",
 				})
 				return
 			}
@@ -127,7 +128,7 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs) func(http.Handler) h
 						oauthConfig = oauth.Github
 					default:
 						httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
-							Message: fmt.Sprintf("unexpected authentication type %q", key.LoginType),
+							Message: fmt.Sprintf("Unexpected authentication type %q", key.LoginType),
 						})
 						return
 					}
@@ -139,7 +140,8 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs) func(http.Handler) h
 					}).Token()
 					if err != nil {
 						httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
-							Message: fmt.Sprintf("couldn't refresh expired oauth token: %s", err.Error()),
+							Message: "Could not refresh expired Oauth token",
+							Detail:  err.Error(),
 						})
 						return
 					}
@@ -154,7 +156,7 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs) func(http.Handler) h
 			// Checking if the key is expired.
 			if key.ExpiresAt.Before(now) {
 				httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
-					Message: fmt.Sprintf("api key expired at %q", key.ExpiresAt.String()),
+					Message: fmt.Sprintf("API key expired at %q", key.ExpiresAt.String()),
 				})
 				return
 			}
@@ -182,7 +184,7 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs) func(http.Handler) h
 				})
 				if err != nil {
 					httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
-						Message: fmt.Sprintf("api key couldn't update: %s", err.Error()),
+						Message: fmt.Sprintf("API key couldn't update: %s", err.Error()),
 					})
 					return
 				}
@@ -194,14 +196,15 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs) func(http.Handler) h
 			roles, err := db.GetAuthorizationUserRoles(r.Context(), key.UserID)
 			if err != nil {
 				httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
-					Message: "roles not found",
+					Message: "Internal error fetching user's roles",
+					Detail:  err.Error(),
 				})
 				return
 			}
 
 			if roles.Status != database.UserStatusActive {
 				httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
-					Message: fmt.Sprintf("user is not active (status = %q), contact an admin to reactivate your account", roles.Status),
+					Message: fmt.Sprintf("User is not active (status = %q). Contact an admin to reactivate your account.", roles.Status),
 				})
 				return
 			}