Skip to content
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.

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

Spring-Boot: Missing logs on HTTP 500 #488

Closed
mindhaq opened this issue Apr 23, 2019 · 6 comments · Fixed by #490
Closed

Spring-Boot: Missing logs on HTTP 500 #488

mindhaq opened this issue Apr 23, 2019 · 6 comments · Fixed by #490

Comments

@mindhaq
Copy link

mindhaq commented Apr 23, 2019

When a controller throws an Exception, not all is logged as expected.

Description

When a spring boot controller throws an Exception, resulting in a HTTP 500 which returns (in case of a RestController) details about the error, Logbook logging is not what I would expect.

I'm talking about cases where DispatcherServlet is logging the error like this:

Servlet.service() for servlet [dispatcherServlet] in context with path [] threw exception [Request processing failed

Expected Behavior

Basic configuration: log request and response

With status strategy as below: log request and response

logbook:
  strategy: status-at-least

Actual Behavior

Basic configuration: only request gets logged

With status strategy as above: neither request nor response are logged

Steps to Reproduce

  1. Spring-boot app with controller throwing an Exception
  2. use default configuration for logbook, except for the case with the strategy, then change as above
  3. curl the URL

Context

I want to log internal server errors including the request and response data.

Your Environment

  • Version used: 2.0.0-RC.0
  • Spring-Boot 2.0.8
@whiskeysierra
Copy link
Collaborator

Are you using ERROR dispatch? How do you produce the response in case of exceptions? Are you using @ExceptionHandler?

mindhaq added a commit to mindhaq/logbook-logging-demo that referenced this issue Apr 24, 2019
@mindhaq
Copy link
Author

mindhaq commented Apr 24, 2019

I created a minimal application showing the behaviour

https://github.com/mindhaq/logbook-logging-demo

No @ExceptionHandler, no special dispatch, just a RuntimeException in a @RestController.

@whiskeysierra
Copy link
Collaborator

If you don't handle the exception at all then it will become an error dispatch, i.e. filters are executed a second time with the DispatcherType.ERROR trying to delegate to an error page. By default you don't get any from Spring, iirc.

Error dispatch is a tricky beast to get right, especially with Logbook. See #32, #155 and #334. Past experiences can be best summarized as Don't use ERROR dispatch.

Instead I'd suggest to use custom @ExceptionHandlers or something like https://github.com/zalando/problem-spring-web.

@mindhaq
Copy link
Author

mindhaq commented Apr 24, 2019

Ok, so having an @ExceptionHandler returning HTTP 500 and maybe some error JSON (similar to problem or custom) would help with logging?

Maybe that should be part of the documentation for the spring-boot integration, it's not so obvious otherwise.

@whiskeysierra
Copy link
Collaborator

Ok, so having an @ExceptionHandler returning HTTP 500 and maybe some error JSON (similar to problem or custom) would help with logging?

Yes. But technically it's not so much about how you produce error responses but rather that the ERROR dispatch is not supported.

Maybe that should be part of the documentation for the spring-boot integration, it's not so obvious otherwise.

Yes, it should.

@whiskeysierra whiskeysierra pinned this issue Apr 24, 2019
whiskeysierra added a commit that referenced this issue Apr 24, 2019
Fixes #488
Fixes #489
@whiskeysierra whiskeysierra mentioned this issue Apr 24, 2019
6 tasks
@mindhaq
Copy link
Author

mindhaq commented Apr 24, 2019

Ok, got it. Adding an @ExceptionHandler prevents the ERROR dispatch, and now logging happens as expected. See the following commit to my demo project:

mindhaq/logbook-logging-demo@dd14e81

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants