Repository navigation
feat(events): add flushAndWait on Apple (tier 2) #536
There are no files selected for viewing
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,162 @@ | ||
| import Foundation | ||
| import OSLog | ||
|
|
||
| // MARK: - Flush | ||
| extension LDClient { | ||
| /** | ||
| Tells the SDK to immediately send any currently queued events to LaunchDarkly. | ||
|
|
||
| There should not normally be a need to call this function. While online, the LDClient automatically reports events | ||
| on an interval defined by `LDConfig.eventFlushInterval`. Note that this function does not block until events are | ||
| sent, it only triggers a background task to send events immediately. | ||
|
|
||
| Because it does not wait, it cannot report how the delivery went. Use `flush(completion:)` where that matters | ||
| but blocking does not suit, or `flushAndWait` where blocking is acceptable. | ||
| */ | ||
| public func flush() { | ||
| LDClient.instancesQueue.sync(flags: .barrier) { | ||
| LDClient.instances?.forEach { $1.internalFlush() } | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| Sends any currently queued events to LaunchDarkly and reports how the delivery went. | ||
|
|
||
| Like `flushAndWait`, but without blocking: the completion runs once every environment has answered. There is no | ||
| timeout, because a delivery always ends, by its own request timeouts if nothing else. This is the one to reach | ||
| for from a Swift concurrency task, where blocking is not safe. | ||
|
|
||
| - Warning: Experimental. This method is not stable, and not subject to any backwards compatibility guarantees or | ||
| semantic versioning. | ||
|
|
||
| - parameter completion: Given `true` if LaunchDarkly accepted the events, or there were none to send, and | ||
| `false` if the client is offline or closed, or any of the events were lost. See `flushAndWait` for what being | ||
| lost covers. Runs once, on a background queue that is neither the caller's thread nor the one recorded events | ||
| wait on, so it is free to record or flush again. | ||
| */ | ||
| public func flush(completion: @escaping (Bool) -> Void) { | ||
| let clients = instancesIncludingThisClient() | ||
| guard !clients.isEmpty | ||
| else { | ||
| os_log("%s called on a closed client", log: config.logger, type: .debug, self.typeName(and: #function)) | ||
| completion(false) | ||
| return | ||
| } | ||
|
cursor[bot] marked this conversation as resolved.
Resolved
|
||
|
|
||
| let lock = UnfairLock() | ||
| /// Only to be used while holding `lock`: each environment answers from whichever thread its delivery ended on. | ||
| var delivered = true | ||
| var outstanding = clients.count | ||
|
|
||
| for client in clients { | ||
| client.eventReporter.flushReportingOutcome { result in | ||
| lock.lock() | ||
| delivered = delivered && result | ||
| outstanding -= 1 | ||
|
Contributor
There was a problem hiding this comment. AI review comment: This countdown relies on every reporter answering exactly once, and nothing enforces that. An answer that never comes hangs this completion forever (there is no timeout), and flushAndWait's own copy of the pattern fails the other way — a double answer can end its wait while another environment is still writing. Options: factor one once-only aggregate type both paths share (#538's PassAnswer is nearly it), and/or take the timeout parameter the durability spec gives this method (§10.2). Either way, a test with a never-answering and a double-answering reporter would pin the contract. Same code carries into #538.
Contributor
Author
There was a problem hiding this comment. Double answer fixed and deadline introduced
Contributor
There was a problem hiding this comment. This seems fragile / not defensive. Any issue results in the top level completion never firing.
Contributor
Author
There was a problem hiding this comment. Good catch. Fixed, now deadline should resolve the issue |
||
| // Nil until the last environment answers, so the completion runs once. A reporter answering twice | ||
| // takes the count past zero rather than back to it, which cannot match again. | ||
| let outcome = outstanding == 0 ? delivered : nil | ||
| lock.unlock() | ||
|
|
||
| if let outcome { | ||
| completion(outcome) | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// Every environment's client, or none where this client is no longer one of them. | ||
| private func instancesIncludingThisClient() -> [LDClient] { | ||
| let clients = LDClient.instancesQueue.sync { LDClient.instances.map { Array($0.values) } } ?? [] | ||
| return clients.contains { $0 === self } ? clients : [] | ||
| } | ||
|
|
||
| /** | ||
| Sends any currently queued events to LaunchDarkly and waits up to `timeout` seconds for the result. | ||
|
|
||
| The timeout bounds the wait, not the delivery: an in-flight request is left to finish. Multiple environments | ||
| deliver at the same time and share the one budget, so the call takes no longer for several of them than for one. | ||
|
|
||
| A delivery that definitively failed is reported as a failure. Some other LaunchDarkly SDKs report that as a | ||
| success, on the grounds that the attempt is over, so an expectation carried from another platform may not hold | ||
| here. | ||
|
|
||
| This blocks the thread it is called on until the answer is in or the budget runs out. Where the answer is not | ||
| needed, `flush` queues the same delivery and returns at once; where it is needed but blocking does not suit, | ||
| `flush(completion:)` reports it without waiting. | ||
|
|
||
| Do not call this from a Swift concurrency task. Blocking there holds a thread of the cooperative pool, which | ||
| cannot grow to replace it, and enough of them will stall every other task in the process. Use | ||
| `flush(completion:)` from a task, wrapped in a continuation where you want to await it. | ||
|
|
||
| This is not a crash-time mechanism. It is safe to call from the main thread, but keep the budget well below | ||
| 15 seconds there. | ||
|
|
||
| - Warning: Experimental. This method is not stable, and not subject to any backwards compatibility guarantees or | ||
| semantic versioning. | ||
|
|
||
| - parameter timeout: How long to wait, in seconds. Zero or less does not wait, and so reports `false`. There is | ||
| no upper limit: a delivery always ends, by its own request timeouts if nothing else, so a very long timeout | ||
| waits for that. | ||
| - returns: `true` if LaunchDarkly accepted the events, or there were none to send. `false` if the timeout expired | ||
| first, the client is offline or closed, or any of the events were lost: refused by LaunchDarkly, still failing | ||
| after one retry, or unable to be serialized. Lost events are not kept for a later flush, so calling this again | ||
| does not resend them. A refusal also stops event delivery until the client is next set online, and until then | ||
| this returns `false` even with nothing to send. A `false` because the timeout expired does not mean the events | ||
| were not sent: the delivery is left running, and may still arrive. | ||
| */ | ||
| @discardableResult | ||
| public func flushAndWait(timeout: TimeInterval) -> Bool { | ||
| if timeout > LDClient.longTimeoutInterval { | ||
|
Contributor
There was a problem hiding this comment. Unsure if this long timeout makes sense for flush and wait. Look more closely at this.
Contributor
Author
There was a problem hiding this comment. removed in tier-3 |
||
| os_log("%s LDClient.flushAndWait was called with a timeout greater than %f seconds. We recommend a timeout of less than %f seconds.", log: config.logger, type: .info, self.typeName(and: #function), LDClient.longTimeoutInterval, LDClient.longTimeoutInterval) | ||
| } | ||
|
|
||
| let clients = instancesIncludingThisClient() | ||
| guard !clients.isEmpty | ||
| else { | ||
| // Closed, or replaced by a later start, or caught while a start is still filling the instances in. None of | ||
| // them can deliver this client's events, and saying otherwise would tell the caller they were safe. | ||
| os_log("%s called on a closed client", log: config.logger, type: .debug, self.typeName(and: #function)) | ||
| return false | ||
| } | ||
| let deadline = DispatchTime.now() + max(0, timeout) | ||
| // Counts answers rather than tracking each delivery, so that a reporter answering twice costs a spare permit | ||
| // no one reads. A DispatchGroup would fit as well, but its `leave` traps without a matching `enter`, which | ||
| // turns a broken reporter into a crash in the application. | ||
| let answered = DispatchSemaphore(value: 0) | ||
| let lock = UnfairLock() | ||
| /// Only to be used while holding `lock`: each environment answers from whichever thread its delivery ended on. | ||
| var delivered = true | ||
|
|
||
| // Every environment is started before any of them is waited on. Each has its own event reporter, so waiting on | ||
| // one before starting the next would spend the caller's budget on deliveries that could have been running all | ||
| // along, and leave the last environment with none of it. | ||
| for client in clients { | ||
| client.eventReporter.flushReportingOutcome { result in | ||
| lock.lock() | ||
| delivered = delivered && result | ||
| lock.unlock() | ||
| answered.signal() | ||
| } | ||
| } | ||
|
|
||
| // Every wait is against the one deadline rather than a fresh copy of it, so the timeout the caller asked for | ||
| // is the time this call can take however many environments there are. A delivery still running when the wait | ||
| // gives up is left to finish rather than canceled: its events are already in flight, so abandoning it would | ||
| // only make losing them certain. | ||
| for _ in clients { | ||
| guard answered.wait(timeout: deadline) == .success | ||
| else { return false } | ||
| } | ||
|
|
||
| // The waits count answers rather than environments, so a reporter answering twice can end them while another | ||
| // environment's answer is still being written. | ||
| lock.lock() | ||
| defer { lock.unlock() } | ||
| return delivered | ||
| } | ||
|
|
||
| func internalFlush() { | ||
| eventReporter.flush(completion: nil) | ||
| } | ||
| } | ||
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -534,11 +534,64 @@ public final class ObjcLDClient: NSObject { | |
| There should not normally be a need to call this function. While online, the LDClient automatically reports events | ||
| on an interval defined by `LDConfig.eventFlushInterval`. Note that this function does not block until events are | ||
| sent, it only triggers a background task to send events immediately. | ||
|
|
||
| Because it does not wait, it cannot report how the delivery went. Use `flushWithCompletion:` where that matters | ||
| but blocking does not suit, or `flushAndWait` where blocking is acceptable. | ||
| */ | ||
| @objc public func flush() { | ||
| ldClient.flush() | ||
| } | ||
|
|
||
| /** | ||
| Sends any currently queued events to LaunchDarkly and reports how the delivery went. | ||
|
|
||
| Like `flushAndWait`, but without blocking: the completion runs once every environment has answered. There is no | ||
| timeout, because a delivery always ends, by its own request timeouts if nothing else. | ||
|
|
||
| - Warning: Experimental. This method is not stable, and not subject to any backwards compatibility guarantees or | ||
| semantic versioning. | ||
|
|
||
| - parameter completion: Given `YES` if LaunchDarkly accepted the events, or there were none to send, and `NO` if | ||
| the client is offline or closed, or any of the events were lost. See `flushAndWait` for what being lost | ||
| covers. Runs once, on a background queue that is not the caller's thread. | ||
| */ | ||
| @objc public func flush(completion: @escaping (Bool) -> Void) { | ||
| ldClient.flush(completion: completion) | ||
| } | ||
|
|
||
| /** | ||
| Sends any currently queued events to LaunchDarkly and waits up to `timeout` seconds for the result. | ||
|
|
||
| The timeout bounds the wait, not the delivery: an in-flight request is left to finish. Multiple environments | ||
| deliver at the same time and share the one budget, so the call takes no longer for several of them than for one. | ||
|
Contributor
There was a problem hiding this comment. This multiple environments deliver part is worded as if it is a concern, but then says it isn't, so that is a little confusing. Reword.
Contributor
Author
There was a problem hiding this comment. Made more explicit message in using @_spi(Experimental) has their own risks. I hope to keep them in such way for short time |
||
|
|
||
| A delivery that definitively failed is reported as a failure. Some other LaunchDarkly SDKs report that as a | ||
| success, on the grounds that the attempt is over, so an expectation carried from another platform may not hold | ||
| here. | ||
|
|
||
| This blocks the thread it is called on until the answer is in or the budget runs out, so where the answer is | ||
| not needed, `flush` queues the same delivery and returns at once. | ||
|
|
||
| This is not a crash-time mechanism. It is safe to call from the main thread, but keep the budget well below | ||
| 15 seconds there. | ||
|
|
||
| - Warning: Experimental. This method is not stable, and not subject to any backwards compatibility guarantees or | ||
| semantic versioning. | ||
|
|
||
| - parameter timeout: How long to wait, in seconds. Zero or less does not wait, and so reports `NO`. There is | ||
| no upper limit: a delivery always ends, by its own request timeouts if nothing else, so a very long timeout | ||
| waits for that. | ||
| - returns: `YES` if LaunchDarkly accepted the events, or there were none to send. `NO` if the timeout expired | ||
| first, the client is offline or closed, or any of the events were lost: refused by LaunchDarkly, still failing | ||
| after one retry, or unable to be serialized. Lost events are not kept for a later flush, so calling this again | ||
| does not resend them. A refusal also stops event delivery until the client is next set online, and until then | ||
| this returns `NO` even with nothing to send. A `NO` because the timeout expired does not mean the events were | ||
| not sent: the delivery is left running, and may still arrive. | ||
| */ | ||
| @objc public func flushAndWait(timeout: TimeInterval) -> Bool { | ||
| ldClient.flushAndWait(timeout: timeout) | ||
| } | ||
|
|
||
| /** | ||
| Starts the LDClient using the passed in `config` & `context`. Call this before requesting feature flag values. The LDClient will not go online until you call this method. | ||
| Starting the LDClient means setting the `config` & `context`, setting the client online if `config.startOnline` is true (the default setting), and starting event recording. The client app must start the LDClient before it will report feature flag values. If a client does not call `start`, no methods will work. | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a huge fan of platform conditionals like this, are there any other options that could remove this conditional or maybe segregate them out of the LDClient.swift? I want to avoid a situation where there are many of these sprinkled throughout the code and it becomes a mess to maintain.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved in SystemCapabilities