This commit updates HttpWebHandlerAdapter
and ResponseStatusExceptionHandler
in order to specify the method/uri in the logged message.
It also logs a WARN
message for bad request (400) HTTP responses in order to get some logs when an exception is thrown due to client error (unable to deserialize request body for example).
Issue: SPR-15083
Comment From: rstoyanchev
@sdeleuze, it looks fine but a 5xx would now be logged twice at error level it seems. Perhaps the RSEH can skip logging 5xx errors, knowing that those will be logged in HWHA. It'd be useful to have a comment to that extent in the code to make it clear not logging the error is intentional.
Comment From: sdeleuze
@rstoyanchev I am not sure 5xx would now be logged twice at error level since it seems that HttpWebHandlerAdapter#handleFailure
is only invoked when ResponseStatusExceptionHandler#handle
returns Mono.error(ex)
(for exception that are not instance of ResponseStatusException
), not when it returns exchange.getResponse().setComplete()
(for ResponseStatusException
instances) so I can't see an overlap between both cases.
Also HttpWebHandlerAdapter
and ResponseStatusExceptionHandler
logging are slightly different because ResponseStatusExceptionHandler
only logs the message (with contains the nested cause) and not the whole stacktrace in order to improve readability of logs and for consistency with Boot (which catches exceptions before our ResponseStatusExceptionHandler
but I have replicated the same logic via this PR), so I would be for keeping the proposed arrangement if you are ok with that, if I am correct on the non-duplication.
Any thoughts?
Comment From: rstoyanchev
Yes, you're right. Looks good :)
Comment From: sdeleuze
Merged via 196f3f8cc1aae9f3df06e9d961c62185e8730bfb.