Repository: gobuffalo/buffalo
Pull request #2108 (2021-05-14 12:20:01)

Feature/improved error reporting in production
This fixes https://github.com/gobuffalo/buffalo/issues/2107

paganotoni: Hey @StefanDorresteijn, thanks for putting this PR together. Could you please add some test cases for the changes in here?
StefanDorresteijn: Test code was added and I've adjusted my code to make sure it passes @paganotoni
StefanDorresteijn: Anything you guys need from me to get this merged?
paganotoni: Ups, thanks for following up @StefanDorresteijn. Nothing else needed. Thank you!

Patch:
diff --git a/Dockerfile.build b/Dockerfile.build
index e0efe151..79cd4d00 100644
--- a/Dockerfile.build
+++ b/Dockerfile.build
@@ -32,7 +32,7 @@ RUN npm install -g --no-progress yarn \
 # Install golangci
 RUN curl -sfL https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | sh -s -- -b $(go env GOPATH)/bin v1.24.0
 # Installing buffalo binary
-RUN curl -sf https://gobinaries.com/gobuffalo/buffalo/buffalo@v0.16.22 | sh 
+RUN curl -sf https://gobinaries.com/gobuffalo/buffalo/buffalo@v0.16.23 | sh 
 RUN go get github.com/gobuffalo/buffalo-pop/v2
 RUN buffalo version
 
diff --git a/Dockerfile.slim.build b/Dockerfile.slim.build
index c5e792ff..da3ffccb 100644
--- a/Dockerfile.slim.build
+++ b/Dockerfile.slim.build
@@ -17,7 +17,7 @@ RUN npm i -g --no-progress yarn \
     && yarn config set yarn-offline-mirror-pruning true
 
 # Pulling docker binary from releases
-RUN curl -sf https://gobinaries.com/gobuffalo/buffalo/buffalo@v0.16.22| sh 
+RUN curl -sf https://gobinaries.com/gobuffalo/buffalo/buffalo@v0.16.23| sh 
 RUN go get github.com/gobuffalo/buffalo-pop/v2
 RUN buffalo version
 
diff --git a/errors.go b/errors.go
index e70a2b25..7d0db087 100644
--- a/errors.go
+++ b/errors.go
@@ -151,7 +151,7 @@ func productionErrorResponseFor(status int) []byte {
 type ErrorResponse struct {
 	XMLName xml.Name `json:"-" xml:"response"`
 	Error   string   `json:"error" xml:"error"`
-	Trace   string   `json:"trace" xml:"trace"`
+	Trace   string   `json:"trace,omitempty" xml:"trace,omitempty"`
 	Code    int      `json:"code" xml:"code,attr"`
 }
 
@@ -161,15 +161,25 @@ func defaultErrorHandler(status int, origErr error, c Context) error {
 	env := c.Value("env")
 	requestCT := defaults.String(httpx.ContentType(c.Request()), defaultErrorCT)
 
+	var defaultErrorResponse *ErrorResponse
+
 	c.LogField("status", status)
 	c.Logger().Error(origErr)
 	c.Response().WriteHeader(status)
 
 	if env != nil && env.(string) == "production" {
-		c.Response().Header().Set("content-type", defaultErrorCT)
-		responseBody := productionErrorResponseFor(status)
-		c.Response().Write(responseBody)
-		return nil
+		switch strings.ToLower(requestCT) {
+		case "application/json", "text/json", "json", "application/xml", "text/xml", "xml":
+			defaultErrorResponse = &ErrorResponse{
+				Code:  status,
+				Error: http.StatusText(status),
+			}
+		default:
+			c.Response().Header().Set("content-type", defaultErrorCT)
+			responseBody := productionErrorResponseFor(status)
+			c.Response().Write(responseBody)
+			return nil
+		}
 	}
 
 	trace := origErr.Error()
@@ -177,21 +187,22 @@ func defaultErrorHandler(status int, origErr error, c Context) error {
 	switch strings.ToLower(requestCT) {
 	case "application/json", "text/json", "json":
 		c.Response().Header().Set("content-type", "application/json")
-		err := json.NewEncoder(c.Response()).Encode(&ErrorResponse{
+
+		err := json.NewEncoder(c.Response()).Encode(errorResponseDefault(defaultErrorResponse, &ErrorResponse{
 			Error: errx.Unwrap(origErr).Error(),
 			Trace: trace,
 			Code:  status,
-		})
+		}))
 		if err != nil {
 			return err
 		}
 	case "application/xml", "text/xml", "xml":
 		c.Response().Header().Set("content-type", "text/xml")
-		err := xml.NewEncoder(c.Response()).Encode(&ErrorResponse{
+		err := xml.NewEncoder(c.Response()).Encode(errorResponseDefault(defaultErrorResponse, &ErrorResponse{
 			Error: errx.Unwrap(origErr).Error(),
 			Trace: trace,
 			Code:  status,
-		})
+		}))
 		if err != nil {
 			return err
 		}
@@ -235,6 +246,13 @@ func defaultErrorHandler(status int, origErr error, c Context) error {
 	return nil
 }
 
+func errorResponseDefault(defaultResponse, alternativeResponse *ErrorResponse) *ErrorResponse {
+	if defaultResponse != nil {
+		return defaultResponse
+	}
+	return alternativeResponse
+}
+
 type inspectHeaders http.Header
 
 func (i inspectHeaders) String() string {
diff --git a/runtime/version.go b/runtime/version.go
index 84e7db05..bdc26974 100644
--- a/runtime/version.go
+++ b/runtime/version.go
@@ -1,4 +1,4 @@
 package runtime
 
 // Version is the current version of the buffalo binary
-var Version = "v0.16.22"
+var Version = "v0.16.23"


Test patch:
diff --git a/errors_test.go b/errors_test.go
index 717b514a..7042ade2 100644
--- a/errors_test.go
+++ b/errors_test.go
@@ -65,7 +65,7 @@ func Test_defaultErrorHandler_Logger(t *testing.T) {
 	r.Equal(http.StatusUnauthorized, testHook.errors[0].Data["status"])
 }
 
-func Test_defaultErrorHandler_JSON(t *testing.T) {
+func Test_defaultErrorHandler_JSON_development(t *testing.T) {
 	r := require.New(t)
 	app := New(Options{})
 	app.GET("/", func(c Context) error {
@@ -83,7 +83,7 @@ func Test_defaultErrorHandler_JSON(t *testing.T) {
 	r.Contains(b, `"trace":"`)
 }
 
-func Test_defaultErrorHandler_XML(t *testing.T) {
+func Test_defaultErrorHandler_XML_development(t *testing.T) {
 	r := require.New(t)
 	app := New(Options{})
 	app.GET("/", func(c Context) error {
@@ -103,6 +103,46 @@ func Test_defaultErrorHandler_XML(t *testing.T) {
 	r.Contains(b, `</response>`)
 }
 
+func Test_defaultErrorHandler_JSON_production(t *testing.T) {
+	r := require.New(t)
+	app := New(Options{})
+	app.Env = "production"
+	app.GET("/", func(c Context) error {
+		return c.Error(http.StatusUnauthorized, fmt.Errorf("boom"))
+	})
+
+	w := httptest.New(app)
+	res := w.JSON("/").Get()
+	r.Equal(http.StatusUnauthorized, res.Code)
+	ct := res.Header().Get("content-type")
+	r.Equal("application/json", ct)
+	b := res.Body.String()
+	r.Contains(b, `"code":401`)
+	r.Contains(b, fmt.Sprintf(`"error":"%s"`, http.StatusText(http.StatusUnauthorized)))
+	r.NotContains(b, `"trace":"`)
+}
+
+func Test_defaultErrorHandler_XML_production(t *testing.T) {
+	r := require.New(t)
+	app := New(Options{})
+	app.Env = "production"
+	app.GET("/", func(c Context) error {
+		return c.Error(http.StatusUnauthorized, fmt.Errorf("boom"))
+	})
+
+	w := httptest.New(app)
+	res := w.XML("/").Get()
+	r.Equal(http.StatusUnauthorized, res.Code)
+	ct := res.Header().Get("content-type")
+	r.Equal("text/xml", ct)
+	b := res.Body.String()
+	r.Contains(b, `<response code="401">`)
+	r.Contains(b, fmt.Sprintf(`<error>%s</error>`, http.StatusText(http.StatusUnauthorized)))
+	r.NotContains(b, `<trace>`)
+	r.NotContains(b, `</trace>`)
+	r.Contains(b, `</response>`)
+}
+
 func Test_PanicHandler(t *testing.T) {
 	app := New(Options{})
 	app.GET("/string", func(c Context) error {