Skip to content

Commit 116b4fc

Browse files
ekkinoxekkinox
authored
feat(httpserver): Removed automatic panic recovery and enhanced error handler error stack printing (#210)
1 parent 7239535 commit 116b4fc

File tree

8 files changed

+17
-78
lines changed

8 files changed

+17
-78
lines changed

‎httpserver/README.md‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@ var server, _ = httpserver.NewDefaultHttpServerFactory().Create()
5757
var server, _ = httpserver.NewDefaultHttpServerFactory().Create(
5858
httpserver.WithDebug(false), // debug disabled by default
5959
httpserver.WithBanner(false), // banner disabled by default
60-
httpserver.WithRecovery(true), // panic recovery middleware enabled by default
6160
httpserver.WithLogger(log.New("default")), // echo default logger
6261
httpserver.WithBinder(&echo.DefaultBinder{}), // echo default binder
6362
httpserver.WithJsonSerializer(&echo.DefaultJSONSerializer{}), // echo default json serializer

‎httpserver/error.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,10 @@ func JsonErrorHandler(obfuscate bool, stack bool) echo.HTTPErrorHandler {
3636
var logRespFields map[string]interface{}
3737

3838
if stack {
39-
errStack := errors.New(err).ErrorStack()
39+
errStack := "n/a"
40+
if err != nil {
41+
errStack = errors.New(err).ErrorStack()
42+
}
4043

4144
switch m := httpError.Message.(type) {
4245
case error:

‎httpserver/factory.go‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package httpserver
22

33
import (
44
"github.com/labstack/echo/v4"
5-
"github.com/labstack/echo/v4/middleware"
65
)
76

87
// HttpServerFactory is the interface for [echo.Echo] factories.
@@ -28,7 +27,6 @@ func NewDefaultHttpServerFactory() HttpServerFactory {
2827
// var server, _ = httpserver.NewDefaultHttpServerFactory().Create(
2928
// httpserver.WithDebug(false), // debug disabled by default
3029
// httpserver.WithBanner(false), // banner disabled by default
31-
// httpserver.WithRecovery(true), // panic recovery middleware enabled by default
3230
// httpserver.WithLogger(log.New("default")), // echo default logger
3331
// httpserver.WithBinder(&echo.DefaultBinder{}), // echo default binder
3432
// httpserver.WithJsonSerializer(&echo.DefaultJSONSerializer{}), // echo default json serializer
@@ -57,9 +55,5 @@ func (f *DefaultHttpServerFactory) Create(options ...HttpServerOption) (*echo.Ec
5755
httpServer.Renderer = appliedOpts.Renderer
5856
}
5957

60-
if appliedOpts.Recovery {
61-
httpServer.Use(middleware.Recover())
62-
}
63-
6458
return httpServer, nil
6559
}

‎httpserver/factory_test.go‎

Lines changed: 0 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@ func TestCreate(t *testing.T) {
5252
httpServer, err := httpserver.NewDefaultHttpServerFactory().Create(
5353
httpserver.WithDebug(true),
5454
httpserver.WithBanner(true),
55-
httpserver.WithRecovery(true),
5655
httpserver.WithLogger(echoLogger),
5756
httpserver.WithBinder(binder),
5857
httpserver.WithJsonSerializer(jsonSerializer),
@@ -2077,41 +2076,3 @@ func TestCreateWithRequestMetricsAndWithoutNormalization(t *testing.T) {
20772076
)
20782077
assert.NoError(t, err)
20792078
}
2080-
2081-
func TestCreateWithPanicRecovery(t *testing.T) {
2082-
t.Parallel()
2083-
2084-
logBuffer := logtest.NewDefaultTestLogBuffer()
2085-
logger, err := log.NewDefaultLoggerFactory().Create(
2086-
log.WithOutputWriter(logBuffer),
2087-
)
2088-
assert.NoError(t, err)
2089-
2090-
httpServer, err := httpserver.NewDefaultHttpServerFactory().Create(
2091-
httpserver.WithLogger(httpserver.NewEchoLogger(logger)),
2092-
httpserver.WithRecovery(true),
2093-
)
2094-
assert.NoError(t, err)
2095-
assert.IsType(t, &echo.Echo{}, httpServer)
2096-
2097-
httpServer.Use(middleware.RequestLoggerMiddleware())
2098-
2099-
httpServer.GET("/panic", func(c echo.Context) error {
2100-
panic("custom panic")
2101-
})
2102-
2103-
defer func() {
2104-
if r := recover(); r != nil {
2105-
t.Error("should have recovered by itself")
2106-
}
2107-
}()
2108-
2109-
for i := 0; i <= 5; i++ {
2110-
req := httptest.NewRequest(http.MethodGet, "/panic", nil)
2111-
rec := httptest.NewRecorder()
2112-
httpServer.ServeHTTP(rec, req)
2113-
2114-
assert.Equal(t, http.StatusInternalServerError, rec.Code)
2115-
assert.Contains(t, rec.Body.String(), `{"message":"Internal Server Error"}`)
2116-
}
2117-
}

‎httpserver/logger.go‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -171,17 +171,17 @@ func (e *EchoLogger) Panicj(j echologger.JSON) {
171171

172172
// Print produces a log with no level.
173173
func (e *EchoLogger) Print(i ...interface{}) {
174-
e.logger.WithLevel(zerolog.NoLevel).Str("level", "-").Msg(fmt.Sprint(i...))
174+
e.logger.WithLevel(zerolog.NoLevel).Str("level", "---").Msg(fmt.Sprint(i...))
175175
}
176176

177177
// Printf produces a formatted log with no level.
178178
func (e *EchoLogger) Printf(format string, i ...interface{}) {
179-
e.logger.WithLevel(zerolog.NoLevel).Str("level", "-").Msgf(format, i...)
179+
e.logger.WithLevel(zerolog.NoLevel).Str("level", "---").Msgf(format, i...)
180180
}
181181

182182
// Printj produces a json log with no level.
183183
func (e *EchoLogger) Printj(j echologger.JSON) {
184-
e.logJSON(e.logger.WithLevel(zerolog.NoLevel).Str("level", "-"), j)
184+
e.logJSON(e.logger.WithLevel(zerolog.NoLevel).Str("level", "---"), j)
185185
}
186186

187187
func (e *EchoLogger) logJSON(event *zerolog.Event, j echologger.JSON) {
@@ -192,29 +192,29 @@ func (e *EchoLogger) logJSON(event *zerolog.Event, j echologger.JSON) {
192192
event.Msg("")
193193
}
194194

195-
//nolint:exhaustive
196195
func convertZeroLevel(level zerolog.Level) echologger.Lvl {
197196
switch level {
198-
case zerolog.TraceLevel:
199-
return echologger.DEBUG
200-
case zerolog.DebugLevel:
197+
case zerolog.TraceLevel, zerolog.DebugLevel:
201198
return echologger.DEBUG
199+
case zerolog.InfoLevel:
200+
return echologger.INFO
202201
case zerolog.WarnLevel:
203202
return echologger.WARN
204-
case zerolog.ErrorLevel:
203+
case zerolog.ErrorLevel, zerolog.FatalLevel, zerolog.PanicLevel:
205204
return echologger.ERROR
206-
case zerolog.NoLevel:
205+
case zerolog.NoLevel, zerolog.Disabled:
207206
return echologger.OFF
208207
default:
209208
return echologger.INFO
210209
}
211210
}
212211

213-
//nolint:exhaustive
214212
func convertEchoLevel(level echologger.Lvl) zerolog.Level {
215213
switch level {
216214
case echologger.DEBUG:
217215
return zerolog.DebugLevel
216+
case echologger.INFO:
217+
return zerolog.InfoLevel
218218
case echologger.WARN:
219219
return zerolog.WarnLevel
220220
case echologger.ERROR:

‎httpserver/logger_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -241,19 +241,19 @@ func TestPrintLogging(t *testing.T) {
241241

242242
echoLogger.Print("test regular message")
243243
logtest.AssertHasLogRecord(t, buffer, map[string]interface{}{
244-
"level": "-",
244+
"level": "---",
245245
"message": "test regular message",
246246
})
247247

248248
echoLogger.Printf("test placeholder message: %s", "placeholder")
249249
logtest.AssertHasLogRecord(t, buffer, map[string]interface{}{
250-
"level": "-",
250+
"level": "---",
251251
"message": "test placeholder message: placeholder",
252252
})
253253

254254
echoLogger.Printj(echologger.JSON{"message": "test json message"})
255255
logtest.AssertHasLogRecord(t, buffer, map[string]interface{}{
256-
"level": "-",
256+
"level": "---",
257257
"message": "test json message",
258258
})
259259
}

‎httpserver/option.go‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99
type Options struct {
1010
Debug bool
1111
Banner bool
12-
Recovery bool
1312
Logger echo.Logger
1413
Binder echo.Binder
1514
JsonSerializer echo.JSONSerializer
@@ -22,7 +21,6 @@ func DefaultHttpServerOptions() Options {
2221
return Options{
2322
Debug: false,
2423
Banner: false,
25-
Recovery: true,
2624
Logger: log.New("default"),
2725
Binder: &echo.DefaultBinder{},
2826
JsonSerializer: &echo.DefaultJSONSerializer{},
@@ -48,13 +46,6 @@ func WithBanner(b bool) HttpServerOption {
4846
}
4947
}
5048

51-
// WithRecovery is used to activate the server automatic panic recovery.
52-
func WithRecovery(r bool) HttpServerOption {
53-
return func(o *Options) {
54-
o.Recovery = r
55-
}
56-
}
57-
5849
// WithLogger is used to specify a [echo.Logger] to be used by the server.
5950
func WithLogger(l echo.Logger) HttpServerOption {
6051
return func(o *Options) {

‎httpserver/option_test.go‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -27,15 +27,6 @@ func TestWithBanner(t *testing.T) {
2727
assert.Equal(t, true, opt.Banner)
2828
}
2929

30-
func TestWithRecovery(t *testing.T) {
31-
t.Parallel()
32-
33-
opt := httpserver.DefaultHttpServerOptions()
34-
httpserver.WithRecovery(false)(&opt)
35-
36-
assert.Equal(t, false, opt.Recovery)
37-
}
38-
3930
func TestWithLogger(t *testing.T) {
4031
t.Parallel()
4132

0 commit comments

Comments
 (0)