dotnet-eventsource
dotnet-eventsource copied to clipboard
LaunchDarkly.EventSource 6.0 rewrite, part 1
This is a major rethink of this fairly old code, closely based on the v4.0.0 redesign of our Java SSE client okhttp-eventsource. It has the following goals:
- Give the caller more control over when and where things are executed, by reversing the overall design from a push model (SSE client runs a worker task which calls event handlers) to a pull model (caller explicitly requests events one at a time, but can run a separate worker task for this if they want to).
- Properly support asynchronous event handling. The previous version used async I/O, but when it called event handlers, they were regular synchronous void functions.
- Make it more feasible to implement a "read data for a large event incrementally" mode, which what
okhttp-eventsourcecallsstreamEventData. This PR does not implement that feature (that's why it is "part 1"), but it's an important first step toward it, because (as I found when trying to implement it in an earlier version ofokhttp-eventsource) the feature doesn't play well with the choice of a push model. - Improve separation of concerns in a way that makes the implementation cleaner, more flexible, and easier to test. The retry delay logic now uses a
RetryDelayStrategyabstraction, where the default implementation is our usual backoff/jitter; the logic for obtaining an input stream now uses aConnectStrategyabstraction, where the default implementation is HTTP. There's also anErrorStrategyabstraction which is fairly similar to the previousConnectionErrorHandler, but its behavior is now a bit better defined. - Fix areas of noncompliance with the SSE spec— primarily that we weren't correctly handling CR-only line endings, which was a side effect of relying on .NET's StreamReader class. This means we can now pass our SSE contract tests, so this PR also adds those tests.
Like the Java equivalent, this PR provides a BackgroundEventSource wrapper class that is the equivalent of the old push model, making it relatively straightforward to adapt any code that was using the older version; the main difference is that the event handlers must return Task (which doesn't mean they have to really be async— synchronous logic can be kept by adding return Task.CompletedTask).
p.s. One thing that still exists in this PR, which does not exist in okhttp-eventsource and I think should probably go away, is the idea of being able to specify whether the data field in a MessageEvent is initially stored as a string or a byte array. The rationale originally was that most applications don't use very large events, and strings are more convenient for them, but those that do (like our SDKs) would prefer to parse the data directly from UTF-8 for efficiency. I think that the latter will be obviated by the addition of an incremental-data-reading mode; applications that expect the data to be very big would use that mode, and everyone else would just treat the data as a string.