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 should format a multiline String #30965

Closed
dsyer opened this issue Jul 30, 2023 · 7 comments
Closed

SseEmitter should format a multiline String #30965

dsyer opened this issue Jul 30, 2023 · 7 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Milestone

Comments

@dsyer
Copy link
Member

dsyer commented Jul 30, 2023

The ServerSentEventHttpMessageWriter (used in Webflux but not MVC I think) has some logic to deal with multiline string data (e.g. when a @Controller method returns a Flux<String>):

if (data instanceof String text) {
  text = StringUtils.replace(text, "\n", "\ndata:");
  result = Flux.just(encodeText(sb + text + "\n\n", mediaType, factory));
}

which produces correct output if the input text has multiple lines. In MVC you have to write an SseEmitter and send data to it, e.g.

emitter.send(text, MediaType.TEXT_PLAIN);

but the emitter doesn't check for multiline text and you don't get the data: prefix after the first line. I can work around it by using text.replace("\n", "\ndata:") but it seems like I should expect Spring to do that for me, like it does in WebFlux.

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

rstoyanchev commented Jul 31, 2023

SseEmitter does add the "data: " prefix, and also has the logic for the extra newline (one after each event line, one on build()). It's not clear what the issue is. A sample would be great.

@rstoyanchev rstoyanchev added status: waiting-for-feedback We need additional information before we can continue in: web Issues in web modules (web, webmvc, webflux, websocket) labels Jul 31, 2023
@dsyer
Copy link
Member Author

dsyer commented Jul 31, 2023

It adds the prefix, but only at the start of the entry - if the entry itself has multiple lines then it doesn't put the prefix at the start of every line (like you get for free in WebFlux IIUC). You can see what I mean in this sample: https://github.com/wimdeblauwe/blog-example-code/blob/master/htmx-sse/src/main/java/com/wimdeblauwe/examples/htmxsse/PdfGenerationController.java#L61 (he has to escape the new lines with \ so that the data is a single line).

@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 Jul 31, 2023
@rstoyanchev
Copy link
Contributor

We have tests for this. I will need a sample or something that demonstrates the issue.

@rstoyanchev
Copy link
Contributor

As for the PdfGenerationController, I think what you're pointing out seems to be about new lines within the data (XML, JSON) itself? I don't believe we support with WebFlux either. Again a sample would be best to proceed.

@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 Aug 2, 2023
@dsyer
Copy link
Member Author

dsyer commented Aug 2, 2023

I don't think any of those tests uses a multiline data. One of us is missing something. Here's a tiny sample: https://github.com/scratches/sse-sample (and a "webflux" branch that shows you don't need the workaround).

@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 Aug 2, 2023
@wimdeblauwe
Copy link

@rstoyanchev Did you have time to check out the example of Dave? Any other information you need?

@rstoyanchev rstoyanchev self-assigned this Sep 20, 2023
@rstoyanchev
Copy link
Contributor

@wimdeblauwe nothing further needed. Thanks for the sample @dsyer, I see what you mean now.

@rstoyanchev rstoyanchev added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on status: feedback-provided Feedback has been provided labels Sep 20, 2023
@rstoyanchev rstoyanchev added this to the 6.1.0-RC1 milestone Sep 20, 2023
@rstoyanchev rstoyanchev changed the title Server Sent Events multline text data comes out differently in MVC and WebFlux SseEmitter should format a multiline String Sep 20, 2023
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