spring-projects / spring-framework

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

SseEmitter cannot format text/plain if StringHttpMessageConverter is not configured #24465

Closed incapable closed 4 years ago

incapable commented 4 years ago

The SseEmitter included in the spring framework does not seem to include a capable text/plain parser in the underlying converter set.

Whenever i try to send any form of event, the emitter automatically includes a "data:" prefix, which it adds with the text/plain mediatype.

After a bit of digging i noticed some of the underlying functions use a subset of spring's default MessageConverters, this subset does not include the default String converter.

So, whenever i try to send any type of event like so:

  SseEmitter.SseEventBuilder event = SseEmitter.event()
                            .data("SSE MVC - " + LocalTime.now().toString())
                            .id(String.valueOf(i))
                            .name("sse event - mvc");
                    emitter.send(event);

it will inevitably result in a java.lang.IllegalArgumentException: No suitable converter for class java.lang.String

The suggestion would be to either include the standard string converter, or let the SseEmitter use a different mediatype for pre- and suffixes that are supported in the Converter chain for the emitter.

I'm currently on spring boot version 2.1.12.RELEASE

rstoyanchev commented 4 years ago

The handling for SseEmitter uses message converters from ResponseBodyEmitterReturnValueHandler which is initialized in RequestMappingHandlerAdapter from the same list of message converters as every other argument resolver.

So it not clear at all what the root cause of your issue is. Please, provide more details or a sample.

rstoyanchev commented 4 years ago

I corrected the title because SseEmitter does no parsing, but rather the opposite.

incapable commented 4 years ago

Sorry, you're right. But, you stated that it should be using all of the message converters, but it simply is not doing that.

Here you see the currently available converters for the ResponseBodyEmitterReturnValueHandler that the SseEmitter uses image

which is added here, in the RequestMappingHandlerAdapter image

And that list is set, in the adapter, over here image

Which is called from the RepositoryRestMvcConfiguration, in the repositoryExporterHandlerAdapter method, from which it get's it's defaultMessageConverter list image https://github.com/spring-projects/spring-data-rest/blob/master/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/config/RepositoryRestMvcConfiguration.java#L651

where only a subselection of the converters are added.

Please note: In my startup chain the repositoryExporterHandlerAdapter is called twice, only one of which comes from the RepositoryRestMvcConfiguration, the other initialization does include all the converters that would be needed. The SseEmitter uses the RepositoryRestMvc list of converters though, and because of that it cannot parse text/plain mediatypes

rstoyanchev commented 4 years ago

Okay this makes is a bit more clear.

RepositoryRestMvcConfiguration is in Spring Data REST and that doesn't seem to include a message converter for text when it configures the repositoryExporterHandlerAdapter bean.

Either Spring Data REST can add StringHttpMessageConverter to the config it uses, or perhaps there is a way that you can configured it into Spring Data REST. @gregturn or @odrotbohm any thoughts?

If this isn't feasible we might be able to add special handling for SSE in case StringHttpMessageConverter isn't available.

odrotbohm commented 4 years ago

Happy to investigate what we can do to help. Just wondering why your controller ends up in the Spring Data REST specific HandlerAdapter? Can you elaborate a bit more on your actual use case?

incapable commented 4 years ago

I'm trying to make it work in an already existing spring data REST application, for outputting updated objects when they are altered. So the data i'd be sending would come from a spring data REST repository, and spring has been initialized to use it.

So, for my test cases, I've tried sending both plaintext data and spring data REST entities, with both results in the same issue.

The SseEmitter will always try to prepend a plaintext prefix, which will always fail because the converter chain lacks a text/plain capable converter.

rstoyanchev commented 4 years ago

I've scheduled this to try an improvement in Spring MVC where we fall back on something for all the plain text in an SSE stream that surrounds the actual data. We're currently assuming there will always be a string message converter but we should be able to write text irrespective of that.

incapable commented 4 years ago

Alright thanks! That seems like the proper solution in my scenario