Skip to content

feat(fxhttpserver): Updated panic recovery to prevent observability middlewares interruption #212

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Apr 17, 2024
Merged

feat(fxhttpserver): Updated panic recovery to prevent observability middlewares interruption #212

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1f43c0a
doc(main): Updated documentation
ekkinox Mar 31, 2024
38c7859
doc(main): Updated documentation
ekkinox Mar 31, 2024
bdf4859
Merge branch 'main' of github.com:ankorstore/yokai into doc/main
ekkinox Mar 31, 2024
92f535b
Merge branch 'main' of github.com:ankorstore/yokai into doc/main
ekkinox Apr 17, 2024
90ce81b
feat(fxhttpserver): Updated panic recovery to prevent observability m…
ekkinox Apr 17, 2024
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
4 changes: 2 additions & 2 deletions fxhttpserver/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ require (
github.com/ankorstore/yokai/fxmetrics v1.2.0
github.com/ankorstore/yokai/fxtrace v1.2.0
github.com/ankorstore/yokai/generate v1.1.0
github.com/ankorstore/yokai/httpserver v1.4.1
github.com/ankorstore/yokai/httpserver v1.5.0
github.com/ankorstore/yokai/log v1.2.0
github.com/ankorstore/yokai/trace v1.2.0
github.com/ankorstore/yokai/trace v1.3.0
github.com/labstack/echo/v4 v4.11.4
github.com/prometheus/client_golang v1.19.0
github.com/stretchr/testify v1.9.0
Expand Down
8 changes: 4 additions & 4 deletions fxhttpserver/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@ github.com/ankorstore/yokai/fxtrace v1.2.0 h1:SXlWbjKSsb2wVH+hXSE9OD2VwyqkznwwW+
github.com/ankorstore/yokai/fxtrace v1.2.0/go.mod h1:ch72eVTlIedETOApK7SXk2NEWpn3yYeM018dNRccocg=
github.com/ankorstore/yokai/generate v1.1.0 h1:tu3S+uEYh+2qNo8Rf/WxWneDjh49YgDPzSnJfF8JkXA=
github.com/ankorstore/yokai/generate v1.1.0/go.mod h1:gqS/i20wnvCOhcXydYdiGcASzBaeuW7GK6YYg/kkuY4=
github.com/ankorstore/yokai/httpserver v1.4.1 h1:Zz25h6fYvRsJ+1TtnbJP2fO4Dt/tD3+Kgqs2QkpCJzw=
github.com/ankorstore/yokai/httpserver v1.4.1/go.mod h1:AOCL4cK2bPKrtGFULvOvc8mKHAOw2bLW30CKJra2BB0=
github.com/ankorstore/yokai/httpserver v1.5.0 h1:42nfCFCGWuBKbwU8Jhlf1/ofrezDes8HlCa0mhiVoI8=
github.com/ankorstore/yokai/httpserver v1.5.0/go.mod h1:AOCL4cK2bPKrtGFULvOvc8mKHAOw2bLW30CKJra2BB0=
github.com/ankorstore/yokai/log v1.2.0 h1:jiuDiC0dtqIGIOsFQslUHYoFJ1qjI+rOMa6dI1LBf2Y=
github.com/ankorstore/yokai/log v1.2.0/go.mod h1:MVvUcms1AYGo0BT6l88B9KJdvtK6/qGKdgyKVXfbmyc=
github.com/ankorstore/yokai/trace v1.2.0 h1:Jnl++IGNpDYumsZJXP3qjhMdvyHbejiajQwIlU604w0=
github.com/ankorstore/yokai/trace v1.2.0/go.mod h1:m7EL2MRBilgCtrly5gA4F0jkGSXR2EbG6LsotbTJ4nA=
github.com/ankorstore/yokai/trace v1.3.0 h1:0ji32oymIcxTmH5h6GRWLo5ypwBbWrZkXRf9rWF9070=
github.com/ankorstore/yokai/trace v1.3.0/go.mod h1:m7EL2MRBilgCtrly5gA4F0jkGSXR2EbG6LsotbTJ4nA=
github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM=
github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw=
github.com/cenkalti/backoff/v4 v4.2.1 h1:y4OZtCnogmCPw98Zjyt5a6+QwPLGkiQsYW5oUqylYbM=
Expand Down
9 changes: 8 additions & 1 deletion fxhttpserver/module.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import (
httpservermiddleware "github.com/ankorstore/yokai/httpserver/middleware"
"github.com/ankorstore/yokai/log"
"github.com/labstack/echo/v4"
"github.com/labstack/echo/v4/middleware"
gommonlog "github.com/labstack/gommon/log"
"github.com/prometheus/client_golang/prometheus"
"go.opentelemetry.io/otel/trace"
"go.uber.org/fx"
Expand Down Expand Up @@ -71,7 +73,6 @@ func NewFxHttpServer(p FxHttpServerParam) (*echo.Echo, error) {
httpServer, err := p.Factory.Create(
httpserver.WithDebug(appDebug),
httpserver.WithBanner(false),
httpserver.WithRecovery(true),
httpserver.WithLogger(echoLogger),
httpserver.WithRenderer(renderer),
httpserver.WithHttpErrorHandler(
Expand Down Expand Up @@ -181,6 +182,12 @@ func withDefaultMiddlewares(httpServer *echo.Echo, p FxHttpServerParam) *echo.Ec
httpServer.Use(httpservermiddleware.RequestMetricsMiddlewareWithConfig(metricsMiddlewareConfig))
}

// recovery middleware
httpServer.Use(middleware.RecoverWithConfig(middleware.RecoverConfig{
DisableErrorHandler: true,
LogLevel: gommonlog.ERROR,
}))

return httpServer
}

Expand Down
76 changes: 76 additions & 0 deletions fxhttpserver/module_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,10 @@ var (

return c.JSON(http.StatusOK, "concrete")
}

panicHandler = func(c echo.Context) error {
panic("test panic")
}
)

//nolint:maintidx
Expand Down Expand Up @@ -700,6 +704,78 @@ func TestModuleWithEchoResources(t *testing.T) {
assert.Equal(t, "SAMEORIGIN", rec.Header().Get(echo.HeaderXFrameOptions)) // Secure middleware
}

func TestModuleWithPanicRecoveryAndDebug(t *testing.T) {
t.Setenv("APP_CONFIG_PATH", "testdata/config")
t.Setenv("APP_DEBUG", "true")

var httpServer *echo.Echo
var logBuffer logtest.TestLogBuffer
var traceExporter tracetest.TestTraceExporter
var metricsRegistry *prometheus.Registry

fxtest.New(
t,
fx.NopLogger,
fxconfig.FxConfigModule,
fxlog.FxLogModule,
fxtrace.FxTraceModule,
fxmetrics.FxMetricsModule,
fxgenerate.FxGenerateModule,
fxhttpserver.FxHttpServerModule,
fx.Provide(service.NewTestService),
fx.Options(
fxhttpserver.AsHandler("GET", "/panic", panicHandler),
),
fx.Populate(&httpServer, &logBuffer, &traceExporter, &metricsRegistry),
).RequireStart().RequireStop()

// [GET] /bar
req := httptest.NewRequest(http.MethodGet, "/panic", nil)
rec := httptest.NewRecorder()
httpServer.ServeHTTP(rec, req)

assert.Equal(t, http.StatusInternalServerError, rec.Code)
assert.Contains(t, rec.Body.String(), `"message": "test panic"`)
assert.Contains(t, rec.Body.String(), `"stack": "*errors.errorString test panic`)

logtest.AssertContainLogRecord(t, logBuffer, map[string]interface{}{
"level": "error",
"service": "test",
"module": "httpserver",
"message": "[PANIC RECOVER] test panic",
})

logtest.AssertContainLogRecord(t, logBuffer, map[string]interface{}{
"level": "error",
"error": "test panic",
"service": "test",
"module": "httpserver",
"stack": "*errors.errorString test panic",
"message": "error handler",
})

tracetest.AssertHasTraceSpan(
t,
traceExporter,
"GET /panic",
semconv.HTTPMethod(http.MethodGet),
semconv.HTTPRoute("/panic"),
semconv.HTTPStatusCode(http.StatusInternalServerError),
)

expectedMetric := `
# HELP http_server_requests_total Number of processed HTTP requests
# TYPE http_server_requests_total counter
http_server_requests_total{method="GET",path="/panic",status="5xx"} 1
`
err := testutil.GatherAndCompare(
metricsRegistry,
strings.NewReader(expectedMetric),
"http_server_requests_total",
)
assert.NoError(t, err)
}

func TestModuleWithMetrics(t *testing.T) {
t.Setenv("APP_CONFIG_PATH", "testdata/config")
t.Setenv("APP_DEBUG", "true")
Expand Down