From 3765f69608097cb80ad04da54dbdc562ec703430 Mon Sep 17 00:00:00 2001 From: Schparky <3172830+Schparky@users.noreply.github.com> Date: Fri, 16 Sep 2022 17:29:58 +0800 Subject: [PATCH 1/4] only provide detailed error output when GO_ENV = development --- errors.go | 7 ++-- errors_test.go | 103 +++++++++++++++++++++++-------------------------- 2 files changed, 52 insertions(+), 58 deletions(-) diff --git a/errors.go b/errors.go index 228bd3865..05f792e16 100644 --- a/errors.go +++ b/errors.go @@ -11,10 +11,11 @@ import ( "sort" "strings" - "github.com/gobuffalo/buffalo/internal/defaults" - "github.com/gobuffalo/buffalo/internal/httpx" "github.com/gobuffalo/events" "github.com/gobuffalo/plush/v4" + + "github.com/gobuffalo/buffalo/internal/defaults" + "github.com/gobuffalo/buffalo/internal/httpx" ) // HTTPError a typed error returned by http Handlers and used for choosing error handlers @@ -174,7 +175,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{ diff --git a/errors_test.go b/errors_test.go index bf55f6002..10d359d03 100644 --- a/errors_test.go +++ b/errors_test.go @@ -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, ``) - r.Contains(b, `boom`) - r.Contains(b, ``) - r.Contains(b, ``) - r.Contains(b, ``) +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, ``) - r.Contains(b, fmt.Sprintf(`%s`, http.StatusText(http.StatusUnauthorized))) - r.NotContains(b, ``) - r.NotContains(b, ``) - r.Contains(b, ``) + + if env == "development" { + if contentType == "text/xml" { + r.Contains(b, ``) + r.Contains(b, `boom`) + r.Contains(b, ``) + r.Contains(b, ``) + r.Contains(b, ``) + } else { + r.Contains(b, `"code":401`) + r.Contains(b, `"error":"boom"`) + r.Contains(b, `"trace":"`) + } + } else { + if contentType == "text/xml" { + r.Contains(b, ``) + r.Contains(b, fmt.Sprintf(`%s`, http.StatusText(http.StatusUnauthorized))) + r.NotContains(b, ``) + r.NotContains(b, ``) + r.Contains(b, ``) + } 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) { From 2a94f0d276adbf5f47391d4e5d466f7b521f832f Mon Sep 17 00:00:00 2001 From: Schparky <3172830+Schparky@users.noreply.github.com> Date: Fri, 16 Sep 2022 17:30:12 +0800 Subject: [PATCH 2/4] fix typo --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 80971fd26..acc5d5d4a 100644 --- a/README.md +++ b/README.md @@ -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. From 77b5e6f94a6c99f93fb8e69ddb8d2c3e1648f2a9 Mon Sep 17 00:00:00 2001 From: Schparky <3172830+Schparky@users.noreply.github.com> Date: Fri, 16 Sep 2022 17:38:25 +0800 Subject: [PATCH 3/4] restore original import ordering --- errors.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/errors.go b/errors.go index 05f792e16..f61839ee7 100644 --- a/errors.go +++ b/errors.go @@ -11,11 +11,11 @@ import ( "sort" "strings" - "github.com/gobuffalo/events" - "github.com/gobuffalo/plush/v4" "github.com/gobuffalo/buffalo/internal/defaults" "github.com/gobuffalo/buffalo/internal/httpx" + "github.com/gobuffalo/events" + "github.com/gobuffalo/plush/v4" ) // HTTPError a typed error returned by http Handlers and used for choosing error handlers From 13d320b2917516df9d7a8674c9f9b7975c75a0c6 Mon Sep 17 00:00:00 2001 From: Schparky <3172830+Schparky@users.noreply.github.com> Date: Fri, 16 Sep 2022 17:38:57 +0800 Subject: [PATCH 4/4] whitespace only --- errors.go | 1 - 1 file changed, 1 deletion(-) diff --git a/errors.go b/errors.go index f61839ee7..ee6a619f2 100644 --- a/errors.go +++ b/errors.go @@ -11,7 +11,6 @@ import ( "sort" "strings" - "github.com/gobuffalo/buffalo/internal/defaults" "github.com/gobuffalo/buffalo/internal/httpx" "github.com/gobuffalo/events"