search

spring-projects / spring-framework

Spring Framework
https://spring.io/projects/spring-framework
Apache License 2.0
56.32k stars 38.01k forks source link

onCompletion SseEmitter callback never gets called [SPR-13292] #17882

Closed spring-projects-issues closed 9 years ago

spring-projects-issues commented 9 years ago

Sergi Almar opened SPR-13292 and commented

onCompletion() is supposed to be called when the async request is complete, but it never gets called. This makes detecting disconnects impossible, dealing to exceptions being thrown when sending new events as the stream is already closed.

Happens on Tomcat, Jetty and Undertow.

Affects: 4.2 RC2

Referenced from: commits https://github.com/spring-projects/spring-framework/commit/27cd87926ac57208996bf30d2e6c57a23bc626e7

0 votes, 5 watchers

spring-projects-issues commented 9 years ago

Rossen Stoyanchev commented

SseEmitter.onCompletion is called when the async request completes from the server side, i.e. following emitter.complete(). The Servlet API has no notification when the connection is closed client-side SERVLET_SPEC-44. The only way to find out is following a write failure.

In ResponseBodyEmitter we do catch all write failures and set DeferredResult.setErrorResult which dispatches back into the container where the exception is then handled with the configured HandlerExceptionResolver's. If the exception remains unresolved however (e.g. Jetty EofException) the exception bubbles up to the container and that's a case in which we currently fail to call onCompletion. It's something I'm about to fix.

In short, we'll make sure onCompletion is called reliably. However we cannot change the fact that if the connection is closed client-side, a write has to fail before we'll know about it (feel free to upvote SERVLET_SPEC_44). BTW on the WebSocket side with SockJS fallbacks, the issue is mitigated by heartbeats which ensure that a write is attempted regularly and therefore closed connections are discovered soon.

spring-projects-issues commented 9 years ago

Sergi Almar commented

Thanks for the explanation Rossen, was aware it was related to spec from the conversations with Sébastien at Spring I/O, but was wondering if there was any possible workaround in the meantime. Seen the commit now, will give it a try.