Conversation
…olicies Adds `clock` to `SupabaseClientOptions.GlobalOptions`, `RealtimeClientOptions` and `AuthClient.Configuration`, all defaulting to `ContinuousClock()`, so consumers can drive time-dependent behaviour with a `TestClock` instead of waiting out real seconds. The two `package`-visibility clock initializers this replaces are gone. Routes Auth through that clock: the auto-refresh loop in `SessionManager` was still on `Task.sleep(nanoseconds:)`, and `RetryRequestInterceptor` was still on a default `ContinuousClock()`, so a caller passing a test clock would have found half of Auth ignoring it. Sets `bufferingPolicy` explicitly at all 14 `makeStream()` sites. Every one stays `.unbounded`, now deliberately and with a stated reason rather than by omission: three of these streams carry control flow (`ChannelStateManager` waits for `.subscribed`, `ConnectionManager`'s observer latches on `.reconnecting`, `URLSessionWebSocket.events` carries `phx_reply` frames) and the rest carry server payloads, so a bounded policy would either hang a waiter or silently drop caller data. Also moves `waitUntil` from `Tests/RealtimeTests/TestSupport.swift` into `Sources/TestHelpers` so the Auth suite can reuse it. Fixes SDK-1799 Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Closes the three parts of SDK-1799 that hold up against the code, and records why the fourth does not.
Clock injection is public now
clockis now onSupabaseClientOptions.GlobalOptions,RealtimeClientOptionsandAuthClient.Configuration, all defaulting toContinuousClock(). Consumers can drivetime-dependent behaviour with a
TestClockinstead of waiting out real seconds. The twopackage-visibility clock initializers this replaces are gone.Auth is routed through that clock for the first time: the auto-refresh loop in
SessionManagerwas still on
Task.sleep(nanoseconds:), andRetryRequestInterceptorwas still on a defaultContinuousClock(). A caller passing a test clock would otherwise have found half of Authignoring it.
Additive and source-compatible — new defaulted parameters only, so no
V3_MIGRATION.mdentry.Buffering policies are explicit, and all stay unbounded
bufferingPolicyis now set at all 14makeStream()sites with a stated reason. Every one stays.unbounded, deliberately rather than by omission.The issue proposed
.bufferingNewest(1)for status and heartbeat. That breaks three existingtests and the pattern they document:
RealtimeLifecycleTests.swift:133already carries the comment "Subscribe before the OS closeso the buffered
.disconnectedevent is not missed even when.reconnectingfollowsimmediately", then does
statusUpdates.first { $0 == .disconnected }. Under.bufferingNewest(1),.connectinglands right behind.disconnectedand evicts it.RealtimeTests.swift:527and:621collect heartbeat statuses and assert the sequence[.sent, .ok]..okfollows.sentwithin one ack, so.sentis dropped.Three more streams carry control flow and must stay unbounded regardless:
ChannelStateManagerwaits for
.subscribed/.unsubscribed,ConnectionManager's observer latches on.reconnecting, andURLSessionWebSocket.eventscarriesphx_replyframes. The rest carryserver payloads, where a bounded policy is silent data loss.
Task ownership: no change, because there is no leak
I audited all 15 sites the issue lists. The long-lived tasks are already owned and cancelled in
deinit(heartbeatTask,messageTask,stateObserverTask,pendingDisconnectTask).URLSessionWebSocket._scheduleReceiveis unowned but self-terminating —close()cancels thetask, the pending
receive()throws, the loop ends. Everything else is a one-shot hop thatcompletes in milliseconds;
AuthClient.deinit's task must outliveselfby design. Full auditis on the Linear issue.
Tests
SessionManagerTests.autoRefreshTicksOnTheInjectedClockdrives a full auto-refresh tick on aTestClockin 0.025s, where the real tick is 30s.SupabaseClientTests.globalClockReachesAuthAndRealtimeasserts the caller's clock instancereaches both sub-clients.
waitUntilmoved fromTests/RealtimeTests/TestSupport.swiftintoSources/TestHelpersso theAuth suite reuses it rather than duplicating it.
swift-format lint --recursive --strict Sources Tests,./scripts/test-docs.shand./scripts/spell-check.share all clean.Reviewer notes
Start with
Sources/Supabase/SupabaseClient.swiftandSources/Auth/Internal/SessionManager.swift— that is the whole behavioural change. The 14 buffering sites are comments plus an explicit
argument, no behaviour change.
One flake worth knowing about: my first version of the auto-refresh test stopped the loop in a
fire-and-forget
defer { Task { ... } }.LiveSessionManagerresolvesDependencies[clientID]on every access, so a loop still running at test exit refreshed against the next test's
transport. I caught it, and the stop is awaited now.
Fixes SDK-1799