Skip to content
This repository has been archived by the owner on Feb 24, 2024. It is now read-only.

Feature/improved error reporting in production #2108

Merged
merged 7 commits into from
May 21, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Dockerfile.build
Original file line number Diff line number Diff line change
Expand Up @@ -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/[email protected].22 | sh
RUN curl -sf https://gobinaries.com/gobuffalo/buffalo/[email protected].23 | sh
RUN go get github.com/gobuffalo/buffalo-pop/v2
RUN buffalo version

Expand Down
2 changes: 1 addition & 1 deletion Dockerfile.slim.build
Original file line number Diff line number Diff line change
Expand Up @@ -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/[email protected].22| sh
RUN curl -sf https://gobinaries.com/gobuffalo/buffalo/[email protected].23| sh
RUN go get github.com/gobuffalo/buffalo-pop/v2
RUN buffalo version

Expand Down
36 changes: 27 additions & 9 deletions errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
}

Expand All @@ -161,37 +161,48 @@ 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()

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
}
Expand Down Expand Up @@ -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 {
Expand Down
44 changes: 42 additions & 2 deletions errors_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down
Loading