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

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

Closed
incapable opened this issue Jan 31, 2020 · 8 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Milestone

Comments

@incapable
Copy link

incapable commented Jan 31, 2020

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

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jan 31, 2020
@rstoyanchev
Copy link
Contributor

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 rstoyanchev added the status: waiting-for-feedback We need additional information before we can continue label Feb 3, 2020
@rstoyanchev rstoyanchev changed the title SseEmitter cannot parse text/plain SseEmitter cannot format text/plain Feb 3, 2020
@rstoyanchev
Copy link
Contributor

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

@incapable
Copy link
Author

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

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Feb 3, 2020
@rstoyanchev
Copy link
Contributor

rstoyanchev commented Feb 3, 2020

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.

@rstoyanchev rstoyanchev added the status: waiting-for-feedback We need additional information before we can continue label Feb 3, 2020
@rstoyanchev rstoyanchev self-assigned this Feb 3, 2020
@rstoyanchev rstoyanchev added in: web Issues in web modules (web, webmvc, webflux, websocket) and removed status: feedback-provided Feedback has been provided labels Feb 3, 2020
@rstoyanchev rstoyanchev changed the title SseEmitter cannot format text/plain SseEmitter cannot format text/plain if StringHttpMessageConverter is not configured Feb 3, 2020
@odrotbohm
Copy link
Member

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?

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Feb 5, 2020
@rstoyanchev rstoyanchev added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Feb 5, 2020
@incapable
Copy link
Author

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.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Feb 5, 2020
@rstoyanchev rstoyanchev added type: enhancement A general enhancement and removed status: feedback-provided Feedback has been provided status: waiting-for-triage An issue we've not yet triaged or decided on labels Feb 5, 2020
@rstoyanchev rstoyanchev added this to the 5.2.4 milestone Feb 5, 2020
@rstoyanchev
Copy link
Contributor

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
Copy link
Author

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

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: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

4 participants