diff --git a/httpserver/README.md b/httpserver/README.md index aade5955..aedfbfc3 100644 --- a/httpserver/README.md +++ b/httpserver/README.md @@ -57,7 +57,6 @@ var server, _ = httpserver.NewDefaultHttpServerFactory().Create() var server, _ = httpserver.NewDefaultHttpServerFactory().Create( httpserver.WithDebug(false), // debug disabled by default httpserver.WithBanner(false), // banner disabled by default - httpserver.WithRecovery(true), // panic recovery middleware enabled by default httpserver.WithLogger(log.New("default")), // echo default logger httpserver.WithBinder(&echo.DefaultBinder{}), // echo default binder httpserver.WithJsonSerializer(&echo.DefaultJSONSerializer{}), // echo default json serializer diff --git a/httpserver/error.go b/httpserver/error.go index 5f913c98..4586f54e 100644 --- a/httpserver/error.go +++ b/httpserver/error.go @@ -36,7 +36,10 @@ func JsonErrorHandler(obfuscate bool, stack bool) echo.HTTPErrorHandler { var logRespFields map[string]interface{} if stack { - errStack := errors.New(err).ErrorStack() + errStack := "n/a" + if err != nil { + errStack = errors.New(err).ErrorStack() + } switch m := httpError.Message.(type) { case error: diff --git a/httpserver/factory.go b/httpserver/factory.go index 474ce173..aca9e8d2 100644 --- a/httpserver/factory.go +++ b/httpserver/factory.go @@ -2,7 +2,6 @@ package httpserver import ( "github.com/labstack/echo/v4" - "github.com/labstack/echo/v4/middleware" ) // HttpServerFactory is the interface for [echo.Echo] factories. @@ -28,7 +27,6 @@ func NewDefaultHttpServerFactory() HttpServerFactory { // var server, _ = httpserver.NewDefaultHttpServerFactory().Create( // httpserver.WithDebug(false), // debug disabled by default // httpserver.WithBanner(false), // banner disabled by default -// httpserver.WithRecovery(true), // panic recovery middleware enabled by default // httpserver.WithLogger(log.New("default")), // echo default logger // httpserver.WithBinder(&echo.DefaultBinder{}), // echo default binder // httpserver.WithJsonSerializer(&echo.DefaultJSONSerializer{}), // echo default json serializer @@ -57,9 +55,5 @@ func (f *DefaultHttpServerFactory) Create(options ...HttpServerOption) (*echo.Ec httpServer.Renderer = appliedOpts.Renderer } - if appliedOpts.Recovery { - httpServer.Use(middleware.Recover()) - } - return httpServer, nil } diff --git a/httpserver/factory_test.go b/httpserver/factory_test.go index 21c5983e..1d5e05e5 100644 --- a/httpserver/factory_test.go +++ b/httpserver/factory_test.go @@ -52,7 +52,6 @@ func TestCreate(t *testing.T) { httpServer, err := httpserver.NewDefaultHttpServerFactory().Create( httpserver.WithDebug(true), httpserver.WithBanner(true), - httpserver.WithRecovery(true), httpserver.WithLogger(echoLogger), httpserver.WithBinder(binder), httpserver.WithJsonSerializer(jsonSerializer), @@ -2077,41 +2076,3 @@ func TestCreateWithRequestMetricsAndWithoutNormalization(t *testing.T) { ) assert.NoError(t, err) } - -func TestCreateWithPanicRecovery(t *testing.T) { - t.Parallel() - - logBuffer := logtest.NewDefaultTestLogBuffer() - logger, err := log.NewDefaultLoggerFactory().Create( - log.WithOutputWriter(logBuffer), - ) - assert.NoError(t, err) - - httpServer, err := httpserver.NewDefaultHttpServerFactory().Create( - httpserver.WithLogger(httpserver.NewEchoLogger(logger)), - httpserver.WithRecovery(true), - ) - assert.NoError(t, err) - assert.IsType(t, &echo.Echo{}, httpServer) - - httpServer.Use(middleware.RequestLoggerMiddleware()) - - httpServer.GET("/panic", func(c echo.Context) error { - panic("custom panic") - }) - - defer func() { - if r := recover(); r != nil { - t.Error("should have recovered by itself") - } - }() - - for i := 0; i <= 5; i++ { - 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":"Internal Server Error"}`) - } -} diff --git a/httpserver/logger.go b/httpserver/logger.go index ba337ed3..b8dc9996 100644 --- a/httpserver/logger.go +++ b/httpserver/logger.go @@ -171,17 +171,17 @@ func (e *EchoLogger) Panicj(j echologger.JSON) { // Print produces a log with no level. func (e *EchoLogger) Print(i ...interface{}) { - e.logger.WithLevel(zerolog.NoLevel).Str("level", "-").Msg(fmt.Sprint(i...)) + e.logger.WithLevel(zerolog.NoLevel).Str("level", "---").Msg(fmt.Sprint(i...)) } // Printf produces a formatted log with no level. func (e *EchoLogger) Printf(format string, i ...interface{}) { - e.logger.WithLevel(zerolog.NoLevel).Str("level", "-").Msgf(format, i...) + e.logger.WithLevel(zerolog.NoLevel).Str("level", "---").Msgf(format, i...) } // Printj produces a json log with no level. func (e *EchoLogger) Printj(j echologger.JSON) { - e.logJSON(e.logger.WithLevel(zerolog.NoLevel).Str("level", "-"), j) + e.logJSON(e.logger.WithLevel(zerolog.NoLevel).Str("level", "---"), j) } func (e *EchoLogger) logJSON(event *zerolog.Event, j echologger.JSON) { @@ -192,29 +192,29 @@ func (e *EchoLogger) logJSON(event *zerolog.Event, j echologger.JSON) { event.Msg("") } -//nolint:exhaustive func convertZeroLevel(level zerolog.Level) echologger.Lvl { switch level { - case zerolog.TraceLevel: - return echologger.DEBUG - case zerolog.DebugLevel: + case zerolog.TraceLevel, zerolog.DebugLevel: return echologger.DEBUG + case zerolog.InfoLevel: + return echologger.INFO case zerolog.WarnLevel: return echologger.WARN - case zerolog.ErrorLevel: + case zerolog.ErrorLevel, zerolog.FatalLevel, zerolog.PanicLevel: return echologger.ERROR - case zerolog.NoLevel: + case zerolog.NoLevel, zerolog.Disabled: return echologger.OFF default: return echologger.INFO } } -//nolint:exhaustive func convertEchoLevel(level echologger.Lvl) zerolog.Level { switch level { case echologger.DEBUG: return zerolog.DebugLevel + case echologger.INFO: + return zerolog.InfoLevel case echologger.WARN: return zerolog.WarnLevel case echologger.ERROR: diff --git a/httpserver/logger_test.go b/httpserver/logger_test.go index ff812603..0365eb66 100644 --- a/httpserver/logger_test.go +++ b/httpserver/logger_test.go @@ -241,19 +241,19 @@ func TestPrintLogging(t *testing.T) { echoLogger.Print("test regular message") logtest.AssertHasLogRecord(t, buffer, map[string]interface{}{ - "level": "-", + "level": "---", "message": "test regular message", }) echoLogger.Printf("test placeholder message: %s", "placeholder") logtest.AssertHasLogRecord(t, buffer, map[string]interface{}{ - "level": "-", + "level": "---", "message": "test placeholder message: placeholder", }) echoLogger.Printj(echologger.JSON{"message": "test json message"}) logtest.AssertHasLogRecord(t, buffer, map[string]interface{}{ - "level": "-", + "level": "---", "message": "test json message", }) } diff --git a/httpserver/option.go b/httpserver/option.go index 16601636..08f8c5fe 100644 --- a/httpserver/option.go +++ b/httpserver/option.go @@ -9,7 +9,6 @@ import ( type Options struct { Debug bool Banner bool - Recovery bool Logger echo.Logger Binder echo.Binder JsonSerializer echo.JSONSerializer @@ -22,7 +21,6 @@ func DefaultHttpServerOptions() Options { return Options{ Debug: false, Banner: false, - Recovery: true, Logger: log.New("default"), Binder: &echo.DefaultBinder{}, JsonSerializer: &echo.DefaultJSONSerializer{}, @@ -48,13 +46,6 @@ func WithBanner(b bool) HttpServerOption { } } -// WithRecovery is used to activate the server automatic panic recovery. -func WithRecovery(r bool) HttpServerOption { - return func(o *Options) { - o.Recovery = r - } -} - // WithLogger is used to specify a [echo.Logger] to be used by the server. func WithLogger(l echo.Logger) HttpServerOption { return func(o *Options) { diff --git a/httpserver/option_test.go b/httpserver/option_test.go index 1deed923..da3ad1eb 100644 --- a/httpserver/option_test.go +++ b/httpserver/option_test.go @@ -27,15 +27,6 @@ func TestWithBanner(t *testing.T) { assert.Equal(t, true, opt.Banner) } -func TestWithRecovery(t *testing.T) { - t.Parallel() - - opt := httpserver.DefaultHttpServerOptions() - httpserver.WithRecovery(false)(&opt) - - assert.Equal(t, false, opt.Recovery) -} - func TestWithLogger(t *testing.T) { t.Parallel()