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

safer error handling #2322

Merged
merged 4 commits into from
Sep 21, 2022
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 README.md
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ First, thank you so much for wanting to contribute! It means so much that you ca
- Consider opening an issue **BEFORE** creating a Pull request (PR): you won't
lose your time on fixing non-existing bugs, or fixing the wrong bug. Also we
can help you to produce the best PR!
- Open a PR against the `main` branch if your PR is for mainstrem or version
- Open a PR against the `main` branch if your PR is for mainstream or version
specific branch e.g. `v1` if your PR is for specific version.
Note that the valid branch for a new feature request PR should be `main`
while a PR against a version specific branch are allowed only for bugfixes.
Expand Down
2 changes: 1 addition & 1 deletion errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ func defaultErrorHandler(status int, origErr error, c Context) error {
c.Logger().Error(origErr)
c.Response().WriteHeader(status)

if env != nil && env.(string) == "production" {
if env != nil && env.(string) != "development" {
switch strings.ToLower(requestCT) {
case "application/json", "text/json", "json", "application/xml", "text/xml", "xml":
defaultErrorResponse = &ErrorResponse{
Expand Down
103 changes: 48 additions & 55 deletions errors_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,81 +66,74 @@ func Test_defaultErrorHandler_Logger(t *testing.T) {
}

func Test_defaultErrorHandler_JSON_development(t *testing.T) {
r := require.New(t)
app := New(Options{})
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, `"error":"boom"`)
r.Contains(b, `"trace":"`)
testDefaultErrorHandler(t, "application/json", "development")
}

func Test_defaultErrorHandler_XML_development(t *testing.T) {
r := require.New(t)
app := New(Options{})
app.GET("/", func(c Context) error {
return c.Error(http.StatusUnauthorized, fmt.Errorf("boom"))
})
testDefaultErrorHandler(t, "text/xml", "development")
}

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, `<error>boom</error>`)
r.Contains(b, `<trace>`)
r.Contains(b, `</trace>`)
r.Contains(b, `</response>`)
func Test_defaultErrorHandler_JSON_staging(t *testing.T) {
testDefaultErrorHandler(t, "application/json", "staging")
}

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"))
})
func Test_defaultErrorHandler_XML_staging(t *testing.T) {
testDefaultErrorHandler(t, "text/xml", "staging")
}

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_JSON_production(t *testing.T) {
testDefaultErrorHandler(t, "application/json", "production")
}

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

func testDefaultErrorHandler(t *testing.T, contentType, env string) {
r := require.New(t)
app := New(Options{})
app.Env = "production"
app.Env = env
app.GET("/", func(c Context) error {
return c.Error(http.StatusUnauthorized, fmt.Errorf("boom"))
})

w := httptest.New(app)
res := w.XML("/").Get()
var res *httptest.Response
if contentType == "application/json" {
res = w.JSON("/").Get().Response
} else {
res = w.XML("/").Get().Response
}
r.Equal(http.StatusUnauthorized, res.Code)
ct := res.Header().Get("content-type")
r.Equal("text/xml", ct)
r.Equal(contentType, 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>`)

if env == "development" {
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>`)
} else {
r.Contains(b, `"code":401`)
r.Contains(b, `"error":"boom"`)
r.Contains(b, `"trace":"`)
}
} else {
if contentType == "text/xml" {
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>`)
} else {
r.Contains(b, `"code":401`)
r.Contains(b, fmt.Sprintf(`"error":"%s"`, http.StatusText(http.StatusUnauthorized)))
r.NotContains(b, `"trace":"`)
}
}
}

func Test_defaultErrorHandler_nil_error(t *testing.T) {
Expand Down