From 141ce75902b455b2255e137160931c1755f8f3ba Mon Sep 17 00:00:00 2001 From: Tom Wilkie Date: Thu, 9 Jun 2016 09:01:50 +0100 Subject: [PATCH] Log errors in response to http requests. (#1569) --- app/api_report.go | 4 ++-- app/api_topologies.go | 6 +++--- app/api_topology.go | 2 +- app/controls.go | 2 +- app/pipes.go | 4 ++-- app/router.go | 8 ++++---- app/server_helpers.go | 9 ++++++++- 7 files changed, 21 insertions(+), 14 deletions(-) diff --git a/app/api_report.go b/app/api_report.go index 380c30387..05c9f0a9a 100644 --- a/app/api_report.go +++ b/app/api_report.go @@ -15,7 +15,7 @@ func makeRawReportHandler(rep Reporter) CtxHandlerFunc { return func(ctx context.Context, w http.ResponseWriter, r *http.Request) { report, err := rep.Report(ctx) if err != nil { - respondWith(w, http.StatusInternalServerError, err.Error()) + respondWith(w, http.StatusInternalServerError, err) return } respondWith(w, http.StatusOK, report) @@ -34,7 +34,7 @@ func makeProbeHandler(rep Reporter) CtxHandlerFunc { return func(ctx context.Context, w http.ResponseWriter, r *http.Request) { rpt, err := rep.Report(ctx) if err != nil { - respondWith(w, http.StatusInternalServerError, err.Error()) + respondWith(w, http.StatusInternalServerError, err) return } result := []probeDesc{} diff --git a/app/api_topologies.go b/app/api_topologies.go index 913390ab8..7ae355542 100644 --- a/app/api_topologies.go +++ b/app/api_topologies.go @@ -278,7 +278,7 @@ func (r *registry) makeTopologyList(rep Reporter) CtxHandlerFunc { return func(ctx context.Context, w http.ResponseWriter, req *http.Request) { report, err := rep.Report(ctx) if err != nil { - respondWith(w, http.StatusInternalServerError, err.Error()) + respondWith(w, http.StatusInternalServerError, err) return } respondWith(w, http.StatusOK, r.renderTopologies(report, req)) @@ -369,13 +369,13 @@ func (r *registry) captureRenderer(rep Reporter, f rendererHandler) CtxHandlerFu } rpt, err := rep.Report(ctx) if err != nil { - respondWith(w, http.StatusInternalServerError, err.Error()) + respondWith(w, http.StatusInternalServerError, err) return } req.ParseForm() renderer, decorator, err := r.rendererForTopology(topologyID, req.Form, rpt) if err != nil { - respondWith(w, http.StatusInternalServerError, err.Error()) + respondWith(w, http.StatusInternalServerError, err) return } f(ctx, renderer, decorator, rpt, w, req) diff --git a/app/api_topology.go b/app/api_topology.go index 3a6eabb89..f61e341a1 100644 --- a/app/api_topology.go +++ b/app/api_topology.go @@ -59,7 +59,7 @@ func handleWebsocket( r *http.Request, ) { if err := r.ParseForm(); err != nil { - respondWith(w, http.StatusInternalServerError, err.Error()) + respondWith(w, http.StatusInternalServerError, err) return } loop := websocketLoop diff --git a/app/controls.go b/app/controls.go index 115359422..38f9a759e 100644 --- a/app/controls.go +++ b/app/controls.go @@ -75,7 +75,7 @@ func handleProbeWS(cr ControlRouter) CtxHandlerFunc { return res }) if err != nil { - respondWith(w, http.StatusBadRequest, err.Error()) + respondWith(w, http.StatusBadRequest, err) return } defer cr.Deregister(ctx, probeID, id) diff --git a/app/pipes.go b/app/pipes.go index 72e9486d0..f51b75e9f 100644 --- a/app/pipes.go +++ b/app/pipes.go @@ -34,7 +34,7 @@ func checkPipe(pr PipeRouter, end End) CtxHandlerFunc { id := mux.Vars(r)["pipeID"] exists, err := pr.Exists(ctx, id) if err != nil { - respondWith(w, http.StatusInternalServerError, err.Error()) + respondWith(w, http.StatusInternalServerError, err) } else if exists { w.WriteHeader(http.StatusNoContent) } else { @@ -73,7 +73,7 @@ func deletePipe(pr PipeRouter) CtxHandlerFunc { pipeID := mux.Vars(r)["pipeID"] log.Infof("Deleting pipe %s", pipeID) if err := pr.Delete(ctx, pipeID); err != nil { - respondWith(w, http.StatusInternalServerError, err.Error()) + respondWith(w, http.StatusInternalServerError, err) } } } diff --git a/app/router.go b/app/router.go index 036912cbc..fce7c7311 100644 --- a/app/router.go +++ b/app/router.go @@ -134,7 +134,7 @@ func RegisterReportPostHandler(a Adder, router *mux.Router) { if strings.Contains(r.Header.Get("Content-Encoding"), "gzip") { reader, err = gzip.NewReader(reader) if err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + respondWith(w, http.StatusBadRequest, err) return } } @@ -150,7 +150,7 @@ func RegisterReportPostHandler(a Adder, router *mux.Router) { } if err := decoder(&rpt); err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + respondWith(w, http.StatusBadRequest, err) return } log.Debugf( @@ -162,7 +162,7 @@ func RegisterReportPostHandler(a Adder, router *mux.Router) { if err := a.Add(ctx, rpt); err != nil { log.Errorf("Error Adding report: %v", err) - http.Error(w, err.Error(), http.StatusBadRequest) + respondWith(w, http.StatusInternalServerError, err) return } w.WriteHeader(http.StatusOK) @@ -188,7 +188,7 @@ func apiHandler(rep Reporter) CtxHandlerFunc { return func(ctx context.Context, w http.ResponseWriter, r *http.Request) { report, err := rep.Report(ctx) if err != nil { - http.Error(w, err.Error(), http.StatusInternalServerError) + respondWith(w, http.StatusInternalServerError, err) return } newVersion.Lock() diff --git a/app/server_helpers.go b/app/server_helpers.go index f1bf06c5b..86e792504 100644 --- a/app/server_helpers.go +++ b/app/server_helpers.go @@ -9,11 +9,18 @@ import ( ) func respondWith(w http.ResponseWriter, code int, response interface{}) { + if err, ok := response.(error); ok { + log.Errorf("Error %d: %v", code, err) + response = err.Error() + } else if 500 <= code && code < 600 { + log.Errorf("Non-error %d: %v", code, response) + } + w.Header().Set("Content-Type", "application/json") w.Header().Add("Cache-Control", "no-cache") w.WriteHeader(code) encoder := codec.NewEncoder(w, &codec.JsonHandle{}) if err := encoder.Encode(response); err != nil { - log.Error(err) + log.Errorf("Error encdoing response: %v", err) } }