Skip to content

Commit f518c5c

Browse files
authored
feat(fxhttpserver): Updated panic recovery to prevent observability middlewares interruption (#212)
1 parent 9293b8f commit f518c5c

File tree

4 files changed

+90
-7
lines changed

4 files changed

+90
-7
lines changed

fxhttpserver/go.mod

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,9 @@ require (
1010
github.com/ankorstore/yokai/fxmetrics v1.2.0
1111
github.com/ankorstore/yokai/fxtrace v1.2.0
1212
github.com/ankorstore/yokai/generate v1.1.0
13-
github.com/ankorstore/yokai/httpserver v1.4.1
13+
github.com/ankorstore/yokai/httpserver v1.5.0
1414
github.com/ankorstore/yokai/log v1.2.0
15-
github.com/ankorstore/yokai/trace v1.2.0
15+
github.com/ankorstore/yokai/trace v1.3.0
1616
github.com/labstack/echo/v4 v4.11.4
1717
github.com/prometheus/client_golang v1.19.0
1818
github.com/stretchr/testify v1.9.0

fxhttpserver/go.sum

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,12 @@ github.com/ankorstore/yokai/fxtrace v1.2.0 h1:SXlWbjKSsb2wVH+hXSE9OD2VwyqkznwwW+
1212
github.com/ankorstore/yokai/fxtrace v1.2.0/go.mod h1:ch72eVTlIedETOApK7SXk2NEWpn3yYeM018dNRccocg=
1313
github.com/ankorstore/yokai/generate v1.1.0 h1:tu3S+uEYh+2qNo8Rf/WxWneDjh49YgDPzSnJfF8JkXA=
1414
github.com/ankorstore/yokai/generate v1.1.0/go.mod h1:gqS/i20wnvCOhcXydYdiGcASzBaeuW7GK6YYg/kkuY4=
15-
github.com/ankorstore/yokai/httpserver v1.4.1 h1:Zz25h6fYvRsJ+1TtnbJP2fO4Dt/tD3+Kgqs2QkpCJzw=
16-
github.com/ankorstore/yokai/httpserver v1.4.1/go.mod h1:AOCL4cK2bPKrtGFULvOvc8mKHAOw2bLW30CKJra2BB0=
15+
github.com/ankorstore/yokai/httpserver v1.5.0 h1:42nfCFCGWuBKbwU8Jhlf1/ofrezDes8HlCa0mhiVoI8=
16+
github.com/ankorstore/yokai/httpserver v1.5.0/go.mod h1:AOCL4cK2bPKrtGFULvOvc8mKHAOw2bLW30CKJra2BB0=
1717
github.com/ankorstore/yokai/log v1.2.0 h1:jiuDiC0dtqIGIOsFQslUHYoFJ1qjI+rOMa6dI1LBf2Y=
1818
github.com/ankorstore/yokai/log v1.2.0/go.mod h1:MVvUcms1AYGo0BT6l88B9KJdvtK6/qGKdgyKVXfbmyc=
19-
github.com/ankorstore/yokai/trace v1.2.0 h1:Jnl++IGNpDYumsZJXP3qjhMdvyHbejiajQwIlU604w0=
20-
github.com/ankorstore/yokai/trace v1.2.0/go.mod h1:m7EL2MRBilgCtrly5gA4F0jkGSXR2EbG6LsotbTJ4nA=
19+
github.com/ankorstore/yokai/trace v1.3.0 h1:0ji32oymIcxTmH5h6GRWLo5ypwBbWrZkXRf9rWF9070=
20+
github.com/ankorstore/yokai/trace v1.3.0/go.mod h1:m7EL2MRBilgCtrly5gA4F0jkGSXR2EbG6LsotbTJ4nA=
2121
github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM=
2222
github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw=
2323
github.com/cenkalti/backoff/v4 v4.2.1 h1:y4OZtCnogmCPw98Zjyt5a6+QwPLGkiQsYW5oUqylYbM=

fxhttpserver/module.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ import (
1212
httpservermiddleware "github.com/ankorstore/yokai/httpserver/middleware"
1313
"github.com/ankorstore/yokai/log"
1414
"github.com/labstack/echo/v4"
15+
"github.com/labstack/echo/v4/middleware"
16+
gommonlog "github.com/labstack/gommon/log"
1517
"github.com/prometheus/client_golang/prometheus"
1618
"go.opentelemetry.io/otel/trace"
1719
"go.uber.org/fx"
@@ -71,7 +73,6 @@ func NewFxHttpServer(p FxHttpServerParam) (*echo.Echo, error) {
7173
httpServer, err := p.Factory.Create(
7274
httpserver.WithDebug(appDebug),
7375
httpserver.WithBanner(false),
74-
httpserver.WithRecovery(true),
7576
httpserver.WithLogger(echoLogger),
7677
httpserver.WithRenderer(renderer),
7778
httpserver.WithHttpErrorHandler(
@@ -181,6 +182,12 @@ func withDefaultMiddlewares(httpServer *echo.Echo, p FxHttpServerParam) *echo.Ec
181182
httpServer.Use(httpservermiddleware.RequestMetricsMiddlewareWithConfig(metricsMiddlewareConfig))
182183
}
183184

185+
// recovery middleware
186+
httpServer.Use(middleware.RecoverWithConfig(middleware.RecoverConfig{
187+
DisableErrorHandler: true,
188+
LogLevel: gommonlog.ERROR,
189+
}))
190+
184191
return httpServer
185192
}
186193

fxhttpserver/module_test.go

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,10 @@ var (
6969

7070
return c.JSON(http.StatusOK, "concrete")
7171
}
72+
73+
panicHandler = func(c echo.Context) error {
74+
panic("test panic")
75+
}
7276
)
7377

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

707+
func TestModuleWithPanicRecoveryAndDebug(t *testing.T) {
708+
t.Setenv("APP_CONFIG_PATH", "testdata/config")
709+
t.Setenv("APP_DEBUG", "true")
710+
711+
var httpServer *echo.Echo
712+
var logBuffer logtest.TestLogBuffer
713+
var traceExporter tracetest.TestTraceExporter
714+
var metricsRegistry *prometheus.Registry
715+
716+
fxtest.New(
717+
t,
718+
fx.NopLogger,
719+
fxconfig.FxConfigModule,
720+
fxlog.FxLogModule,
721+
fxtrace.FxTraceModule,
722+
fxmetrics.FxMetricsModule,
723+
fxgenerate.FxGenerateModule,
724+
fxhttpserver.FxHttpServerModule,
725+
fx.Provide(service.NewTestService),
726+
fx.Options(
727+
fxhttpserver.AsHandler("GET", "/panic", panicHandler),
728+
),
729+
fx.Populate(&httpServer, &logBuffer, &traceExporter, &metricsRegistry),
730+
).RequireStart().RequireStop()
731+
732+
// [GET] /bar
733+
req := httptest.NewRequest(http.MethodGet, "/panic", nil)
734+
rec := httptest.NewRecorder()
735+
httpServer.ServeHTTP(rec, req)
736+
737+
assert.Equal(t, http.StatusInternalServerError, rec.Code)
738+
assert.Contains(t, rec.Body.String(), `"message": "test panic"`)
739+
assert.Contains(t, rec.Body.String(), `"stack": "*errors.errorString test panic`)
740+
741+
logtest.AssertContainLogRecord(t, logBuffer, map[string]interface{}{
742+
"level": "error",
743+
"service": "test",
744+
"module": "httpserver",
745+
"message": "[PANIC RECOVER] test panic",
746+
})
747+
748+
logtest.AssertContainLogRecord(t, logBuffer, map[string]interface{}{
749+
"level": "error",
750+
"error": "test panic",
751+
"service": "test",
752+
"module": "httpserver",
753+
"stack": "*errors.errorString test panic",
754+
"message": "error handler",
755+
})
756+
757+
tracetest.AssertHasTraceSpan(
758+
t,
759+
traceExporter,
760+
"GET /panic",
761+
semconv.HTTPMethod(http.MethodGet),
762+
semconv.HTTPRoute("/panic"),
763+
semconv.HTTPStatusCode(http.StatusInternalServerError),
764+
)
765+
766+
expectedMetric := `
767+
# HELP http_server_requests_total Number of processed HTTP requests
768+
# TYPE http_server_requests_total counter
769+
http_server_requests_total{method="GET",path="/panic",status="5xx"} 1
770+
`
771+
err := testutil.GatherAndCompare(
772+
metricsRegistry,
773+
strings.NewReader(expectedMetric),
774+
"http_server_requests_total",
775+
)
776+
assert.NoError(t, err)
777+
}
778+
703779
func TestModuleWithMetrics(t *testing.T) {
704780
t.Setenv("APP_CONFIG_PATH", "testdata/config")
705781
t.Setenv("APP_DEBUG", "true")

0 commit comments

Comments
 (0)