Server Side event Responses from an server behind Spring-clod-gateway-mvc

[AkashB23]

I have a Spring boot mvc server that has SseEmitter as Response, and the server is behind Spring-clod-gateway-mvc gateway which has the Route for the path with handler function being  ProxyExchangeHandlerFunction

The server sent events are consumed properly when server is being called directly without gateway, but once we have gateway to be as reverse proxy, SSE does not work with MVC version of spring-cloud-gateway

we see EOF reached while reading exception

[rworsnop]

This is because of the way Tomcat's OutputBuffer works. That's what is being written to when applications write to a response's OutputStream. As its name suggests, it will buffer data before actually sending anything to the client. A good idea for downloading a file, but does not work with SSE. The solution is to call OutputStream.flush after every single write. That's not what SCG does, as it's using InputStream.transferTo via StreamUtils.copy. Ideally SCG would offer a way of specifying whether or not a route should flush after every write. Maybe a variant of HandlerFunctions.http. Failing that, you'll probably have to provide your own handler to use instead of HandlerFunctions.http.

[jensmatw]

I've written such a Handler but had to copy several classes and using named beans because many spring classes are final, everything is static or private so I can't just use inheritance or write a wrapper around these. Theres a long chain of invocations where it is hard to just offer a "second implementation to chose from", eg. theres the static HandlerFunction which instanciates the nonpublic LookupProxyExchangeHandlerFunction which resolves a ProxyExchangeHandlerFunction which privatly owns the ProxyExchange which needs to have the implementation.

@rworsnop Is there a reason why SCG would need both cases, a flushing and a nonflushing variant? I could offer a PR to add flushing behaviour just in the original RestClientProxyExchange, do you see any disadvantage if in worst case where there is no eventing, just lots of data, every BUFFER_SIZE bytes (eg, 16k) there is an additional flush? Maybe it has some performance impact for writing long responses.

If you can point me in the right direction to write a second handlerfunction (including registration) I could try to create a PR with that.

[rworsnop]

~~IMO SCG shouldn't need to buffer data it gets back from an origin server in any situation. The origin server itself should have already done that. Perhaps @spencergibb will disagree.~~

How about looking at Content-Type and flushing after every write when it is set to text/event-stream?

[jensmatw]

@rworsnop this would solve it for me, should be easy enough to just pass the Content-Type header to the copy method (Request object is there).

But it would only solve it for SSE, but not for other event types like the classic long-polling where you usually just have application/json as Content-Type.

I have the feeling it would solve more cases if always a flush is applied after a buffer-write or if you are able to chose the variant you want to use.

Since it would be my first commit in Spring: Is it ok to just create a PR here with the former and then we can discuss it along with the code?

[rworsnop]

Just to be clear: I am not on the Spring team! I'm just offering my opinions.

I would focus on SSE to start with, since it always makes sense to flush for text/event-stream. I suspect it would be easier to get a PR approved if it changes behavior only for SSE rather than for every request.

I also think it would be good if SSE worked "out of the box" without developers needing to troubleshoot and figure out that they need to set a flag—something that could be achieved with the Content-Type solution.

IMO the ability to choose a variant should be added as well as the above, not instead of it—perhaps in a different issue/PR. But I'm not really sure if there's that much interest in long polling these days.

[jensmatw]

I've added the content-type check to my code that I use for some weeks now and put it into the PR. @spencergibb what do you think?

[jensmatw]

Sorry, the PR is not working, @rworsnop we don't know the content-type, since it is set in the response, which is not avaiable in this place. :(

[jensmatw]

I have added my solution without the Content-Type up for discussion, not sure if it hits performance for big files because of too many flushes?

[rworsnop]

@jensmatw Your PR isn't working because you're looking at the server request headers. You need to look at the headers in clientResponse available in doExchange.

[jensmatw]

@jensmatw Your PR isn't working because you're looking at the server request headers. You need to look at the headers in clientResponse available in doExchange.

Oh I was confused about the copy of request body and the copy of response body, I'll try to update.

[jensmatw]

Added a new PR with Content-Type check.

[jensmatw]

I'll leave both PRs here for reference, @spencergibb maybe you can just close one of them and we continue discussing on the better one.

[rworsnop]

This is the least intrusive workaround I could think of. It does require that clients set the Accept header to text/event-stream.

public class SseFilter extends OncePerRequestFilter {
    @Override
    protected void doFilterInternal(
            HttpServletRequest request,
            HttpServletResponse response,
            FilterChain chain) throws ServletException, IOException {
        if (TEXT_EVENT_STREAM_VALUE.equals(request.getHeader(ACCEPT))) {
            chain.doFilter(request, new SseResponseWrapper(response));
        } else {
            chain.doFilter(request, response);
        }
    }

    static class SseResponseWrapper extends HttpServletResponseWrapper {

        public SseResponseWrapper(HttpServletResponse response) {
            super(response);
        }

        @Override
        public ServletOutputStream getOutputStream() throws IOException {
            ServletOutputStream sos = super.getOutputStream();
            return new ServletOutputStream() {
                @Override
                public boolean isReady() {
                    return sos.isReady();
                }

                @Override
                public void setWriteListener(WriteListener listener) {
                    sos.setWriteListener(listener);
                }

                @Override
                public void write(int b) throws IOException {
                    sos.write(b);
                    flush();
                }

                @Override
                public void write(@NotNull byte[] b, int off, int len) throws IOException {
                    sos.write(b, off, len);
                    flush();
                }

                @Override
                public void flush() throws IOException {
                    sos.flush();
                }

                @Override
                public void close() throws IOException {
                    sos.close();
                }
            };
        }

    }
}

[jensmatw]

Nice idea, I did the complex approach with copying multiple classes from spring (since they are final, methods private or everthing static) and use a custom HandlerFunctions.http that uses them.

[AkashB23]

@spencergibb @jensmatw this fix would not work in case of the underlying client being the reactor neety client due to the blocking implementation done at wrapper for the netty response in spring here ReactorClientHttpRequest.java#L109

[spencergibb]

@AkashB23 that client is not used in the webmvc server so it is unrelated