From b5362d58b9f04972fc664a4ce7d7b4b6a8fb0d33 Mon Sep 17 00:00:00 2001
From: Taras Madan <tarasmadan@google.com>
Date: Wed, 24 Jul 2024 11:17:03 +0200
Subject: dashboard/app/api.go: handleAPI can return less 5xx errors

---
 dashboard/app/api.go     | 19 +++++++------------
 dashboard/app/handler.go | 25 +++++++++++++++----------
 2 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/dashboard/app/api.go b/dashboard/app/api.go
index cd230d341..3b510779f 100644
--- a/dashboard/app/api.go
+++ b/dashboard/app/api.go
@@ -104,11 +104,8 @@ func handleJSON(fn JSONHandler) http.Handler {
 		c := appengine.NewContext(r)
 		reply, err := fn(c, r)
 		if err != nil {
-			// ErrAccess is logged earlier.
-			if err != ErrAccess {
-				log.Errorf(c, "%v", err)
-			}
-			http.Error(w, err.Error(), http.StatusInternalServerError)
+			status := logErrorPrepareStatus(c, err)
+			http.Error(w, err.Error(), status)
 			return
 		}
 		w.Header().Set("Content-Type", "application/json")
@@ -131,6 +128,10 @@ func handleAPI(c context.Context, r *http.Request) (reply interface{}, err error
 	client := r.PostFormValue("client")
 	method := r.PostFormValue("method")
 	log.Infof(c, "api %q from %q", method, client)
+	if client == "" {
+		// Don't log as error if somebody just invokes /api.
+		return nil, fmt.Errorf("client is empty: %w", ErrClientBadRequest)
+	}
 	auth := auth.MakeEndpoint(auth.GoogleTokenInfoEndpoint)
 	subj, err := auth.DetermineAuthSubj(timeNow(c), r.Header["Authorization"])
 	if err != nil {
@@ -139,13 +140,7 @@ func handleAPI(c context.Context, r *http.Request) (reply interface{}, err error
 	password := r.PostFormValue("key")
 	ns, err := checkClient(getConfig(c), client, password, subj)
 	if err != nil {
-		if client != "" {
-			log.Errorf(c, "checkClient('%s') error: %v", client, err)
-		} else {
-			// Don't log as error if somebody just invokes /api.
-			log.Infof(c, "checkClient('%s') error: %v", client, err)
-		}
-		return nil, err
+		return nil, fmt.Errorf("checkClient('%s') error: %w", client, err)
 	}
 	var payload []byte
 	if str := r.PostFormValue("payload"); str != "" {
diff --git a/dashboard/app/handler.go b/dashboard/app/handler.go
index 145f79ef9..91c051e2b 100644
--- a/dashboard/app/handler.go
+++ b/dashboard/app/handler.go
@@ -60,16 +60,7 @@ func handleContext(fn contextHandler) http.Handler {
 				return
 			}
 
-			status := http.StatusInternalServerError
-			logf := log.Errorf
-			var clientError *ErrClient
-			if errors.As(err, &clientError) {
-				// We don't log these as errors because they can be provoked
-				// by invalid user requests, so we don't wan't to pollute error log.
-				logf = log.Warningf
-				status = clientError.HTTPStatus()
-			}
-			logf(c, "%v", err)
+			status := logErrorPrepareStatus(c, err)
 			w.WriteHeader(status)
 			if err1 := templates.ExecuteTemplate(w, "error.html", data); err1 != nil {
 				combinedError := fmt.Sprintf("got err \"%v\" processing ExecuteTemplate() for err \"%v\"", err1, err)
@@ -79,6 +70,20 @@ func handleContext(fn contextHandler) http.Handler {
 	})
 }
 
+func logErrorPrepareStatus(c context.Context, err error) int {
+	status := http.StatusInternalServerError
+	logf := log.Errorf
+	var clientError *ErrClient
+	if errors.As(err, &clientError) {
+		// We don't log these as errors because they can be provoked
+		// by invalid user requests, so we don't wan't to pollute error log.
+		logf = log.Warningf
+		status = clientError.HTTPStatus()
+	}
+	logf(c, "%v", err)
+	return status
+}
+
 func isRobot(r *http.Request) bool {
 	userAgent := strings.ToLower(strings.Join(r.Header["User-Agent"], " "))
 	if strings.HasPrefix(userAgent, "curl") ||
-- 
cgit mrf-deployment