Consistent error handling when streaming with servlet vs reactive stack [SPR-17440]

[spring-projects-issues]

Michal Domagala opened SPR-17440 and commented

Consider the following code:

@RequestMapping("/1")
Flux<String> v1()
    return Flux.error(new ResponseStatusException(BAD_REQUEST));

@RequestMapping("/2")
Flux<String> v2()
    return Flux.concat(Flux.just("x"),Flux.error(new ResponseStatusException(BAD_REQUEST)));

 
In reactive stack initial exception has special treatment: class ChannelSendOperator has special state NEW: /** No emissions from the upstream source yet. */

Due special treatment http response status for /1 is 400 and for /2 is 200
In servlet stack http response status for both endpoints is 200

Nice to have consistent implementation. Reactive stack way is correct one.

---

Affects: 5.0.10

Reference URL: https://stackoverflow.com/questions/53013088/how-to-peep-if-flux-has-exception

[spring-projects-issues]

Rossen Stoyanchev commented

Indeed the asynchronous, streaming support in Spring MVC (e.g. SseEmitter, ResponseBodyEmitter) was created first and it's always behaved as it does now, i.e. flushing the status and headers before any items are emitted. The solution in WebFlux went further, and it had to, because async is the norm there and it's the only way for @ExceptionHandler to be supported. We should try and make the two consistent.

[spring-projects-issues]

vrawat79 commented

Actually, since we are returning a Flux, i.e. a series of events, wherein each event could have its own status, shouldn't we be returning a HTTP 207 as the overall response code.

400 as the overall status code will be mis-leading. 

 

[spring-projects-issues]

Rossen Stoyanchev commented

The ResponseBodyEmitter (and SseEmitter) were designed for a stream of events (like SSE) and that's very different. The SSE spec has no reference to HTTP status 207 and I've never seen that status used for such purposes. We're only talking about the initial status before the stream actually begins emitting. From what I can see HTTP 207 is defined in WebDAV and that's a very specific thing for multiple operations or resources.