dashboard/app: don't log /api requests as errors

Not so interesting if it was just /api request.

2 files changed, 17 insertions, 9 deletions

diff --git a/dashboard/app/api.go b/dashboard/app/api.go
index e1c44fd6d..210bee0d0 100644
--- a/dashboard/app/api.go
+++ b/dashboard/app/api.go
@@ -68,7 +68,10 @@ func handleJSON(fn JSONHandler) http.Handler {
 		c := appengine.NewContext(r)
 		reply, err := fn(c, r)
 		if err != nil {
-			log.Errorf(c, "%v", err)
+			// ErrAccess is logged earlier.
+			if err != ErrAccess {
+				log.Errorf(c, "%v", err)
+			}
 			http.Error(w, err.Error(), http.StatusInternalServerError)
 			return
 		}
@@ -90,8 +93,13 @@ func handleAPI(c context.Context, r *http.Request) (reply interface{}, err error
 	log.Infof(c, "api %q from %q", method, client)
 	ns, err := checkClient(c, client, r.PostFormValue("key"))
 	if err != nil {
-		log.Warningf(c, "%v", err)
-		return nil, fmt.Errorf("unauthorized request")
+		if client != "" {
+			log.Warningf(c, "%v", err)
+		} else {
+			// Don't log as error if somebody just invokes /api.
+			log.Infof(c, "%v", err)
+		}
+		return nil, err
 	}
 	var payload []byte
 	if str := r.PostFormValue("payload"); str != "" {
@@ -125,7 +133,7 @@ func checkClient(c context.Context, name0, key0 string) (string, error) {
 	for name, key := range config.Clients {
 		if name == name0 {
 			if key != key0 {
-				return "", fmt.Errorf("wrong client %q key", name0)
+				return "", ErrAccess
 			}
 			return "", nil
 		}
@@ -134,13 +142,13 @@ func checkClient(c context.Context, name0, key0 string) (string, error) {
 		for name, key := range cfg.Clients {
 			if name == name0 {
 				if key != key0 {
-					return "", fmt.Errorf("wrong client %q key", name0)
+					return "", ErrAccess
 				}
 				return ns, nil
 			}
 		}
 	}
-	return "", fmt.Errorf("unauthorized api request from %q", name0)
+	return "", ErrAccess
 }
 
 func apiLogError(c context.Context, r *http.Request, payload []byte) (interface{}, error) {
diff --git a/dashboard/app/app_test.go b/dashboard/app/app_test.go
index 0fe5c3f7b..504bcc3b1 100644
--- a/dashboard/app/app_test.go
+++ b/dashboard/app/app_test.go
@@ -227,9 +227,9 @@ func TestApp(t *testing.T) {
 	c.expectOK(c.API(client1, key1, "upload_build", build, nil))
 
 	// Some bad combinations of client/key.
-	c.expectFail("unauthorized request", c.API(client1, "", "upload_build", build, nil))
-	c.expectFail("unauthorized request", c.API("unknown", key1, "upload_build", build, nil))
-	c.expectFail("unauthorized request", c.API(client1, key2, "upload_build", build, nil))
+	c.expectFail("unauthorized", c.API(client1, "", "upload_build", build, nil))
+	c.expectFail("unauthorized", c.API("unknown", key1, "upload_build", build, nil))
+	c.expectFail("unauthorized", c.API(client1, key2, "upload_build", build, nil))
 
 	crash1 := &dashapi.Crash{
 		BuildID:     "build1",