`ServerSentEvent` type does not match the WhatWG specification WRT comments

[arturaz]

The model defined in https://github.com/softwaremill/sttp-model/blob/afd84a6e357c53b79ee7344a890223758285fbc5/core/src/main/scala/sttp/model/sse/ServerSentEvent.scala#L5-L18 does not match the specification at https://html.spec.whatwg.org/multipage/server-sent-events.html#event-stream-interpretation

Specifically, this part:

If the line starts with a U+003A COLON character (:)

Ignore the line.

Therefore there is currently no way to send or receive "comment" messages, which are useful to perform keep-alives.

One could argue that you can just send event: keep-alive here, to which I agree, but just noticed this discrepancy and wondered whether we should add a way to send out these comment messages.

[adamw]

Thanks for the issue - yes we should :) Maybe you could attempt drafting a PR?

[arturaz]

What is the policy regarding binary compatibility? I don't see how to model this without breaking it.

---- On Wed, 22 Jan 2025 17:59:08 +0200 @.*** wrote ----

Thanks for the issue - yes we should :) Maybe you could attempt drafting a PR?

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.

[adamw]

The policy is we need to keep bincompat :) You can check this by running mimaReportBinaryIssues

If you add a new case class parameter, but also add a constructor with the old signature, maybe it might work?

[arturaz]

I don't think we can just a new parameter. If it is a comment, then nothing else is allowed. So it should be modelled as a discriminated union?

Also the parsing side should ignore these comments instead of parsing them?

---- On Wed, 22 Jan 2025 20:08:59 +0200 @.*** wrote ----

The policy is we need to keep bincompat :) You can check this by running mimaReportBinaryIssues

If you add a new case class parameter, but also add a constructor with the old signature, maybe it might work?

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.

[adamw]

Ah, so you can't have an event e.g.:

: comment
data: abc

?

Even then, given the current constraints (that is the definition of ServerSentEvents that we have), we might model this in an imperfect way, but still giving the possibility to send comment-messages at all. That is: add a comment: Option[String] field; add a SSE.comment constructor; if comments can't be combined with data, add an assertion in the constructor that both are not set.