-
Notifications
You must be signed in to change notification settings - Fork 16
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
add streaming event data mode (6.0 rewrite, part 2) #94
base: eb/sc-183031/rewrite-with-pull-model
Are you sure you want to change the base?
add streaming event data mode (6.0 rewrite, part 2) #94
Conversation
/// If so, reading <see cref="MessageEvent.DataUtf8Bytes"/> will return the same | ||
/// byte array, to avoid unnecessary copying. | ||
/// </remarks> | ||
public struct Utf8ByteSpan |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is no longer a public type because MessageEvent
no longer has a DataUtf8Bytes
property. It's now in Internal
and is used only as a convenience container for a tuple of (buffer, offset, length).
_taskFactory.StartNew(taskFn) | ||
.Unwrap() | ||
.GetAwaiter() | ||
.GetResult(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is used only to implement the synchronous Read
method in the special stream provided by EventParser. The implementation is borrowed from LaunchDarkly.InternalSdk.
@@ -1,9 +1,11 @@ | |||
using System; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file is now extremely close to the Java version. The differences are mainly due to the .NET async model, and some other slight differences between .NET's Stream type and Java's InputStream type (for instance, in Java you return -1 for EOF, but in .NET you return 0).
This adds the incremental loading feature that was one of the main motivations for the whole rewrite. As before, the implementation is closely based on the Java version in
okhttp-eventsource
. Briefly:StreamEventData(true)
, then when you receive aMessageEvent
it contains a special stream, accessed via theDataStream
property. When you read from this stream, it is really reading from theEventParser
, consuming the value of everydata:
field up till it hits a blank line, at which point that stream returns EOF.DataStream
for the previous event becomes invalid, andEventParser
skips over any content that hasn't yet been consumed from that one.One wrinkle with this, which is a necessary departure from the SSE spec, is that
EventParser
has to return theMessageEvent
before it has seen anything past the firstdata:
line. So at that point, it doesn't yet know for sure that there really will be a full event before the end of the stream— and, if any other fields likeevent:
orid:
appear afterdata:
, it can't put them into theMessageEvent
. This is not an issue for using EventSource in the LaunchDarkly SDKs, because 1. until all of the data is consumed we won't actually be storing any of it, just parsing it; 2. the LD services always sendevent:
beforedata:
; and 3. with the newExpectFields
option, the SDK can specify that it definitely needs to see anevent:
field and so if for some reason the fields were sent out of order, EventSource will just go back to buffering the full event like before.