From ccb2ee607bdeb1707ffb4d5fd971090b3abd7af4 Mon Sep 17 00:00:00 2001
From: Tom Wilkie <tom.wilkie@gmail.com>
Date: Fri, 6 Jul 2018 18:44:45 +0100
Subject: [PATCH] Log errors encountered when marshalling and writing
 responses.

Signed-off-by: Tom Wilkie <tom.wilkie@gmail.com>
---
 web/api/v1/api.go      | 25 +++++++++++++++++++------
 web/api/v1/api_test.go | 12 ++++++++----
 web/web.go             |  1 +
 3 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/web/api/v1/api.go b/web/api/v1/api.go
index a7d728797..17a11d7d3 100644
--- a/web/api/v1/api.go
+++ b/web/api/v1/api.go
@@ -28,6 +28,8 @@ import (
 	"time"
 	"unsafe"
 
+	"github.com/go-kit/kit/log"
+	"github.com/go-kit/kit/log/level"
 	jsoniter "github.com/json-iterator/go"
 	"github.com/prometheus/common/model"
 	"github.com/prometheus/common/route"
@@ -125,6 +127,7 @@ type API struct {
 
 	db          func() *tsdb.DB
 	enableAdmin bool
+	logger      log.Logger
 }
 
 // NewAPI returns an initialized API type.
@@ -138,6 +141,7 @@ func NewAPI(
 	readyFunc func(http.HandlerFunc) http.HandlerFunc,
 	db func() *tsdb.DB,
 	enableAdmin bool,
+	logger log.Logger,
 ) *API {
 	return &API{
 		QueryEngine:           qe,
@@ -160,9 +164,9 @@ func (api *API) Register(r *route.Router) {
 			setCORS(w)
 			data, err, finalizer := f(r)
 			if err != nil {
-				respondError(w, err, data)
+				api.respondError(w, err, data)
 			} else if data != nil {
-				respond(w, data)
+				api.respond(w, data)
 			} else {
 				w.WriteHeader(http.StatusNoContent)
 			}
@@ -819,7 +823,7 @@ func mergeLabels(primary, secondary []*prompb.Label) []*prompb.Label {
 	return result
 }
 
-func respond(w http.ResponseWriter, data interface{}) {
+func (api *API) respond(w http.ResponseWriter, data interface{}) {
 	w.Header().Set("Content-Type", "application/json")
 	w.WriteHeader(http.StatusOK)
 
@@ -829,12 +833,15 @@ func respond(w http.ResponseWriter, data interface{}) {
 		Data:   data,
 	})
 	if err != nil {
+		level.Error(api.logger).Log("msg", "error marshalling json response", "err", err)
 		return
 	}
-	w.Write(b)
+	if n, err := w.Write(b); err != nil {
+		level.Error(api.logger).Log("msg", "error writing response", "n", n, "err", err)
+	}
 }
 
-func respondError(w http.ResponseWriter, apiErr *apiError, data interface{}) {
+func (api *API) respondError(w http.ResponseWriter, apiErr *apiError, data interface{}) {
 	w.Header().Set("Content-Type", "application/json")
 
 	var code int
@@ -864,7 +871,13 @@ func respondError(w http.ResponseWriter, apiErr *apiError, data interface{}) {
 	if err != nil {
 		return
 	}
-	w.Write(b)
+	if err != nil {
+		level.Error(api.logger).Log("msg", "error marshalling json response", "err", err)
+		return
+	}
+	if n, err := w.Write(b); err != nil {
+		level.Error(api.logger).Log("msg", "error writing response", "n", n, "err", err)
+	}
 }
 
 func parseTime(s string) (time.Time, error) {
diff --git a/web/api/v1/api_test.go b/web/api/v1/api_test.go
index 1cb7fdd02..2e3d0981d 100644
--- a/web/api/v1/api_test.go
+++ b/web/api/v1/api_test.go
@@ -750,7 +750,8 @@ func TestReadEndpoint(t *testing.T) {
 
 func TestRespondSuccess(t *testing.T) {
 	s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		respond(w, "test")
+		api := API{}
+		api.respond(w, "test")
 	}))
 	defer s.Close()
 
@@ -787,7 +788,8 @@ func TestRespondSuccess(t *testing.T) {
 
 func TestRespondError(t *testing.T) {
 	s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		respondError(w, &apiError{errorTimeout, errors.New("message")}, "test")
+		api := API{}
+		api.respondError(w, &apiError{errorTimeout, errors.New("message")}, "test")
 	}))
 	defer s.Close()
 
@@ -1039,7 +1041,8 @@ func TestRespond(t *testing.T) {
 
 	for _, c := range cases {
 		s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-			respond(w, c.response)
+			api := API{}
+			api.respond(w, c.response)
 		}))
 		defer s.Close()
 
@@ -1078,7 +1081,8 @@ func BenchmarkRespond(b *testing.B) {
 		},
 	}
 	b.ResetTimer()
+	api := API{}
 	for n := 0; n < b.N; n++ {
-		respond(&testResponseWriter, response)
+		api.respond(&testResponseWriter, response)
 	}
 }
diff --git a/web/web.go b/web/web.go
index 4a4c30078..af75a0827 100644
--- a/web/web.go
+++ b/web/web.go
@@ -227,6 +227,7 @@ func New(logger log.Logger, o *Options) *Handler {
 		h.testReady,
 		h.options.TSDB,
 		h.options.EnableAdminAPI,
+		logger,
 	)
 
 	if o.RoutePrefix != "/" {