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

ResponseEntity<StreamingResponseBody> Exception Handling Issues #25490

Closed
austinarbor opened this issue Jul 29, 2020 · 5 comments
Closed

ResponseEntity<StreamingResponseBody> Exception Handling Issues #25490

austinarbor opened this issue Jul 29, 2020 · 5 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: bug A general bug
Milestone

Comments

@austinarbor
Copy link

austinarbor commented Jul 29, 2020

Summary

  • When using @ExceptionHandler (or @ControllerAdvice), exceptions thrown while writing to a StreamingResponseBody are not handled correctly (or at least as I expect them to be...) when the request mapping returns ResponseEntity<StreamingResponseBody>
  • Exception handling does work as expected when the return type is just StreamingResponseBody
  • It's possible my understanding of how these should work is incorrect, however I haven't seen any evidence yet to point to this

Description

  • If a runtime exception occurs while writing to the StreamingResponseBody wrapped in a ResponseEntity the endpoint does invoke the @ExceptionHandler but returns a 500 status with no response body no matter what
  • If the request mapping returns a StreamingResponseBody directly, error handling does work as expected, and the response code and response body are correct
  • While writing tests for this, I can't get the tests to return anything other than 200 status. While running tests, I also don't see the exception handlers getting invoked. I'm not sure if this is another bug or if my test setup is incorrect. If you hit the endpoints directly in the browser or via CURL, you do see the 500 status with no body. This is my first time using StreamingResponseBody so I apologize if I'm just doing things incorrectly
  • Sample project to reproduce: https://github.com/austinarbor/spring-boot-streaming-response-issue

Misc

@bclozel bclozel transferred this issue from spring-projects/spring-boot Jul 29, 2020
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jul 29, 2020
@vforchi
Copy link

vforchi commented Nov 25, 2020

I have a similar issue: the application works as expected, no matter if I wrap the StreamingResponseBody in a ResponseEntity or not, but in the tests I always get a 200 and an empty body.
I tried with @WebMvcTest+mockMvc and with @SpringBootTest+TestRestTemplate: same behaviour.

@rstoyanchev
Copy link
Contributor

The methods that return ResponseEntity set the content type to "text/csv" so the exception handler later fails to write the ErrorDetails with that content type. I see:

2020-12-01 21:18:59.916  INFO 331037 --- [  XNIO-1 task-1] dev.aga.controller.SomeController        : Inside handleIllegalArgumentException
2020-12-01 21:19:00.788  WARN 331037 --- [  XNIO-1 task-1] .m.m.a.ExceptionHandlerExceptionResolver : Failure in @ExceptionHandler dev.aga.controller.SomeController#handleIllegalArgumentException(IllegalArgumentException, HttpServletRequest, HttpServletResponse)

org.springframework.http.converter.HttpMessageNotWritableException: No converter for [class dev.aga.model.ErrorDetails] with preset Content-Type 'text/csv'

You'll need to also set the content type on the ResponseEntity of the error handler method. However I found out that doesn't work because the ServletServerHttpResponse wrapper does not propagate header changes to the underlying response until it writes the body, and yet it looks up values in it. We'll need to fix that.

In the mean time you can work around this by setting the content type directly on the response:

@ExceptionHandler(IllegalArgumentException.class)
ResponseEntity<ErrorDetails> handleIllegalArgumentException(IllegalArgumentException e, HttpServletRequest request, HttpServletResponse response) {
    log.info("Inside handleIllegalArgumentException");
    var errorDetails = new ErrorDetails(request.getRequestURI(), e.getMessage());
    response.setContentType("application/json");
    return ResponseEntity.status(HttpStatus.BAD_GATEWAY)
            .contentType(MediaType.APPLICATION_JSON)
            .body(errorDetails);
}

@rstoyanchev rstoyanchev self-assigned this Dec 1, 2020
@rstoyanchev rstoyanchev added in: web Issues in web modules (web, webmvc, webflux, websocket) type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Dec 1, 2020
@rstoyanchev rstoyanchev added this to the 5.3.2 milestone Dec 1, 2020
@rstoyanchev
Copy link
Contributor

As for the tests, MockMvc doesn't perform async dispatches, so all you see is the initial handling up until the point the async result is produced. You can perform the async dispatches manually. This is described in the docs. However I would also recommend using the WebTestClient support with MockMvc that's new 5.3

@austinarbor
Copy link
Author

austinarbor commented Dec 2, 2020

Hi @rstoyanchev thanks for you response. Based on your info, I think these are the changes required to make everything work: https://github.com/austinarbor/spring-boot-streaming-response-issue/pull/1/files

@rstoyanchev
Copy link
Contributor

@austinarbor yes that would be the workaround for the @ExceptionHandler method. Also the test updates look about right from a quick look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: bug A general bug
Projects
None yet
Development

No branches or pull requests

4 participants