feat: support for Server-Sent Events (SSE) #2776
Conversation
| EXPERIMENTAL_LIVE_SSE_QUERY_PARAM, | ||
| } from './constants' | ||
| import { | ||
| EventSourceMessage, |
There was a problem hiding this comment.
Let's not load this node polyfill in the browser. It can make behavior inconsistent and it's extra code. We can detect when we're in node and conditionally load the polyfill.
There was a problem hiding this comment.
This is not a node polyfill. This is a library that implements SSE via standard fetch rather than via the browser's EventSource API which is very limited. We always use this library (in the browser, on node, etc.)
There was a problem hiding this comment.
Hmm yeah ok. So looking closer this is a good thing as all the fetch options still work when we shift from http to SSE.
Have we looked at how SSE will work in a proxy?
There was a problem hiding this comment.
@KyleAMathews I don't think anyone has looked into how SSE will work with a proxy. It will likely be more complicated than the current proxy pattern but it should be feasible to accept incoming SSE requests, then use fetch-event-source to proxy it through, and then in the onopen, onmessage, and onclose handlers forward those SSE events to the user.
There was a problem hiding this comment.
If the proxy is just passing the headers and streaming the response through then it should just work™️. I.e.: the fact that the connection stays open and the response streams through in parts should be transparent to most proxies?
There was a problem hiding this comment.
Yeah I did some research and there's good support. And given this will always be an optional "fast" mode — electric will always work in basic http long-polling.
| * The LSN is only present in the up-to-date control message when in SSE mode. | ||
| * If we are not in SSE mode this function will return undefined. | ||
| */ | ||
| export function getOffset(message: ControlMessage): Offset | undefined { |
There was a problem hiding this comment.
I'm not sure we should be using the LSN to compute an offset in the client. This is an internal server side implementation detail that is quite likely to change.
Would it be better to include the offset on each message from the server when in sse mode?
There was a problem hiding this comment.
That would indeed be better. However, this is what @thruflo documented the client should do in #2544 (comment) and you also did this in your original implementation: #2546. Do you want me to change the server such that the server returns an electric-offset in the body rather than the current global_last_seen_lsn ? (cc @samwillis @thruflo)
There was a problem hiding this comment.
Would this require additional deserialisation / serialisation? As in, to special case it for the SSE? Where are we on providing this info generally -- perhaps we can push the change out of scope and revisit the SSE implementation when we firm up how we expose the offset generally?
|
Is there a timeline or plan for releasing this functionality, even if behind a flag? |
|
@kevin-dp I suspect this was supposed to be merged into |
|
Oh I thought this was released — @kevin-dp what's the next step here? |
|
That's odd. Let's re-open @samwillis' PR in which we merged this one and then merge that one into main? @KyleAMathews |
|
👍 yeah sounds good |
|
@wadefletch I merged SSE support into main last week, cf. #2856 |
This is a follow up PR on #2546 and #2544. It solves a bug related with 409s (must refetch) in SSE mode and it replaces the EventSource browser API by the fetch-event-source library. I refactored the
ShapeStream.#startmethod which was becoming very big and complex. To this end, i split the logic into helper methods that handle the different parts that need to happen (building the shape URL, making the request, parsing the response headers, handling the response body, etc.).I had to patch the fetch-event-source library because it relies on browser-specific features such as
documentandwindow(cf. Azure/fetch-event-source#41). But we want our client to also work in server-side JS environments.I also had to patch the
fetch-event-sourcelibrary because it does not abort the fetch when you pass an already aborted signal. A complete description of the bug and the fix can be found here: Azure/fetch-event-source#98.To use, add
experimentalLiveSSEto your shape options e.g.