Add support for SSE comments - #478
Conversation
`ServerSentEvent` now carries a `comments` field, so that comment lines (those starting with `:`) can be both parsed and serialised. Per the WhatWG specification such lines are ignored by clients, which makes them the idiomatic keep-alive: they stop proxies from dropping an idle connection without dispatching an event to the application. Binary compatibility with 1.7.18 is preserved in the same way as for `ContentTypeRange`: the old-arity constructor, `copy` and `apply` are kept alongside the new ones. Closes #379 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
| ): ServerSentEvent = ServerSentEvent(data, eventType, id, retry, this.comments) | ||
|
|
||
| override def toString: String = { | ||
| val _comments: Array[Option[String]] = comments.map(comment => Some(s": $comment")).toArray |
There was a problem hiding this comment.
Comments are not split on \n, unlike data one line below. comment("a\nb").toString emits a bare b line, which is dropped on re-parse. comment("ping\n\n") emits a blank line, which ends the event. Since comment(...) accepts arbitrary text, a \n can also inject other fields (comment("x\ndata: y")).
Suggestion (also drops the Some/Array wrapping):
val _comments = comments.flatMap(_.split("\n")).map(c => Some(s": $c")).toArray| def parse(event: List[String]): ServerSentEvent = { | ||
| event.foldLeft(ServerSentEvent()) { (event, line) => | ||
| if (line.startsWith("data:")) combineData(event, removeLeadingSpace(line.substring(5))) | ||
| if (line.startsWith(":")) event.copy(comments = event.comments :+ removeLeadingSpace(line.substring(1))) |
There was a problem hiding this comment.
Behavior change worth a release note: comment-only blocks used to parse to ServerSentEvent(). Code that skips keep-alives via _ != ServerSentEvent() or case ServerSentEvent(None, None, None, None) will now pass them through.
It would help to add a migration target, e.g.:
/** True if the event carries no data, event type, id or retry - only comments, if any.
* Clients ignore comments, so such events (e.g. keep-alive pings) can usually be skipped.
*/
def isCommentOnly: Boolean = data.isEmpty && eventType.isEmpty && id.isEmpty && retry.isEmptyFollow-up: audit tapir & sttp client for ServerSentEvent() comparisons/patterns and update them once this is released.
| event match { | ||
| case e @ ServerSentEvent(Some(oldData), _, _, _) => e.copy(data = Some(s"$oldData\n$newData")) | ||
| case e @ ServerSentEvent(None, _, _, _) => e.copy(data = Some(newData)) | ||
| case e @ ServerSentEvent(Some(oldData), _, _, _, _) => e.copy(data = Some(s"$oldData\n$newData")) |
There was a problem hiding this comment.
These patterns only needed changing because they depend on the field count. An accessor form avoids the same edit next time a field is added:
event.copy(data = Some(event.data.fold(newData)(old => s"$old\n$newData")))| class ServerSentEventTest extends AnyFlatSpec with Matchers { | ||
| val data = List( | ||
| (List(": this is a test stream"), ServerSentEvent()), | ||
| (List(": this is a test stream"), ServerSentEvent(comments = List("this is a test stream"))), |
There was a problem hiding this comment.
This was the only row reaching the final else in parse (ignore unknown lines). To keep that covered, add e.g. (List("foo: bar", "data: x"), ServerSentEvent(Some("x"))).
| ) | ||
| ), | ||
| ( | ||
| List(": keep-alive", "data: with a comment"), |
There was a problem hiding this comment.
This row is covered by the next one, which also tests a comment after data. Could be dropped.
2 participants
Fixes #379.
ServerSentEventgains acomments: List[String]field, so comment lines (those starting with:) round-trip through bothparseandtoString.ServerSentEvent.comment("ping")builds the keep-alive frame the WhatWG specification recommends sending every 15 seconds or so, to stop legacy proxies from dropping an idle connection.Design notes
List[String]rather thanOption[String], because a single event block may carry several comment lines.There is deliberately no assertion that a comment and data aren't both set (as considered in the issue discussion). The specification ignores comments per line, not per event, so a block such as
is valid and still dispatches
abc. A guard would reject valid streams.Binary compatibility
Preserved with the same approach already used for
ContentTypeRange: the old-arity constructor,copyandapplyare kept alongside the new ones.Note one source-level (not binary) change: downstream positional patterns like
case ServerSentEvent(d, e, i, r)now need a fifth_. This is inherent to adding a field and is not visible to MiMa.