Skip to content

feat!: AsyncSequence-first public API#106

Open
keelerm84 wants to merge 5 commits into
4.xfrom
mk/SDK-2607/asyncsequence-api
Open

feat!: AsyncSequence-first public API#106
keelerm84 wants to merge 5 commits into
4.xfrom
mk/SDK-2607/asyncsequence-api

Conversation

@keelerm84

@keelerm84 keelerm84 commented Jun 29, 2026

Copy link
Copy Markdown
Member

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.

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 EventHandler is removed and EventSource.Config no longer takes a handler—callers use for await on EventSource.events instead.

Adds EventSourceEvent (opened, closed, message, comment, error). Network errors are .error values, not stream termination; the client keeps reconnecting until stop() (or consumer cancellation). stop() finishes the stream; cancelling the consumer tears the connection down via onTermination; deinit also stops if never shut down. Reconnect scheduling is blocked once shutdown.

Internally, ContinuationEventHandler still feeds the existing parser/delegate path into the stream continuation (URLSession delegate unchanged).

Tests move to async observation via EventCollector / poll-based AsyncSink; the contract test service forwards the stream with CallbackForwarder in a Task.

Reviewed by Cursor Bugbot for commit 52b7ac5. Bugbot is set up for automated code reviews on this repo. Configure here.

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).
@keelerm84 keelerm84 requested a review from a team as a code owner June 29, 2026 12:44
@keelerm84 keelerm84 marked this pull request as draft June 29, 2026 14:29
…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.
@keelerm84 keelerm84 marked this pull request as ready for review June 29, 2026 16:25
/// 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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In old code, error handler was invoked even for unrecoverable errors. Is this an intentional behavior change?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants