Clean up API error messages (hashicorp-forge#203)

* Standardize on plain text HTTP error messages for now * Correctly surface HTTP error messages
razorpay · Jun 6, 2023 · 928acc5 · 928acc5
1 parent eac44ec
commit 928acc5
Show file tree

Hide file tree

Showing 11 changed files with 22 additions and 37 deletions.
diff --git a/internal/api/approvals.go b/internal/api/approvals.go
@@ -289,19 +289,19 @@ func ApprovalHandler(
 			userEmail := r.Context().Value("userEmail").(string)
 			if docObj.GetStatus() != "In-Review" && docObj.GetStatus() != "In Review" {
 				http.Error(w,
-					`{"error": "Only documents in the "In-Review" status can be approved"}`,
+					"Only documents in the \"In-Review\" status can be approved",
 					http.StatusBadRequest)
 				return
 			}
 			if !contains(docObj.GetApprovers(), userEmail) {
 				http.Error(w,
-					`{"error": "Not authorized as a document approver"}`,
+					"Not authorized as a document approver",
 					http.StatusUnauthorized)
 				return
 			}
 			if contains(docObj.GetApprovedBy(), userEmail) {
 				http.Error(w,
-					`{"error": "Document already approved by user"}`,
+					"Document already approved by user",
 					http.StatusBadRequest)
 				return
 			}

diff --git a/internal/api/documents.go b/internal/api/documents.go
@@ -215,9 +215,7 @@ func DocumentHandler(
 			// Authorize request (only the owner can PATCH the doc).
 			userEmail := r.Context().Value("userEmail").(string)
 			if docObj.GetOwners()[0] != userEmail {
-				http.Error(w,
-					`{"error": "Not a document owner"}`,
-					http.StatusUnauthorized)
+				http.Error(w, "Not a document owner", http.StatusUnauthorized)
 				return
 			}
 

diff --git a/internal/api/drafts.go b/internal/api/drafts.go
@@ -72,8 +72,7 @@ func DraftsHandler(
 				"path", r.URL.Path,
 				"error", err,
 			)
-			errJSON := fmt.Sprintf(`{"error": "%s"}`, userErrMsg)
-			http.Error(w, errJSON, httpCode)
+			http.Error(w, userErrMsg, httpCode)
 		}
 
 		// Authorize request.
@@ -515,7 +514,7 @@ func DraftsDocumentHandler(
 		}
 		if !isOwner && !isContributor {
 			http.Error(w,
-				`{"error": "Only owners or contributors can access a draft document"}`,
+				"Only owners or contributors can access a draft document",
 				http.StatusUnauthorized)
 			return
 		}
@@ -613,7 +612,7 @@ func DraftsDocumentHandler(
 			// Authorize request.
 			if !isOwner {
 				http.Error(w,
-					`{"error": "Only owners can delete a draft document"}`,
+					"Only owners can delete a draft document",
 					http.StatusUnauthorized)
 				return
 			}

diff --git a/internal/api/helpers.go b/internal/api/helpers.go
@@ -106,6 +106,5 @@ func respondError(
 			"path", r.URL.Path,
 		}, extraArgs...)...,
 	)
-	errJSON := fmt.Sprintf(`{"error": "%s"}`, userErrMsg)
-	http.Error(w, errJSON, httpCode)
+	http.Error(w, userErrMsg, httpCode)
 }
diff --git a/internal/api/me.go b/internal/api/me.go
@@ -37,8 +37,7 @@ func MeHandler(
 				"path", r.URL.Path,
 				"error", err,
 			)
-			errJSON := fmt.Sprintf(`{"error": "%s"}`, userErrMsg)
-			http.Error(w, errJSON, httpCode)
+			http.Error(w, userErrMsg, httpCode)
 		}
 
 		// Authorize request.
@@ -67,8 +66,7 @@ func MeHandler(
 					"path", r.URL.Path,
 					"error", err,
 				)
-				errJSON := fmt.Sprintf(`{"error": "%s"}`, userErrMsg)
-				http.Error(w, errJSON, httpCode)
+				http.Error(w, userErrMsg, httpCode)
 			}
 
 			ppl, err := s.SearchPeople(userEmail, "emailAddresses,names,photos")

diff --git a/internal/api/me_subscriptions.go b/internal/api/me_subscriptions.go
@@ -2,7 +2,6 @@ package api
 
 import (
 	"encoding/json"
-	"fmt"
 	"net/http"
 
 	"github.com/hashicorp-forge/hermes/internal/config"
@@ -30,8 +29,7 @@ func MeSubscriptionsHandler(
 				"path", r.URL.Path,
 				"error", err,
 			)
-			errJSON := fmt.Sprintf(`{"error": "%s"}`, userErrMsg)
-			http.Error(w, errJSON, httpCode)
+			http.Error(w, userErrMsg, httpCode)
 		}
 
 		// Authorize request.

diff --git a/internal/api/reviews.go b/internal/api/reviews.go
@@ -188,8 +188,7 @@ func ReviewHandler(
 					"method", r.Method,
 					"doc_id", docID,
 				)
-				http.Error(w, `{"error": "Error creating review"}`,
-					http.StatusInternalServerError)
+				http.Error(w, "Error creating review", http.StatusInternalServerError)
 				return
 			}
 
@@ -202,8 +201,7 @@ func ReviewHandler(
 					"method", r.Method,
 					"doc_id", docID,
 				)
-				http.Error(w, `{"error": "Error creating review"}`,
-					http.StatusInternalServerError)
+				http.Error(w, "Error creating review", http.StatusInternalServerError)
 				return
 			}
 			docObj.SetModifiedTime(modifiedTime.Unix())
@@ -515,7 +513,7 @@ func ReviewHandler(
 						"method", r.Method,
 						"path", r.URL.Path,
 					)
-					http.Error(w, `{"error": "Error sending subscriber email"}`,
+					http.Error(w, "Error sending subscriber email",
 						http.StatusInternalServerError)
 					return
 				}
@@ -545,7 +543,7 @@ func ReviewHandler(
 								"method", r.Method,
 								"path", r.URL.Path,
 							)
-							http.Error(w, `{"error": "Error sending subscriber email"}`,
+							http.Error(w, "Error sending subscriber email",
 								http.StatusInternalServerError)
 							return
 						}

diff --git a/internal/auth/auth.go b/internal/auth/auth.go
@@ -21,8 +21,7 @@ func AuthenticateRequest(
 		if err != nil {
 			log.Error("error creating Okta authenticator")
 			return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-				http.Error(w,
-					`{"error": "Internal server error"}`, http.StatusInternalServerError)
+				http.Error(w, "Internal server error", http.StatusInternalServerError)
 				return
 			})
 		}
@@ -55,8 +54,7 @@ func validateUserEmail(
 		log.Error("userEmail is not set in the request context",
 			"method", r.Method,
 			"path", r.URL.Path)
-		http.Error(w,
-			`{"error": "Internal server error"}`, http.StatusInternalServerError)
+		http.Error(w, "Internal server error", http.StatusInternalServerError)
 		return
 	}
 }
diff --git a/internal/auth/google/google.go b/internal/auth/google/google.go
@@ -29,17 +29,15 @@ func AuthenticateRequest(
 				"method", r.Method,
 				"path", r.URL.Path,
 			)
-			http.Error(w,
-				`{"error": "Unauthorized"}`, http.StatusUnauthorized)
+			http.Error(w, "Unauthorized", http.StatusUnauthorized)
 			return
 		}
 		if ti.Email == "" {
 			log.Error("no user email found in Google access token",
 				"method", r.Method,
 				"path", r.URL.Path,
 			)
-			http.Error(w,
-				`{"error": "Unauthorized"}`, http.StatusUnauthorized)
+			http.Error(w, "Unauthorized", http.StatusUnauthorized)
 			return
 		}
 

diff --git a/internal/auth/oktaalb/oktaalb.go b/internal/auth/oktaalb/oktaalb.go
@@ -55,7 +55,7 @@ func (oa *OktaAuthorizer) EnforceOktaAuth(next http.Handler) http.Handler {
 				"method", r.Method,
 				"path", r.URL.Path,
 			)
-			http.Error(w, `{"error": "Unauthorized"}`, http.StatusUnauthorized)
+			http.Error(w, "Unauthorized", http.StatusUnauthorized)
 			return
 		} else {
 			// Set user email from the OIDC claims.

diff --git a/web/app/services/fetch.ts b/web/app/services/fetch.ts
@@ -50,9 +50,8 @@ export default class FetchService extends Service {
           // handle poll-call failures via the session service
           return;
         }
-        // log the response so it's easier to debug environment-specific errors
-        console.error("fetch error:", resp);
-        throw new Error(`Bad response: ${resp.statusText}`);
+        const errText = await resp.text();
+        throw new Error(`Bad response: ${errText}`);
       }
 
       return resp;