[Bug] Formatting error when sending multiple lines of data

[endereyewxy]

According to MDN, when sending multiple lines of data via SSE, each line should be preceded by data:, and the current implementation is clearly doing it wrong.

If you try to send multiple lines of data like:

app.get('/live', () => {
    return new Stream(async (stream) => {
        stream.send("first line\nsecond line");
        stream.close();
    });
})

You will get:

id: <some id>
data: first line
second line  // Wrong here

[LucienLeMagicien]

I got bitten by this just yesterday. As you spotted, the issue is in this forest of ternaries: https://github.com/elysiajs/stream/blob/dafd14a3a9344be103e2d6ecbd63406e57123741/src/index.ts#L180-L192 which basically only processes the input as if it was one single line. Browsers seeing this just fail with a nebulous error event.

The lines above that snippet (where it concats the labels when data is an Uint8Array) also have a similar issue, because it treats the data as one single "line": this means that eg. sending a text file from an S3 bucket fails.

I don't understand why this lib encodes and enqueues text into Uint8Arrays, instead of decoding them into text. AFAIK, SSE Streams are supposed to be text, and there is an implicit conversion happening somewhere. Am I missing something?

Since this is adjacent code, I might as well mention that I don't really like the stream.send API. It should allow to set event on a per-message basis instead of having to bake it (+ an id) into the string I'm sending. This was mentioned here: https://github.com/elysiajs/stream/issues/1#issuecomment-1781516615_ :

It does, however, feel a bit hacky, so it would be great if the event name could be passed in as a parameter to the send method.

This whole Stream class feels a bit Frankenstein-y to me. The way label is used and send is written, it means that:

• If you construct your Stream with an event option, then your stream is SSE even if it consumes an Iterable, a Response or a ReadableStream
• If you don't pass an event option, but you pass it an Iterable, a Response or a ReadableStream whose chunks are strings, it WILL turn your stream into an SSE one by prepending an id: here. (The docs only say that it's an SSE stream if you pass a callback or nothing to the constructor)

I'm not sure how much of it is intentional and I'm missing the point versus how much of it is just code growing organically, but in my opinion it would be better to split the Stream class (the one that consumes Iterables/AsyncIterables/Responses/ReadableStreams) and make it send raw chunks to the ReadableStream, from the SSEStream class which would only handle text and have all the logic for event names / retries / multiline data. No different logic based on what you pass in. This would keep these two concepts of "raw stream" and "SSE protocol stream" separate, and both are simple enough on their own.

@SaltyAom If you're open to it I could draft a PR (but it would be a breaking change). I'm not sure where this project stands WRT contributions and versioning.

[LucienLeMagicien]

By the way, the doc says:

By default, Stream will return Response with content-type of text/event-stream; charset=utf8.

But what actually seems to happen is that it returns a ReadableStream, and Elysia sets the Response's Header as a text/event-stream; charset=utf-8 here: https://github.com/elysiajs/elysia/blob/78671b1827ecb40add10c6a0fab28ce4187f28ef/src/handler.ts#L169-L181

Not sure what to make of that; it feels like this plugin's goals are pretty ill-defined. Is this only geared toward streaming text (OpenAI API), only SSE, a mix of both, or are there use-cases where you'd want to stream binary data (that aren't Bun.Files, since Bun/Elysia streams those automagically?)