Improve logging and answer 500 on marshalling errors

go-graphite · Sep 14, 2020 · 8a7b7a4 · 8a7b7a4
1 parent 7342a75
commit 8a7b7a4
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 8 deletions.
diff --git a/render/handler.go b/render/handler.go
@@ -37,6 +37,17 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	var prefix string
 	var err error
 
+	defer func() {
+		if rec := recover(); rec != nil {
+			logger.Error("panic during eval:",
+				zap.String("requestID", scope.String(r.Context(), "requestID")),
+				zap.Any("reason", rec),
+				zap.Stack("stack"),
+			)
+			answer := fmt.Sprintf("%v\nStack trace: %v", rec, zap.Stack("").String)
+			http.Error(w, answer, http.StatusInternalServerError)
+		}
+	}()
 	fetchRequests := make(MultiFetchRequest)
 
 	r.ParseMultipartForm(1024 * 1024)

diff --git a/render/reply_pickle.go b/render/reply_pickle.go
@@ -2,6 +2,7 @@ package render
 
 import (
 	"bufio"
+	"fmt"
 	"net/http"
 	"time"
 
@@ -92,16 +93,18 @@ func (h *Handler) ReplyPickle(w http.ResponseWriter, r *http.Request, data *Data
 		pickleTime += time.Since(pickleStart)
 	}
 
-	writeMetric := func(points []point.Point) {
+	writeMetric := func(points []point.Point) error {
 		metricName := data.Points.MetricName(points[0].MetricID)
 		step, err := data.GetStep(points[0].MetricID)
 		if err != nil {
 			logger.Error("fail to get step", zap.Error(err))
-			return
+			http.Error(w, fmt.Sprintf("failed to get step for metric: %v", data.Points.MetricName(points[0].MetricID)), http.StatusInternalServerError)
+			return err
 		}
 		for _, a := range data.Aliases.Get(metricName) {
 			writeAlias(a.DisplayName, a.Target, points, step)
 		}
+		return nil
 	}
 	// group by Metric
 	var i, n int
@@ -111,13 +114,17 @@ func (h *Handler) ReplyPickle(w http.ResponseWriter, r *http.Request, data *Data
 
 	for i = 1; i < l; i++ {
 		if points[i].MetricID != points[n].MetricID {
-			writeMetric(points[n:i])
+			if err := writeMetric(points[n:i]); err != nil {
+				return
+			}
 			n = i
 			continue
 		}
 	}
 
-	writeMetric(points[n:i])
+	if err := writeMetric(points[n:i]); err != nil {
+		return
+	}
 
 	p.Stop()
 }
diff --git a/render/reply_protobuf.go b/render/reply_protobuf.go
@@ -3,6 +3,7 @@ package render
 import (
 	"bufio"
 	"bytes"
+	"fmt"
 	"net/http"
 
 	"go.uber.org/zap"
@@ -38,17 +39,19 @@ func (h *Handler) ReplyProtobuf(w http.ResponseWriter, r *http.Request, perfix s
 		}
 		totalWritten++
 
-		writeMetric := func(points []point.Point) {
+		writeMetric := func(points []point.Point) error {
 			metricName := data.Points.MetricName(points[0].MetricID)
 			step, err := data.GetStep(points[0].MetricID)
 			if err != nil {
 				logger.Error("fail to get step", zap.Error(err))
-				return
+				http.Error(w, fmt.Sprintf("failed to get step for metric: %v", data.Points.MetricName(points[0].MetricID)), http.StatusInternalServerError)
+				return err
 			}
 
 			for _, a := range data.Aliases.Get(metricName) {
 				writeAlias(mb, mb2, writer, a.Target, a.DisplayName, from, until, step, points)
 			}
+			return nil
 		}
 
 		// group by Metric
@@ -59,12 +62,16 @@ func (h *Handler) ReplyProtobuf(w http.ResponseWriter, r *http.Request, perfix s
 
 		for i = 1; i < l; i++ {
 			if points[i].MetricID != points[n].MetricID {
-				writeMetric(points[n:i])
+				if err := writeMetric(points[n:i]); err != nil {
+					return
+				}
 				n = i
 				continue
 			}
 		}
-		writeMetric(points[n:i])
+		if err := writeMetric(points[n:i]); err != nil {
+			return
+		}
 	}
 
 	if totalWritten == 0 {