Skip to content

feat(events): add flushAndWait on Apple (tier 2) - #536

Open
abelonogov-ld wants to merge 27 commits into
andrey/event-durability-tier1-bufferfrom
andrey/event-durability-tier2-bounded-flush
Open

abelonogov-ld wants to merge 27 commits into
andrey/event-durability-tier1-bufferfrom
andrey/event-durability-tier2-bounded-flush

Conversation

@abelonogov-ld

Copy link
Copy Markdown
Contributor

Tier 2 of the mobile event durability work. It stacks on tier 1 (#530), and nothing here persists events; that is tier 3.

Requirements

  • I have added test coverage for new or changed functionality
  • I have followed the repository's pull request submission guidelines
  • I have validated my changes against all supported platform versions

Related issues

Stacked on #530. Android already has the equivalent LDClient.flushAndWait(long, TimeUnit).

Describe the solution you've provided

A bounded flush: LDClient.flushAndWait(timeout:) spends up to a caller-supplied budget trying to deliver, then reports whether the events left the SDK's hands. Seconds, to match identify and start; ObjcLDClient.flushAndWait(timeout:) exposes it to Objective-C.

  • EventReporting.flushReportingOutcome is the seam. It reports true when the events were delivered, refused for good, or there were none, and false when the SDK is offline or a retryable failure means they are still waiting.
  • Response classification is now a decision rather than a Bool. processEventResponse returns settled, retryable, or dropped — the three outcomes its two callers already implied — which is what lets the flush say which one happened.
  • No deadlock from the main thread. TimeoutExecutor bounds the wait, and completions run on a queue that is neither main nor the reporter's delivery queue, so a lifecycle caller on the main thread cannot end up waiting for itself.
  • Backgrounding tries to deliver. It runs the delivery inside a ProcessInfo.performExpiringActivity assertion (BackgroundActivity), so the system does not suspend the process under a request that has only just reached the network. UIApplication.beginBackgroundTask is not available because the framework is built extension-safe.

Describe alternatives you've considered

  • An SDK-installed crash hook that flushes. Not done: the SDK has no crash hook of its own on Apple, and a Mach exception handler is not a supported caller. This is not a crash-time API, and its documentation says so.
  • Reporting delivery as a plain Bool from the response handler. It could not distinguish a permanent refusal (settled, nothing to retry) from a retryable failure, and the flush has to tell them apart.

Additional context

Spec: Event Durability §8–§12 (recoverable failure: bounded flush, response classification, lifecycle hooks, and the honesty requirement that flushAndWait is not a promise under termination).

Made with Cursor

abelonogov-ld and others added 11 commits September 18, 2026 18:38
A bounded flush spends up to a caller-supplied budget trying to deliver, then
reports whether the events left the SDK's hands. Android already had this; Apple
now has LDClient.flushAndWait(timeout:), spelled in seconds to match identify
and start.

EventReporting.flushReportingOutcome is the seam: true when the events were
delivered, refused for good, or there were none; false when the SDK is offline
or a retryable failure means they are still waiting. Saying that needed the
response classification to be a decision rather than a Bool, so
processEventResponse now returns settled, retryable, or dropped -- the three
outcomes that were already implied by its two callers.

TimeoutExecutor bounds the wait. Completions run on a queue that is neither main
nor the reporter's delivery queue, so a lifecycle caller on the main thread
cannot deadlock waiting for itself.

Backgrounding now spends its last moment trying to deliver, inside a
ProcessInfo activity assertion so the system does not suspend the process out
from under a request that has only just reached the network.

This is not a crash-time API. The SDK has no crash hook of its own on Apple, and
a Mach exception handler is not a supported caller.

Spec: Event Durability §10, §11, §17.3.
Co-authored-by: Cursor <cursoragent@cursor.com>
…-durability-tier2-bounded-flush

# Conflicts:
#	LaunchDarkly/LaunchDarkly/ServiceObjects/EventReporter.swift
…-durability-tier2-bounded-flush

Co-authored-by: Cursor <cursoragent@cursor.com>

# Conflicts:
#	LaunchDarkly.xcodeproj/project.pbxproj
…-durability-tier2-bounded-flush

Co-authored-by: Cursor <cursoragent@cursor.com>

# Conflicts:
#	LaunchDarkly/LaunchDarkly/LDClient.swift
@abelonogov-ld
abelonogov-ld requested a review from a team as a code owner October 1, 2026 15:33

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale Bugbot comment from a previous run.

Comment thread LaunchDarkly/LaunchDarkly/LDClient.swift Outdated
A flush starting while a delivery was on the wire found an empty event
store -- the events had left it when the request was sent -- and reported
success for events that could still fail. Backgrounding is where that
mattered: the hook released its activity assertion as soon as it heard
success, letting the system suspend the process out from under the very
request it was holding the assertion for.

A delivery now claims the reporter for the length of its round trip, and a
flush arriving inside that window is answered by the pass that follows it,
with the in-flight outcome folded into its answer. A tier 2 failure drops
the events rather than leaving them for that pass to retry, so a waiter
told only about the pass would hear success for events that are gone.

internalFlushAndWait read its result after a wait that had already
expired, which only the semaphore's signal orders against the write.

BackgroundActivity's documentation claimed undelivered events were already
on disk. Nothing is written until tier 3.

Co-authored-by: Cursor <cursoragent@cursor.com>

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale Bugbot comment from a previous run.

Comment thread LaunchDarkly/LaunchDarkly/ServiceObjects/EventReporter.swift Outdated
abelonogov-ld and others added 3 commits October 1, 2026 09:20
…correctly from flushAndWait

Co-authored-by: Cursor <cursoragent@cursor.com>
@cursor

cursor Bot commented Oct 2, 2026

Copy link
Copy Markdown

Current version of PR was reviewed by /review-bugbot on Oct 1, 13:41 PDT. It flagged 0 findings.

Bugbot on commit 47700b3 is skipped.

///
/// Uses `ProcessInfo.performExpiringActivity` because the framework is extension-safe and cannot use
/// `UIApplication.beginBackgroundTask`. The system may refuse or end the assertion early.
enum BackgroundActivity {

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.

Is a background activity the only option here? Is there no background service that can run for a few seconds after the application backgrounds?

@abelonogov-ld abelonogov-ld Oct 7, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

it is only way without asking customer setup xcode entitlements or asking customer to add new methods into AppDelegate. Customer can use those advanced methods itself with help of flushAndWait as well.

/// True from sending a request until its response is handled. Its events have already left `eventStore`.
private var isDelivering = false
/// A delivery was requested while one was in flight.
private var hasWaitingRequest = false

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.

We have used something like pendingRequest in other implementations. The word "pending" has reasonable precedent for this situation.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Now it is hasPendingFlush

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Renamed to hasPendingFlush / pendingFlushCompletions, matching Android's pendingFlush

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

replaced by queuedDeliverything

// A terminal response takes the reporter offline after settling; with nothing left, that is not a failure.
let nothingPending = eventStore.isEmpty && !contextSummarizer.hasLoggedRequests
reportEvents { result in
waiting.forEach { $0(delivered && (result || nothingPending)) }

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.

Is it ok to call these while inside self.eventQueue.async ? It probably could be fine if they are short.

@abelonogov-ld abelonogov-ld Oct 7, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, it is not ideal
Tier 3 moved recording off the queue entirely during optimization work. It renamed eventQueue to deliveryQueue.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Also now it is testable

}
finished()
}
}

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.

Do we need to put any rate limiting in place here? Could this pathway allow an SDK to make requests more rapidly than it did previously in such a way that could lead to more load on the servers?

@tanderson-ld tanderson-ld Oct 5, 2026 •

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.

Matching Android is probably sufficient.

extension LDClient: TypeIdentifying { }

// MARK: - Event delivery
extension LDClient {

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.

Why an extension in the same file as the LDClient impl? Is this a swift norm for organization?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, it is usually a norm but not in every company :)
There is standard practice to use extension LDClient { to split code into parts, here flush functionality.

In this repo we already did that but only with creating new files. Gonna follow existing precedent with LDClientIdentifyHook.swift and LDClientVariation.swift

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Created LDClientFlush.swift

else {
os_log("%s Failed to serialize event(s) for publication: %s", log: service.config.logger, type: .error, typeName(and: #function), String(describing: events))
completion?()
// Encoding is deterministic, so no retry would succeed.

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.

The batch was already removed from eventStore, so an un-encodable batch is unrecoverable data loss — reported as true. "No retry would succeed" justifies not retrying, not reporting success; and the partial case has the same shape (encodeSkippingFailures silently drops individual events and the surviving batch still settles as true). This is reachable from app data: a NaN/±inf Double via track's metricValue/data or a context attribute makes the JSON writer fail, and track does no finite-number validation. A caller using flushAndWait to decide "are my events safe?" gets true for events that were destroyed. .dropped → false seems like the more honest classification for this path — or validate at track time so it can't arise.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

  1. Now an un-encodable batch returns false: encode answering nil short-circuits before
  2. Worth a follow-up if you think it matters, tier 3 reports these losses explicitly.

/**
Sends any currently queued events and waits up to `timeout` for the delivery to finish.

Returns `YES` if the events were delivered, or there were none to deliver. Returns `NO` if the timeout

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.

This doc doesn't match the implementation on two counts: (1) a terminal 401/403 drops the batch, takes the reporter offline, and returns YES (it classifies as .settled) — squarely "otherwise unable to deliver them"; (2) once offline, flushAndWait returns NO even with nothing pending, contradicting "or there were none to deliver" (the offline guard runs before the empty-store check, and a terminal response makes that sticky for the rest of the process). The behavior is defensible and matches the flushReportingOutcome contract; these sentences (and the Swift doc's "left the SDK's hands") are what need to change.

Also worth deciding deliberately: .NET's FlushAndWait counts "definitively failed" as true, so our .dropped → false diverges. Either semantic is fine, but we should pick one on purpose and document it on both APIs. (Nit: a - returns: line here would match the Swift side.)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The ObjC doc had also drifted from the Swift one, so I brought them back into lockstep — it was missing the
paragraph on the timeout bounding the wait rather than the delivery, and the - parameter timeout: line was a
single clause against Swift's. They're now identical apart from true/false versus YES/NO.

let deadline = Date().addingTimeInterval(max(0, timeout))
var delivered = true
for client in clients {
let remaining = max(0, deadline.timeIntervalSinceNow)

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.

Two issues with the shared budget: (1) each environment adds its own +0.25s grace (finished.wait(timeout: .now() + timeout + 0.25)), so the real worst case is timeout + 0.25×N, not the documented single budget; (2) once the budget is exhausted, later environments run with remaining == 0, where the TimeoutExecutor fallback (asyncAfter(.now())) essentially always beats the reporter's hop to eventQueue — they report false even with nothing to send, and the && drags the whole result down, in nondeterministic Dictionary.values order. Consider computing the slack once against the shared deadline (or running environments concurrently against it), and short-circuiting once the budget is spent.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Both fixed. There is now one absolute deadline computed once up front, and every wait is against that same
instant, so the worst case is the timeout the caller asked for however many environments there are the
per-environment +0.25s grace is gone, and so is the TimeoutExecutor indirection that produced it.

func recordFlagEvaluationEvents(flagKey: LDFlagKey, value: LDValue, defaultValue: LDValue, featureFlag: FeatureFlag?, context: LDContext, includeReason: Bool)
func flush(completion: CompletionClosure?)

/// Like `flush`. Reports `true` if events were delivered, permanently refused, or absent; `false` if offline or

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.

Two things worth capturing in this contract while it's being defined: (1) completions run on the reporter's serial eventQueue, so a completion that re-enters the reporter (e.g. record, which does eventQueue.sync) deadlocks — no current caller does, but tier 3 should know the queue contract; (2) a waiter queued behind an in-flight delivery inherits that batch's failure even if its own events succeed in the follow-up pass (delivered && (result || nothingPending)). Conservative is the right direction, but a caller treating false as "my events are still buffered, safe to re-send" will duplicate events that actually landed — worth a sentence here and on flushAndWait.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

  1. On tier 3 this is a contract change for anything that assumed completions were serialized on eventQueue. Tier 3's delivery state is confined to deliveryQueue, so I'll need to re-check that when merging forward.
  2. It is gone with the single-flight gate. A delivery that has already started is no
    longer joined, so an answer is always about the batch the caller's own delivery took, never one an earlier
    delivery lost.

var publishedEventData: Data?
/// Holds responses so a test can act while a delivery is in flight.
var holdsEventCompletions = false
private var heldEventCompletions: [ServiceCompletionHandler] = []

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.

holdsEventCompletions/heldEventCompletions (and the existing call-count/data fields) are mutated from DispatchQueue.global() — and the retry's queue — while the test thread writes the flag and clears the array in releaseHeldEventCompletions, with no synchronization. A concurrent append and = [] can corrupt the array; this will flake CI and light up TSan. A small lock or serial queue around the event-publish state would do it.

abelonogov-ld and others added 6 commits October 7, 2026 15:17
…lush into LDClientFlush.swift

Co-authored-by: Cursor <cursoragent@cursor.com>
Also gates the background flush to platforms that suspend, holds one
background assertion per client, and reports flushAndWait's outcome as
Android does: true only when LaunchDarkly accepted the events.

Co-authored-by: Cursor <cursoragent@cursor.com>
A delivery that had started held back every later one, including the
periodic timer's, until it answered. On the retryable path that is about
twice the connection timeout, and nothing emptied the event store for the
whole of it, so a busy application on a flaky network lost events at the
capacity it was supposed to be buffered by.

Deliveries now coalesce only while queued and not yet started, which is
what Android's DirectEventProcessor does: runDelivery clears the queued
waiters before it takes the store, so anything arriving after that gets a
delivery of its own. A flush still joins one that has not started, since
that delivery will carry its events too.

An answer is now always about the batch the caller's own delivery took.
That retires the inherited failure a waiter used to pick up from the
in-flight batch, which could report events lost that had in fact landed.

Completions are also answered off the reporter's queue, uniformly. They
had been split between it and whichever thread the delivery ended on, so
recording from a completion deadlocked, but only when the event store
happened to be empty.

Also from review:

- Report false from flushAndWait for a client a later start replaced,
  rather than another client's outcome.
- Answer publishEventData where there is no mobile key, rather than
  dropping the completion.
- Keep a flush nobody waits on from adding a waiter.
- Hold no assertion around work that does not need one in
  BackgroundActivity, which drops the refusal case entirely.
- Match the Objective-C flushAndWait doc to the Swift one, and say on
  both that a definitive failure is reported as a failure, since other
  LaunchDarkly SDKs report it as a success.
- Guard the event-publish state in DarklyServiceMock, which concurrent
  deliveries now reach from more than one thread.

Co-authored-by: Cursor <cursoragent@cursor.com>
flushAndWait is the only way to learn whether events reached
LaunchDarkly, and it blocks. From a Swift concurrency task that is not
safe: the cooperative pool does not grow to replace a thread held that
way, so enough of these stall every other task in the process, including
whichever one would have finished the delivery.

flush(completion:) reports the same outcome over every environment and
returns at once. It takes no timeout, because a delivery always ends by
its own request timeouts if nothing else, and the caller is not waiting
on it in any case.

The documentation on all three now says which to reach for, and
flushAndWait says outright that it blocks the calling thread and must
not be called from a task.

Co-authored-by: Cursor <cursoragent@cursor.com>
…efusal

The assertion the process grants was taken directly from ProcessInfo, so
nothing could exercise the paths that matter: a system that refuses one
outright, and the cap on work that never finishes. The backgrounding
test in LDClientSpec was also at the mercy of whether the simulator felt
like granting one.

processAssertion takes the system call and the cap as arguments, so a
test can refuse an assertion, grant one, or let it run past the cap,
without the system's cooperation. Three tests cover those, and the
refusal one fails if the work is moved back inside the activity block,
which is where it would never run under the resource pressure this is
here for.

The refusal is now logged as well. The delivery goes ahead either way,
so it is no longer silent, but a log could not tell a held assertion
from one that was never granted.

Co-authored-by: Cursor <cursoragent@cursor.com>
…-durability-tier2-bounded-flush

Co-authored-by: Cursor <cursoragent@cursor.com>

# Conflicts:
#	LaunchDarkly/LaunchDarkly/LDClient.swift

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale Bugbot comment from a previous run.

os_log("%s called on a closed client", log: config.logger, type: .debug, self.typeName(and: #function))
completion(false)
return
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Flush completion runs on caller

Medium Severity

flush(completion:) documents that the completion always runs on a background queue, not the caller’s thread, so it is safe to record or flush again. A closed client and NullEventReporter (sendEvents false) invoke that completion synchronously on the caller instead, which can deadlock a caller that hops back onto its current queue.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit d5215f5. Configure here.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.

There are 3 total unresolved issues (including 1 from previous review).

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit f6cd58c. Configure here.

}
finished()
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Background flush skips in-flight delivery

High Severity

Backgrounding calls flushReportingOutcome, which does not join a delivery that has already taken the store. If events are on the wire or in the retry delay, the store is empty, the new flush answers true at once, and finished() drops the assertion. The process can then suspend and kill that in-flight request. Those events are already gone from the store, so they are lost, and the “did not reach LaunchDarkly” log does not fire. That hole is widest during the retry window this change otherwise went out of its way to keep draining.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit f6cd58c. Configure here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Tier 3 (#538), which sits directly on this branch, narrows that without relying on the disk. A delivery no longer takes events out of the store: claimDeliverableBatches only marks payload IDs in batchesInFlight, and remove runs solely once LaunchDarkly has accepted or permanently refused the batch. With persistence off that batch sits in inMemoryBatches until then, so a suspension that is later resumed redelivers it. Neither branch ships on its own.


func flushReportingOutcome(completion: @escaping FlushOutcomeClosure) {
completion(true)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Flush completions run on caller thread

Medium Severity

NullEventReporter.flushReportingOutcome (used when sendEvents is false) and the closed-client path of flush(completion:) invoke the completion on the caller’s thread. The API promises a background queue that is neither the caller nor the reporter queue, so callers are told they may record or flush again. A caller who follows that and re-enters the same serial queue or lock deadlocks. The same sync answer also lets flushAndWait(timeout: 0) return true when events are disabled, despite the documented “zero does not wait, reports false” rule.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit f6cd58c. Configure here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

NullEventReporter.flush and .flushReportingOutcome, and the closed-client path of LDClient.flush(completion:), now answer on a background queue. Fixed on tier-3

abelonogov-ld and others added 4 commits October 12, 2026 09:40
…-durability-tier2-bounded-flush

Co-authored-by: Cursor <cursoragent@cursor.com>
On macOS processAssertion runs the work inline and never asks the system, so the specs that wait for the system to end the assertion timed out there and failed test-swiftpm. macOS gets a spec for its own behavior instead.

Co-authored-by: Cursor <cursoragent@cursor.com>
…-durability-tier2-bounded-flush

Co-authored-by: Cursor <cursoragent@cursor.com>

This branch has not been deployed

No deployments
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