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

Piti/feat/trace on dev test #2358

Closed
wants to merge 3 commits into from
Closed
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
3 changes: 2 additions & 1 deletion default_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"encoding/json"
"errors"
"fmt"
pkgError "github.com/pkg/errors"
"io"
"net/http"
"net/url"
Expand Down Expand Up @@ -196,7 +197,7 @@ func (d *DefaultContext) LogFields(values map[string]interface{}) {
}

func (d *DefaultContext) Error(status int, err error) error {
return HTTPError{Status: status, Cause: err}
return HTTPError{Status: status, Cause: pkgError.WithStack(err)}
}

var mapType = reflect.ValueOf(map[string]interface{}{}).Type()
Expand Down
40 changes: 36 additions & 4 deletions errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/gobuffalo/buffalo/internal/httpx"
"github.com/gobuffalo/events"
"github.com/gobuffalo/plush/v4"
pkgError "github.com/pkg/errors"
)

// HTTPError a typed error returned by http Handlers and used for choosing error handlers
Expand All @@ -36,6 +37,16 @@ func (h HTTPError) Error() string {
return "unknown cause"
}

func (h HTTPError) StackTrace() pkgError.StackTrace {
if h.Cause == nil {
return nil
}
if tracer, ok := h.Cause.(stackTracer); ok {
return tracer.StackTrace()
}
return nil
}

// ErrorHandler interface for handling an error for a
// specific status code.
type ErrorHandler func(int, error, Context) error
Expand Down Expand Up @@ -165,8 +176,27 @@ type ErrorResponse struct {

const defaultErrorCT = "text/html; charset=utf-8"

func defaultErrorHandler(status int, origErr error, c Context) error {
type stackTracer interface {
StackTrace() pkgError.StackTrace
}

// Attempt to extract stacktrace error with stack if it implements stacktracer
func getTrace(err error) string {

if errStack, ok := err.(stackTracer); ok {
return fmt.Sprintf("%+v", errStack.StackTrace())
} else {
return err.Error()
}
}

func isUnsafeEnvironment(c Context) bool {
env := c.Value("env")
return env != nil && env.(string) != "development" && env.(string) != "test"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the environment "test" should execute the same code set as "production" (with data for testing) and this is not acceptable.

}

func defaultErrorHandler(status int, origErr error, c Context) error {

requestCT := defaults.String(httpx.ContentType(c.Request()), defaultErrorCT)

var defaultErrorResponse *ErrorResponse
Expand All @@ -175,7 +205,7 @@ func defaultErrorHandler(status int, origErr error, c Context) error {
c.Logger().Error(origErr)
c.Response().WriteHeader(status)

if env != nil && env.(string) != "development" {
if isUnsafeEnvironment(c) {
switch strings.ToLower(requestCT) {
case "application/json", "text/json", "json", "application/xml", "text/xml", "xml":
defaultErrorResponse = &ErrorResponse{
Expand All @@ -190,7 +220,7 @@ func defaultErrorHandler(status int, origErr error, c Context) error {
}
}

trace := origErr.Error()
trace := getTrace(origErr)
if cause := errors.Unwrap(origErr); cause != nil {
origErr = cause
}
Expand All @@ -217,7 +247,9 @@ func defaultErrorHandler(status int, origErr error, c Context) error {
default:
c.Response().Header().Set("content-type", defaultErrorCT)
if err := c.Request().ParseForm(); err != nil {
trace = fmt.Sprintf("%s\n%s", err.Error(), trace)
trace = fmt.Sprintf("%s\n%s\n%s", err.Error(), origErr.Error(), trace)
} else {
trace = fmt.Sprintf("%s\n%s", origErr.Error(), trace)
}

routes := c.Value("routes")
Expand Down
19 changes: 15 additions & 4 deletions errors_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package buffalo

import (
errors "errors"
"fmt"
"net/http"
"os"
Expand All @@ -13,7 +14,7 @@ import (
"github.com/stretchr/testify/require"
)

//testLoggerHook is useful to test whats being logged.
// testLoggerHook is useful to test whats being logged.
type testLoggerHook struct {
errors []*logrus.Entry
}
Expand Down Expand Up @@ -65,6 +66,14 @@ func Test_defaultErrorHandler_Logger(t *testing.T) {
r.Equal(http.StatusUnauthorized, testHook.errors[0].Data["status"])
}

func Test_defaultErrorHandler_JSON_test(t *testing.T) {
testDefaultErrorHandler(t, "application/json", "test")
}

func Test_defaultErrorHandler_XML_test(t *testing.T) {
testDefaultErrorHandler(t, "text/xml", "test")
}

func Test_defaultErrorHandler_JSON_development(t *testing.T) {
testDefaultErrorHandler(t, "application/json", "development")
}
Expand Down Expand Up @@ -94,7 +103,7 @@ func testDefaultErrorHandler(t *testing.T, contentType, env string) {
app := New(Options{})
app.Env = env
app.GET("/", func(c Context) error {
return c.Error(http.StatusUnauthorized, fmt.Errorf("boom"))
return c.Error(http.StatusUnauthorized, errors.New("boom"))
})

w := httptest.New(app)
Expand All @@ -108,18 +117,20 @@ func testDefaultErrorHandler(t *testing.T, contentType, env string) {
ct := res.Header().Get("content-type")
r.Equal(contentType, ct)
b := res.Body.String()

if env == "development" {
isDevOrTest := env == "development" || env == "test"
if isDevOrTest {
if contentType == "text/xml" {
r.Contains(b, `<response code="401">`)
r.Contains(b, `<error>boom</error>`)
r.Contains(b, `<trace>`)
r.Contains(b, `</trace>`)
r.Contains(b, `</response>`)
r.Contains(b, "github.com") // making sure trace is not empty
} else {
r.Contains(b, `"code":401`)
r.Contains(b, `"error":"boom"`)
r.Contains(b, `"trace":"`)
r.Contains(b, "github.com") // making sure trace is not empty
}
} else {
if contentType == "text/xml" {
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ require (
github.com/gorilla/mux v1.8.0
github.com/gorilla/sessions v1.2.1
github.com/monoculum/formam v3.5.5+incompatible
github.com/pkg/errors v0.9.1 // indirect
github.com/psanford/memfs v0.0.0-20210214183328-a001468d78ef
github.com/sirupsen/logrus v1.9.0
github.com/spf13/cobra v1.6.1
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,8 @@ github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrk
github.com/monoculum/formam v3.5.5+incompatible h1:iPl5csfEN96G2N2mGu8V/ZB62XLf9ySTpC8KRH6qXec=
github.com/monoculum/formam v3.5.5+incompatible/go.mod h1:RKgILGEJq24YyJ2ban8EO0RUVSJlF1pGsEvoLEACr/Q=
github.com/pkg/diff v0.0.0-20210226163009-20ebb0f2a09e/go.mod h1:pJLUxLENpZxwdsKMEsNbx1VGcRFpLqf3715MtcvvzbA=
github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/psanford/memfs v0.0.0-20210214183328-a001468d78ef h1:NKxTG6GVGbfMXc2mIk+KphcH6hagbVXhcFkbTgYleTI=
Expand Down