From d82635e9e4976ff9d16313d2a163277dbf25d4fd Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Thu, 12 Sep 2024 17:15:58 -0300 Subject: [PATCH] wip channel detach retry --- Sources/AblyChat/RoomLifecycleManager.swift | 22 ++++++-- Sources/AblyChat/SimpleClock.swift | 7 +++ .../MockRoomLifecycleContributorChannel.swift | 8 +-- .../AblyChatTests/Mocks/MockSimpleClock.swift | 9 ++++ .../RoomLifecycleManagerTests.swift | 54 +++++++++++++++++-- 5 files changed, 89 insertions(+), 11 deletions(-) create mode 100644 Sources/AblyChat/SimpleClock.swift create mode 100644 Tests/AblyChatTests/Mocks/MockSimpleClock.swift diff --git a/Sources/AblyChat/RoomLifecycleManager.swift b/Sources/AblyChat/RoomLifecycleManager.swift index ef037db4..3d3e49c2 100644 --- a/Sources/AblyChat/RoomLifecycleManager.swift +++ b/Sources/AblyChat/RoomLifecycleManager.swift @@ -26,17 +26,20 @@ internal actor RoomLifecycleManager { internal private(set) var error: ARTErrorInfo? private let logger: InternalLogger + private let clock: SimpleClock private let contributors: [Contributor] - internal init(contributors: [Contributor], logger: InternalLogger) { + internal init(contributors: [Contributor], logger: InternalLogger, clock: SimpleClock) { self.contributors = contributors self.logger = logger + self.clock = clock } - internal init(forTestingWhatHappensWhenCurrentlyIn current: RoomLifecycle, contributors: [Contributor], logger: InternalLogger) { + internal init(forTestingWhatHappensWhenCurrentlyIn current: RoomLifecycle, contributors: [Contributor], logger: InternalLogger, clock: SimpleClock) { self.current = current self.contributors = contributors self.logger = logger + self.clock = clock } // TODO: clean up old subscriptions (https://github.com/ably-labs/ably-chat-swift/issues/36) @@ -199,7 +202,20 @@ internal actor RoomLifecycleManager { changeStatus(to: .failed, error: error) } default: - fatalError("TODO") + // CHA-RL2h3: Retry until detach succeeds + while true { + do { + logger.log(message: "Will attempt to detach non-failed contributor \(contributor) in 1s.", level: .info) + // TODO: what's the correct wait time? + try await clock.sleep(nanoseconds: 1_000_000_000) + logger.log(message: "Detaching non-failed contributor \(contributor)", level: .info) + try await contributor.channel.detach() + break + } catch { + // Loop repeats after a wait + logger.log(message: "Failed to detach non-failed contributor \(contributor), error \(error). Will retry.", level: .info) + } + } } } } diff --git a/Sources/AblyChat/SimpleClock.swift b/Sources/AblyChat/SimpleClock.swift new file mode 100644 index 00000000..a7aeac15 --- /dev/null +++ b/Sources/AblyChat/SimpleClock.swift @@ -0,0 +1,7 @@ +/// A clock that causes the current task to sleep. +/// +/// Exists for mocking in tests. Note that we can’t use the Swift `Clock` type since it doesn’t exist in our minimum supported OS versions. +internal protocol SimpleClock: Sendable { + /// Behaves like `Task.sleep(nanoseconds:)`. + func sleep(nanoseconds duration: UInt64) async throws +} diff --git a/Tests/AblyChatTests/Mocks/MockRoomLifecycleContributorChannel.swift b/Tests/AblyChatTests/Mocks/MockRoomLifecycleContributorChannel.swift index fde79e65..3778b750 100644 --- a/Tests/AblyChatTests/Mocks/MockRoomLifecycleContributorChannel.swift +++ b/Tests/AblyChatTests/Mocks/MockRoomLifecycleContributorChannel.swift @@ -5,20 +5,22 @@ final actor MockRoomLifecycleContributorChannel: RoomLifecycleContributorChannel private let attachBehavior: AttachOrDetachBehavior? private let detachBehavior: AttachOrDetachBehavior? + var state: ARTRealtimeChannelState + var errorReason: ARTErrorInfo? + private(set) var attachCallCount = 0 private(set) var detachCallCount = 0 init( + initialState: ARTRealtimeChannelState, attachBehavior: AttachOrDetachBehavior?, detachBehavior: AttachOrDetachBehavior? ) { + state = initialState self.attachBehavior = attachBehavior self.detachBehavior = detachBehavior } - var state: ARTRealtimeChannelState = .initialized - var errorReason: ARTErrorInfo? - enum AttachOrDetachResult { case success case failure(ARTErrorInfo) diff --git a/Tests/AblyChatTests/Mocks/MockSimpleClock.swift b/Tests/AblyChatTests/Mocks/MockSimpleClock.swift new file mode 100644 index 00000000..9ca1ffdf --- /dev/null +++ b/Tests/AblyChatTests/Mocks/MockSimpleClock.swift @@ -0,0 +1,9 @@ +@testable import AblyChat + +actor MockSimpleClock: SimpleClock { + private(set) var sleepCallArguments: [UInt64] = [] + + func sleep(nanoseconds duration: UInt64) async throws { + sleepCallArguments.append(duration) + } +} diff --git a/Tests/AblyChatTests/RoomLifecycleManagerTests.swift b/Tests/AblyChatTests/RoomLifecycleManagerTests.swift index 64311a0a..ff03761a 100644 --- a/Tests/AblyChatTests/RoomLifecycleManagerTests.swift +++ b/Tests/AblyChatTests/RoomLifecycleManagerTests.swift @@ -16,20 +16,21 @@ private func makeAsyncFunction() -> (returnFromFunction: @Sendable (MockRoomLife ) } -private func createManager(contributors: [RoomLifecycleManager.Contributor] = []) -> RoomLifecycleManager { - .init(contributors: contributors, logger: TestLogger()) +private func createManager(contributors: [RoomLifecycleManager.Contributor] = [], clock: SimpleClock = MockSimpleClock()) -> RoomLifecycleManager { + .init(contributors: contributors, logger: TestLogger(), clock: clock) } -private func createManager(forTestingWhatHappensWhenCurrentlyIn current: RoomLifecycle, contributors: [RoomLifecycleManager.Contributor] = []) -> RoomLifecycleManager { - .init(forTestingWhatHappensWhenCurrentlyIn: current, contributors: contributors, logger: TestLogger()) +private func createManager(forTestingWhatHappensWhenCurrentlyIn current: RoomLifecycle, contributors: [RoomLifecycleManager.Contributor] = [], clock: SimpleClock = MockSimpleClock()) -> RoomLifecycleManager { + .init(forTestingWhatHappensWhenCurrentlyIn: current, contributors: contributors, logger: TestLogger(), clock: clock) } private func createContributor( + initialState: ARTRealtimeChannelState = .initialized, feature: RoomFeature = .messages, // Arbitrarily chosen, its value only matters in test cases where we check which error is thrown attachBehavior: MockRoomLifecycleContributorChannel.AttachOrDetachBehavior? = nil, detachBehavior: MockRoomLifecycleContributorChannel.AttachOrDetachBehavior? = nil ) -> RoomLifecycleManager.Contributor { - .init(feature: feature, channel: .init(attachBehavior: attachBehavior, detachBehavior: detachBehavior)) + .init(feature: feature, channel: .init(initialState: initialState, attachBehavior: attachBehavior, detachBehavior: detachBehavior)) } final class RoomLifecycleManagerTests: XCTestCase { @@ -480,4 +481,47 @@ final class RoomLifecycleManagerTests: XCTestCase { try assertIsChatError(maybeError, withCode: .presenceDetachmentFailed, cause: contributor1DetachError) } } + + // @spec CHA-RL2h3 + func test_detach_whenAContributorFailsToDetachAndEntersANonFailedState_pausesAWhileThenRetriesDetach() async throws { + // Given: A RoomLifecycleManager, with a contributor for whom: + // + // - the first two times `detach` is called, it throws an error, leaving it in the ATTACHED state + // - the third time `detach` is called, it succeeds + let detachImpl = { @Sendable (callCount: Int) async -> MockRoomLifecycleContributorChannel.AttachOrDetachResult in + if callCount < 3 { + return .failure(ARTErrorInfo(domain: "SomeDomain", code: 123)) // exact error is unimportant + } + return .success + } + let contributor = createContributor(initialState: .attached, detachBehavior: .fromFunction(detachImpl)) + let clock = MockSimpleClock() + + let manager = createManager(contributors: [contributor], clock: clock) + + let statusChangeSubscription = await manager.onChange(bufferingPolicy: .unbounded) + async let asyncLetStatusChanges = Array(statusChangeSubscription.prefix(2)) + + // When: `performDetachOperation()` is called on the manager + let roomDetachError: Error? + do { + try await manager.performDetachOperation() + roomDetachError = nil + } catch { + roomDetachError = error + } + + // Then: It attempts to detach the channel 3 times, waiting 0.5s between each attempt, the room transitions from DETACHING to DETACHED with no status updates in between, and the call to `performDetachOperation()` succeeds + let detachCallCount = await contributor.channel.detachCallCount + XCTAssertEqual(detachCallCount, 3) + + // We use "did it call clock.sleep(…)?" as a good-enough proxy for the question "did it wait for the right amount of time at the right moment?" + let clockSleepArguments = await clock.sleepCallArguments + XCTAssertEqual(clockSleepArguments, Array(repeating: 1_000_000_000, count: 2)) + + let statusChanges = await asyncLetStatusChanges + XCTAssertEqual(statusChanges.map(\.current), [.detaching, .detached]) + + XCTAssertNil(roomDetachError) + } }