feat!: AsyncSequence-first public API#106
Open
keelerm84 wants to merge 5 commits into
Open
Conversation
Phase 2 of the Swift 6 modernization (SDK-2607). Replaces the push-based EventHandler callback with an AsyncStream of events that consumers iterate with `for await`. This is the surface the swift-core streaming FDv2Synchronizer (Phase 3) needs; an AsyncStream maps almost 1:1 onto its `next() async`. BREAKING CHANGE: EventHandler is no longer public and EventSource.Config no longer takes a handler. Consume `EventSource.events` instead. Public API: - Add `EventSourceEvent` (opened / closed / message / comment / error). - `EventSource.events` exposes an `AsyncStream<EventSourceEvent>`; `Config.init` drops the `handler` parameter (now just `init(url:)`). - Errors are delivered as `.error` values, not stream terminations: the client keeps retrying with backoff and the stream continues, ending only on stop() (or consumer cancellation). Lifecycle stays explicit via start()/stop(). Internal: - EventHandler becomes internal; EventParser still drives it. A private ContinuationEventHandler forwards the delegate's and parser's callbacks into the stream's continuation. The URLSessionDataDelegate data path is unchanged (no URLSession.bytes, which is unavailable on Linux/Windows Foundation). - The delegate finishes the continuation at terminal shutdown; the consumer cancelling iteration tears the connection down via onTermination. Tests / contract service: - Network tests become async and observe events via a non-blocking poll-based AsyncSink drained from the stream (no thread blocking on the cooperative executor); the suite stays .serialized for the global URLProtocol state. The parser tests keep the synchronous EventSink. Buffering is .unbounded. - Contract test service consumes `events` in a Task and forwards to the callback URL instead of implementing EventHandler.
The network tests were gated behind a blanket #if !os(Linux) && !os(Windows) dating to when swift-corelibs-foundation had no custom-URLProtocol support. That is stale: 11 of the 12 tests pass on Linux today. Enable them there and guard only the parts that depend on Darwin-specific URLSession/URLProtocol behavior, each annotated with the reason: - Session httpAdditionalHeaders (Accept, Cache-Control) are not surfaced to URLProtocol on swift-corelibs-foundation -> those assertions stay Darwin-only. - The request body stays in httpBody rather than an httpBodyStream on Linux -> assert httpBody on Linux, bodyStreamAsData() on Darwin. - A reconnect after an error-response .cancel is not re-routed through the custom URLProtocol on Linux -> retryOnInvalidResponseCode stays Darwin-only. (The open -> finish -> reconnect path used by other tests works cross-platform.) Windows remains excluded (mock interception there is unverified). Linux now runs 72 tests (was 61); the full set runs on macOS.
Follow-up to the Phase 2 review (SDK-2607). No public-API change.
- Document the events stream contract: single-consumer, and unbounded with no
backpressure (consumer must drain continuously). Records why .unbounded is
deliberate -- dropping events would break last-event-id resumption and the
URLSessionDataDelegate path cannot backpressure the socket anyway.
- Restore the "fully consumed" teardown invariant the swift-testing migration
dropped: every network test asserts no leftover mock request and no leftover
stream event. The check is deterministic -- it awaits the collector's drain
(which finishes when the stream finishes) then inspects both sinks, rather than
relying on a timing window.
- Harden EventSource lifetime: deinit { stop() } so a dropped, never-iterated
source tears its connection down; capture the delegate weakly in
onTermination so the continuation does not retain it; guard connect() against
reopening after shutdown.
- Test hygiene: slim EventSink to the methods the parser tests use (drops an
unused force-unwrap-on-timeout path); make RequestHandler.stopped lock-safe so
teardown can be observed without a data race.
- Add tests for the two under-covered behaviors: cancelling the consumer tears
the connection down, and the stream keeps delivering across a reconnect (an
error/close is a value, not a termination).
…ream
Aligns the AsyncSequence events surface with LaunchDarkly's Flutter SSE client
so the two SDKs' FDv2 streaming layers stay parallel and the swift-core consumer
(SDK-2614) can read what the FDv2 protocol needs.
BREAKING CHANGE: EventSourceEvent.opened now carries the response headers, and
.error now carries a structured EventSourceError instead of (any Error).
- .opened(headers:) carries the connection's response headers (keys lowercased),
mirroring Flutter's OpenEvent.headers. The FDv2 protocol reads x-ld-envid and
x-ld-fd-fallback from these.
- .error(EventSourceError) carries statusCode (when present), the response
headers (load-bearing on error responses too), a recoverable flag, and the
underlying transport error. Mirrors Flutter's SseHttpError.
- The client classifies recoverability itself (5xx, 400, 408, 429 are
recoverable; everything else, including 204, is terminal) -- matching Flutter's
isHttpStatusCodeRecoverable. A terminal error is reported on the stream and
then the stream finishes, so it is distinguishable from stop(). connectionErrorHandler
becomes an optional override that can force a terminal outcome; its default is
now { _ in .proceed }.
Decision 3.1 (non-throwing stream, errors-as-events) is reaffirmed: a throwing
AsyncThrowingStream can't represent a recoverable error that keeps the stream
alive, so it can't mirror Flutter. Updated in plan.md.
Keeps the URLSessionDataDelegate bridge (no URLSession.bytes; Linux/Windows
safe). swift-testing coverage added for header surfacing and the
recoverable/terminal classification; README gains a Usage section.
URLSession on Darwin wraps the error passed to a URLProtocol's didFailWithError into an NSError, so the transport error's underlyingError is not the original DummyError there (swift-corelibs-foundation passes it through unwrapped, which is why this passed on Linux). Assert that an underlying error is present and the status code is absent, rather than asserting the concrete wrapped type.
| /// recoverable). The `connectionErrorHandler` may force a terminal outcome by returning `.shutdown`. | ||
| func emitError(_ error: Error, statusCode: Int?, headers: [String: String]) -> Bool { | ||
| let statusRecoverable = statusCode.map(EventSourceDelegate.isRecoverable) ?? true | ||
| let recoverable = statusRecoverable && config.connectionErrorHandler(error) != .shutdown |
Contributor
There was a problem hiding this comment.
In old code, error handler was invoked even for unrecoverable errors. Is this an intentional behavior change?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase 2 of the Swift 6 modernization (SDK-2607). Replaces the push-based
EventHandler callback with an AsyncStream of events that consumers iterate with
for await. This is the surface the swift-core streaming FDv2Synchronizer(Phase 3) needs; an AsyncStream maps almost 1:1 onto its
next() async.BREAKING CHANGE: EventHandler is no longer public and EventSource.Config no
longer takes a handler. Consume
EventSource.eventsinstead.Public API:
EventSourceEvent(opened / closed / message / comment / error).EventSource.eventsexposes anAsyncStream<EventSourceEvent>;Config.initdrops the
handlerparameter (now justinit(url:))..errorvalues, not stream terminations: the clientkeeps retrying with backoff and the stream continues, ending only on stop()
(or consumer cancellation). Lifecycle stays explicit via start()/stop().
Internal:
ContinuationEventHandler forwards the delegate's and parser's callbacks into
the stream's continuation. The URLSessionDataDelegate data path is unchanged
(no URLSession.bytes, which is unavailable on Linux/Windows Foundation).
cancelling iteration tears the connection down via onTermination.
Tests / contract service:
AsyncSink drained from the stream (no thread blocking on the cooperative
executor); the suite stays .serialized for the global URLProtocol state. The
parser tests keep the synchronous EventSink. Buffering is .unbounded.
eventsin a Task and forwards to the callbackURL instead of implementing EventHandler.
Note
High Risk
Breaking public API and changed connection/stream lifecycle (unbounded buffering, single consumer, cancellation teardown) affect all integrators and long-lived SSE consumers.
Overview
Breaking change: public
EventHandleris removed andEventSource.Configno longer takes a handler—callers usefor awaitonEventSource.eventsinstead.Adds
EventSourceEvent(opened,closed,message,comment,error). Network errors are.errorvalues, not stream termination; the client keeps reconnecting untilstop()(or consumer cancellation).stop()finishes the stream; cancelling the consumer tears the connection down viaonTermination;deinitalso stops if never shut down. Reconnect scheduling is blocked once shutdown.Internally,
ContinuationEventHandlerstill feeds the existing parser/delegate path into the stream continuation (URLSession delegate unchanged).Tests move to async observation via
EventCollector/ poll-basedAsyncSink; the contract test service forwards the stream withCallbackForwarderin aTask.Reviewed by Cursor Bugbot for commit 52b7ac5. Bugbot is set up for automated code reviews on this repo. Configure here.