From b389d9170c0964046bb332f02480f35b60c43547 Mon Sep 17 00:00:00 2001 From: Dmitry Vyukov Date: Tue, 20 Mar 2018 11:49:18 +0100 Subject: dashboard/app: don't log /api requests as errors Not so interesting if it was just /api request. --- dashboard/app/api.go | 20 ++++++++++++++------ dashboard/app/app_test.go | 6 +++--- 2 files changed, 17 insertions(+), 9 deletions(-) (limited to 'dashboard') 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", -- cgit mrf-deployment