Repository: gobuffalo/buffalo
Pull request #1351 (2018-10-04 19:07:29)

Better handle errors display as JSON or XML
* Make sure using the XML mime type returns an XML-formatted error.
* Use the same struct for XML & JSON.
* Add a new attribute to have only the cause and not the full trace.

codecov[bot]: # [Codecov](https://codecov.io/gh/gobuffalo/buffalo/pull/1351?src=pr&el=h1) Report
> Merging [#1351](https://codecov.io/gh/gobuffalo/buffalo/pull/1351?src=pr&el=desc) into [development](https://codecov.io/gh/gobuffalo/buffalo/commit/3ceb2bc8829febf5f8044f743c8a9ec01a260beb?src=pr&el=desc) will **increase** coverage by `0.06%`.
> The diff coverage is `73.33%`.

[![Impacted file tree graph](https://codecov.io/gh/gobuffalo/buffalo/pull/1351/graphs/tree.svg?width=650&token=1Yyy8wHMMl&height=150&src=pr)](https://codecov.io/gh/gobuffalo/buffalo/pull/1351?src=pr&el=tree)

```diff
@@               Coverage Diff               @@
##           development    #1351      +/-   ##
===============================================
+ Coverage        48.76%   48.82%   +0.06%     
===============================================
  Files               92       92              
  Lines             4048     4057       +9     
===============================================
+ Hits              1974     1981       +7     
- Misses            1968     1969       +1     
- Partials           106      107       +1
```


| [Impacted Files](https://codecov.io/gh/gobuffalo/buffalo/pull/1351?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [errors.go](https://codecov.io/gh/gobuffalo/buffalo/pull/1351/diff?src=pr&el=tree#diff-ZXJyb3JzLmdv) | `75% <73.33%> (+0.18%)` | :arrow_up: |
| [response.go](https://codecov.io/gh/gobuffalo/buffalo/pull/1351/diff?src=pr&el=tree#diff-cmVzcG9uc2UuZ28=) | `30% <0%> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/gobuffalo/buffalo/pull/1351?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/gobuffalo/buffalo/pull/1351?src=pr&el=footer). Last update [3ceb2bc...56bd263](https://codecov.io/gh/gobuffalo/buffalo/pull/1351?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

stanislas-m: Fixes  #1353.

Patch:
diff --git a/errors.go b/errors.go
index aa261939..bac3c6b2 100644
--- a/errors.go
+++ b/errors.go
@@ -3,6 +3,7 @@ package buffalo
 import (
 	"database/sql"
 	"encoding/json"
+	"encoding/xml"
 	"fmt"
 	"net/http"
 	"sort"
@@ -131,6 +132,14 @@ func productionErrorResponseFor(status int) []byte {
 	return []byte(prodErrorTmpl)
 }
 
+// ErrorResponse is a used to display errors as JSON or XML
+type ErrorResponse struct {
+	XMLName xml.Name `json:"-" xml:"response"`
+	Error   string   `json:"error" xml:"error"`
+	Trace   string   `json:"trace" xml:"trace"`
+	Code    int      `json:"code" xml:"code,attr"`
+}
+
 func defaultErrorHandler(status int, origErr error, c Context) error {
 	env := c.Value("env")
 	ct := defaults.String(httpx.ContentType(c.Request()), "text/html; charset=utf-8")
@@ -145,20 +154,29 @@ func defaultErrorHandler(status int, origErr error, c Context) error {
 		return nil
 	}
 
-	msg := fmt.Sprintf("%+v", origErr)
+	trace := fmt.Sprintf("%+v", origErr)
 	switch strings.ToLower(ct) {
 	case "application/json", "text/json", "json":
-		err := json.NewEncoder(c.Response()).Encode(map[string]interface{}{
-			"error": msg,
-			"code":  status,
+		err := json.NewEncoder(c.Response()).Encode(&ErrorResponse{
+			Error: errors.Cause(origErr).Error(),
+			Trace: trace,
+			Code:  status,
 		})
 		if err != nil {
 			return errors.WithStack(err)
 		}
 	case "application/xml", "text/xml", "xml":
+		err := xml.NewEncoder(c.Response()).Encode(&ErrorResponse{
+			Error: errors.Cause(origErr).Error(),
+			Trace: trace,
+			Code:  status,
+		})
+		if err != nil {
+			return errors.WithStack(err)
+		}
 	default:
 		if err := c.Request().ParseForm(); err != nil {
-			msg = fmt.Sprintf("%s\n%s", err.Error(), msg)
+			trace = fmt.Sprintf("%s\n%s", err.Error(), trace)
 		}
 		routes := c.Value("routes")
 		if cd, ok := c.(*DefaultContext); ok {
@@ -167,7 +185,7 @@ func defaultErrorHandler(status int, origErr error, c Context) error {
 		}
 		data := map[string]interface{}{
 			"routes":      routes,
-			"error":       msg,
+			"error":       trace,
 			"status":      status,
 			"data":        c.Data(),
 			"params":      c.Params(),


Test patch:
diff --git a/errors_test.go b/errors_test.go
index 31f3681e..0b778201 100644
--- a/errors_test.go
+++ b/errors_test.go
@@ -23,6 +23,44 @@ func Test_defaultErrorHandler_SetsContentType(t *testing.T) {
 	r.Equal("text/html; charset=utf-8", ct)
 }
 
+func Test_defaultErrorHandler_JSON(t *testing.T) {
+	r := require.New(t)
+	app := New(Options{})
+	app.GET("/", func(c Context) error {
+		return c.Error(401, errors.New("boom"))
+	})
+
+	w := httptest.New(app)
+	res := w.JSON("/").Get()
+	r.Equal(401, 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, `"error":"boom"`)
+	r.Contains(b, `"trace":"`)
+}
+
+func Test_defaultErrorHandler_XML(t *testing.T) {
+	r := require.New(t)
+	app := New(Options{})
+	app.GET("/", func(c Context) error {
+		return c.Error(401, errors.New("boom"))
+	})
+
+	w := httptest.New(app)
+	res := w.XML("/").Get()
+	r.Equal(401, res.Code)
+	ct := res.Header().Get("content-type")
+	r.Equal("application/xml", ct)
+	b := res.Body.String()
+	r.Contains(b, `<response code="401">`)
+	r.Contains(b, `<error>boom</error>`)
+	r.Contains(b, `<trace>`)
+	r.Contains(b, `</trace>`)
+	r.Contains(b, `</response>`)
+}
+
 func Test_PanicHandler(t *testing.T) {
 	app := New(Options{})
 	app.GET("/string", func(c Context) error {