Skip to content

Commit

Permalink
fix: improve error handling to prevent panics
Browse files Browse the repository at this point in the history
In some scenarios, the status code of the response object was set to
200, but the error was non nil. The middleware was logging 200, but echo
was actually responding with 500.

In other scenarios, the status code of the response was set to 500, but
the error was nil. The middleware was checking the status code of the
response and if it was not ok, it tried to log the error message, which
resulted in a panic.

This patch should make error handling more robust.

Signed-off-by: Michal Wasilewski <[email protected]>
  • Loading branch information
mwasilew2 committed Jul 27, 2023
1 parent e6dfc90 commit bf93ed2
Showing 1 changed file with 14 additions and 2 deletions.
16 changes: 14 additions & 2 deletions middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ func NewWithConfig(logger *slog.Logger, config Config) echo.MiddlewareFunc {

err = next(c)

if err != nil {
c.Error(err)
}

requestID := req.Header.Get(echo.HeaderXRequestID)
if requestID == "" {
requestID = res.Header().Get(echo.HeaderXRequestID)
Expand Down Expand Up @@ -91,9 +95,17 @@ func NewWithConfig(logger *slog.Logger, config Config) echo.MiddlewareFunc {

switch {
case status >= http.StatusInternalServerError:
logger.LogAttrs(context.Background(), config.ServerErrorLevel, err.Error(), attributes...)
var errMsg string
if err != nil {
errMsg = err.Error()
}
logger.LogAttrs(context.Background(), config.ServerErrorLevel, errMsg, attributes...)
case status >= http.StatusBadRequest && status < http.StatusInternalServerError:
logger.LogAttrs(context.Background(), config.ClientErrorLevel, err.Error(), attributes...)
var errMsg string
if err != nil {
errMsg = err.Error()
}
logger.LogAttrs(context.Background(), config.ClientErrorLevel, errMsg, attributes...)
case status >= http.StatusMultipleChoices && status < http.StatusBadRequest:
logger.LogAttrs(context.Background(), config.DefaultLevel, "Redirection", attributes...)
default:
Expand Down

0 comments on commit bf93ed2

Please sign in to comment.