uhttp stop return panic detail to client (#241)

don’t return http panic detail to client

modules/uhttp/filters.go

@@ -31,8 +31,11 @@ import (
 
 	"github.com/opentracing/opentracing-go"
 	"github.com/opentracing/opentracing-go/ext"
+	"github.com/pkg/errors"
 )
 
+const _panicResponse = "Server Error"
+
 // Filter applies filters on requests or responses such as
 // adding tracing to the context
 type Filter interface {
@@ -100,11 +103,9 @@ func (f panicFilter) Apply(w http.ResponseWriter, r *http.Request, next http.Han
 	ctx := r.Context()
 	defer func() {
 		if err := recover(); err != nil {
-			ulog.Logger(ctx).Error("Panic recovered serving request", "error", err, "url", r.URL)
+			ulog.Logger(ctx).Error("Panic recovered serving request", "error", errors.Errorf("panic in handler: %+v", err), "url", r.URL)
 			stats.HTTPPanicCounter.Inc(1)
-			w.Header().Add(ContentType, ContentTypeText)
-			w.WriteHeader(http.StatusInternalServerError)
-			fmt.Fprintf(w, "Server error: %+v", err)
+			http.Error(w, _panicResponse, http.StatusInternalServerError)
 		}
 	}()
 	next.ServeHTTP(w, r)

modules/uhttp/filters_test.go

@@ -37,14 +37,15 @@ import (
 
 	"github.com/opentracing/opentracing-go"
 	"github.com/stretchr/testify/assert"
+	"github.com/uber-go/tally"
 	"github.com/uber-go/zap"
 	"github.com/uber/jaeger-client-go/config"
 )
 
 func TestFilterChain(t *testing.T) {
 	host := service.NopHost()
-	chain := newFilterChainBuilder(host).AddFilters([]Filter{}...).Build(getNopHandler(host))
-	response := testServeHTTP(chain, host)
+	chain := newFilterChainBuilder(host).AddFilters([]Filter{}...).Build(getNopHandler())
+	response := testServeHTTP(chain)
 	assert.True(t, strings.Contains(response.Body.String(), "filters ok"))
 }
 
@@ -66,8 +67,8 @@ func TestTracingFilterWithLogs(t *testing.T) {
 
 		host := service.NopHostConfigured(auth.NopClient, loggerWithZap, tracer)
 		ulog.SetLogger(host.Logger())
-		chain := newFilterChainBuilder(host).AddFilters([]Filter{contextFilter{host}, tracingServerFilter{}}...).Build(getNopHandler(host))
-		response := testServeHTTP(chain, host)
+		chain := newFilterChainBuilder(host).AddFilters([]Filter{contextFilter{host}, tracingServerFilter{}}...).Build(getNopHandler())
+		response := testServeHTTP(chain)
 		assert.Contains(t, response.Body.String(), "filters ok")
 		assert.True(t, len(buf.Lines()) > 0)
 		var tracecount = 0
@@ -91,9 +92,9 @@ func TestFilterChainFilters(t *testing.T) {
 		tracingServerFilter{},
 		authorizationFilter{
 			authClient: host.AuthClient(),
-		}).Build(getNopHandler(host))
+		}).Build(getNopHandler())
 
-	response := testServeHTTP(chain, host)
+	response := testServeHTTP(chain)
 	assert.Contains(t, response.Body.String(), "filters ok")
 }
 
@@ -104,22 +105,45 @@ func TestFilterChainFilters_AuthFailure(t *testing.T) {
 		tracingServerFilter{},
 		authorizationFilter{
 			authClient: host.AuthClient(),
-		}).Build(getNopHandler(host))
-	response := testServeHTTP(chain, host)
+		}).Build(getNopHandler())
+	response := testServeHTTP(chain)
 	assert.Equal(t, response.Body.String(), "Unauthorized access: Error authorizing the service\n")
 	assert.Equal(t, 401, response.Code)
 }
 
-func testServeHTTP(chain filterChain, host service.Host) *httptest.ResponseRecorder {
+func TestPanicFilter(t *testing.T) {
+	host := service.NopHost()
+	testScope := host.Metrics()
+
+	chain := newFilterChainBuilder(host).AddFilters(
+		panicFilter{},
+	).Build(getPanicHandler())
+	response := testServeHTTP(chain)
+
+	snapshot := testScope.(tally.TestScope).Snapshot()
+	counters := snapshot.Counters()
+
+	assert.True(t, counters["panic"].Value() > 0)
+	assert.Equal(t, response.Body.String(), _panicResponse+"\n")
+	assert.Equal(t, http.StatusInternalServerError, response.Code)
+}
+
+func testServeHTTP(chain filterChain) *httptest.ResponseRecorder {
 	request := httptest.NewRequest("", "http://filters", nil)
 	response := httptest.NewRecorder()
 	chain.ServeHTTP(response, request)
 	return response
 }
 
-func getNopHandler(host service.Host) http.HandlerFunc {
+func getNopHandler() http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		ulog.Logger(r.Context()).Info("Inside Noop Handler")
 		io.WriteString(w, "filters ok")
 	}
 }
+
+func getPanicHandler() http.HandlerFunc {
+	return func(w http.ResponseWriter, r *http.Request) {
+		panic("panic")
+	}
+}