Revert "fix(api): recycle WebSocket and resubscribe on same-online network path change (#4202)" (#4228)

This reverts commit be0a214.

Author: harsh62

AGENTS.md

@@ -98,10 +98,7 @@ swift test --filter AWSCognitoAuthPluginUnitTests    # Specific target
 - **Unit tests**: XCTest, defined in Package.swift (19 test targets)
 - **Integration tests**: Xcode host app projects under `AmplifyPlugins/<Category>/Tests/<Category>HostApp/`
 - **Conventions**: Mock via behavior protocols, use `AmplifyTestCommon` for shared utilities, `AmplifyAsyncTesting` for async helpers
-- **Test documentation (MANDATORY)**: Every new or modified test method
-  **must** have a Given/When/Then doc comment. No exceptions — this applies
-  to unit tests, integration tests, and regression tests alike. Reviewers
-  should reject PRs that add tests without this structure.
+- **Test documentation**: Use Given/When/Then doc comments on all test methods:
   ```swift
   /// Test description
   ///

AmplifyPlugins/API/Sources/AWSAPIPlugin/AppSyncRealTimeClient/AppSyncRealTimeClient.swift

@@ -251,13 +251,8 @@ actor AppSyncRealTimeClient: AppSyncRealTimeClientProtocol {
 
     private func resumeExistingSubscriptions() {
         log.debug("[AppSyncRealTimeClient] Resuming existing subscriptions")
-        for (id, subscription) in subscriptions {
+        for (id, _) in subscriptions {
             Task { [weak self] in
-                // Reset local state so subscribe() re-sends .start to the
-                // server. After a reconnect, the server has no memory of
-                // prior subscriptions, so stale local .subscribed state must
-                // not short-circuit the resubscription.
-                await subscription.prepareForResubscribe()
                 do {
                     if let cancellable = try await self?.startSubscription(id) {
                         await self?.storeInConnectionCancellables(cancellable)

AmplifyPlugins/API/Sources/AWSAPIPlugin/AppSyncRealTimeClient/AppSyncRealTimeSubscription.swift

@@ -84,15 +84,6 @@ actor AppSyncRealTimeSubscription {
         state.send(.subscribed)
     }
 
-    /// Reset local subscription state so `subscribe()` will actually resend
-    /// the `.start` request after a WebSocket reconnect. The server drops
-    /// subscription state when the connection dies, so local `.subscribed`
-    /// state is stale and must not short-circuit the resubscription path.
-    /// Fix for https://github.com/aws-amplify/amplify-swift/issues/3976
-    func prepareForResubscribe() {
-        state.send(.none)
-    }
-
     func unsubscribe() async throws {
         guard state.value == .subscribed else {
             log.debug("[AppSyncRealTimeSubscription-\(id)] Subscription should be subscribed to be unsubscribed")

AmplifyPlugins/API/Tests/APIHostApp/AWSAPIPluginFunctionalTests/AppSyncRealTimeClientTests.swift

@@ -169,100 +169,6 @@ class AppSyncRealTimeClientTests: XCTestCase {
         withExtendedLifetime(cancellables) { }
     }
 
-    /// End-to-end regression test for https://github.com/aws-amplify/amplify-swift/issues/3976
-    /// against a real AppSync backend. Simulates the scenePhase-triggered
-    /// NWPath recycle by driving two .online states through an injected
-    /// AmplifyNetworkMonitor and asserts that the subscription is actually
-    /// re-established (server issues a second start_ack).
-    ///
-    /// - Given:
-    ///    - An AppSyncRealTimeClient wired to a real AmplifyNetworkMonitor
-    ///      and a real AppSync endpoint from the bundled config.
-    ///    - A live subscription that has already received .subscribed from
-    ///      the server (first start_ack confirmed).
-    /// - When:
-    ///    - After the WebSocketClient's internal sink has attached (200ms),
-    ///      updateState(.online) is called twice on the network monitor,
-    ///      producing the (.online, .online) tuple from issue #3976.
-    /// - Then:
-    ///    - The WebSocket is recycled, AppSyncRealTimeClient reconnects,
-    ///      resumeExistingSubscriptions() re-sends `start`, and the server
-    ///      returns a second start_ack — causing a second .subscribed event.
-    func testSubscribe_afterOnlineToOnlinePathChange_shouldRecycleAndResubscribe() async throws {
-        var cancellables = Set<AnyCancellable>()
-
-        let data = try TestConfigHelper.retrieve(
-            forResource: GraphQLModelBasedTests.amplifyConfiguration
-        )
-        let amplifyConfig = try JSONDecoder().decode(JSONValue.self, from: data)
-        let (endpoint, apiKey) = (amplifyConfig.api?.plugins?.awsAPIPlugin?.asObject?.values
-            .map { ($0.endpoint?.stringValue, $0.apiKey?.stringValue) }
-            .first { $0.0 != nil && $0.1 != nil }
-            .map { ($0.0!, $0.1!) })!
-
-        // Inject a real AmplifyNetworkMonitor we can drive directly.
-        let networkMonitor = AmplifyNetworkMonitor()
-
-        let webSocketClient = WebSocketClient(
-            url: AppSyncRealTimeClientFactory.appSyncRealTimeEndpoint(URL(string: endpoint)!),
-            handshakeHttpHeaders: [
-                URLRequestConstants.Header.webSocketSubprotocols: "graphql-ws",
-                URLRequestConstants.Header.userAgent: AmplifyAWSServiceConfiguration.userAgentLib + " (intg-test-3976)"
-            ],
-            interceptor: APIKeyAuthInterceptor(apiKey: apiKey),
-            networkMonitor: networkMonitor
-        )
-        let client = AppSyncRealTimeClient(
-            endpoint: URL(string: endpoint)!,
-            requestInterceptor: APIKeyAuthInterceptor(apiKey: apiKey),
-            webSocketClient: webSocketClient
-        )
-        defer { Task { await client.reset() } }
-
-        // Wait for WebSocketClient's internal sink to attach to the monitor's
-        // publisher (it's kicked off via a Task in init). Prime with .online
-        // AFTER the subscriber is attached so the PassthroughSubject actually
-        // delivers the event. This gets the scan to (.none, .online) —
-        // WebSocketClient will ignore it because autoConnect is still false.
-        try await Task.sleep(nanoseconds: 200_000_000)
-        await networkMonitor.updateState(.online)
-
-        let firstSubscribed = expectation(description: "Initial subscription established")
-        let resubscribedAfterPathChange = expectation(description: "Subscription re-established after (.online, .online)")
-        resubscribedAfterPathChange.assertForOverFulfill = false
-
-        let id = UUID().uuidString
-        let subscribedCount = AtomicInt()
-        let subscription = try await client.subscribe(
-            id: id,
-            query: Self.appSyncQuery(with: subscriptionRequest)
-        ).sink { event in
-            if case .subscribed = event {
-                let count = subscribedCount.increment()
-                if count == 1 {
-                    firstSubscribed.fulfill()
-                } else {
-                    resubscribedAfterPathChange.fulfill()
-                }
-            }
-        }
-        cancellables.insert(subscription)
-
-        try await client.connect()
-        await fulfillment(of: [firstSubscribed], timeout: 10)
-
-        // Simulate the path-recycle: second .online emission produces
-        // (.online, .online) through the scan — the exact bug tuple.
-        // In the buggy code, nothing happens; WebSocketClient keeps the
-        // zombie connection. With the fix, it should tear down and reconnect,
-        // and AppSyncRealTimeClient.resumeExistingSubscriptions() should
-        // re-subscribe.
-        await networkMonitor.updateState(.online)
-
-        await fulfillment(of: [resubscribedAfterPathChange], timeout: 15)
-        withExtendedLifetime(cancellables) { }
-    }
-
     private func makeOneSubscription(
         id: String = UUID().uuidString,
         onSubscriptionEvents: ((AppSyncSubscriptionEvent) -> Void)?
@@ -295,14 +201,3 @@ class AppSyncRealTimeClientTests: XCTestCase {
     }
 
 }
-
-private final class AtomicInt: @unchecked Sendable {
-    private var value: Int = 0
-    private let lock = NSLock()
-    func increment() -> Int {
-        lock.lock()
-        defer { lock.unlock() }
-        value += 1
-        return value
-    }
-}

AmplifyPlugins/Core/AWSPluginsCore/WebSocket/WebSocketClient.swift

@@ -292,23 +292,6 @@ extension WebSocketClient {
         case (.offline, .online):
             log.debug("[WebSocketClient] NetworkMonitor - Device back online")
             await createConnectionAndRead()
-        case (.online, .online):
-            // NWPathMonitor's pathUpdateHandler only fires on real path
-            // changes, so a second .satisfied emission while we were already
-            // online means the underlying path was swapped (e.g., iOS recycled
-            // the TCP route during a scenePhase transition). The existing
-            // URLSessionWebSocketTask is now bound to a stale route and any
-            // further reads/writes will silently fail, leaving the client in
-            // a zombie state that cached consumers can't recover from.
-            // Fix for https://github.com/aws-amplify/amplify-swift/issues/3976
-            guard connection?.state == .running else {
-                log.debug("[WebSocketClient] NetworkMonitor - Path changed but connection is not running, skipping recycle")
-                break
-            }
-            log.debug("[WebSocketClient] NetworkMonitor - Network path changed while online, recycling connection")
-            connection?.cancel(with: .invalid, reason: nil)
-            subject.send(.disconnected(.invalid, nil))
-            await createConnectionAndRead()
         default:
             break
         }

AmplifyPlugins/Core/AWSPluginsCoreTests/WebSocket/WebSocketClientTests.swift

@@ -139,168 +139,6 @@ class WebSocketClientTests: XCTestCase {
         await fulfillment(of: [reconnectExpectation], timeout: timeout)
     }
 
-    /// Regression test for https://github.com/aws-amplify/amplify-swift/issues/3976.
-    /// When iOS recycles the TCP route during a scenePhase transition,
-    /// NWPathMonitor reports .satisfied both before and after, producing
-    /// (.online, .online) through AmplifyNetworkMonitor's scan. Before the
-    /// fix, WebSocketClient.onNetworkStateChange hit `default: break` and
-    /// left the stale URLSessionWebSocketTask in place — a zombie.
-    ///
-    /// - Given:
-    ///    - A WebSocketClient connected via a MockNetworkMonitor whose scan
-    ///      seed is (.online, .online), so one updateState(.online) produces
-    ///      the bug tuple deterministically.
-    ///    - autoConnectOnNetworkStatusChange is true.
-    /// - When:
-    ///    - The mock emits .online, producing an (.online, .online) tuple.
-    /// - Then:
-    ///    - WebSocketClient sends a `.disconnected` event (stale task torn down).
-    ///    - WebSocketClient emits a fresh `.connected` event (new connection).
-    func testWebSocketClient_whenNetworkPathChangesWhileOnline_shouldRecycleConnection() async throws {
-        var cancellables = Set<AnyCancellable>()
-        guard let endpoint = try localWebSocketServer?.start() else {
-            XCTFail("Local WebSocket server failed to start")
-            return
-        }
-
-        let mockNetworkMonitor = MockNetworkMonitor()
-        let webSocketClient = WebSocketClient(url: endpoint, networkMonitor: mockNetworkMonitor)
-        await verifyConnected(webSocketClient, autoConnectOnNetworkStatusChange: true)
-
-        let disconnectExpectation = expectation(description: "Path change should force a disconnect")
-        let reconnectExpectation = expectation(description: "Path change should trigger a reconnect")
-
-        await webSocketClient.publisher.sink { event in
-            switch event {
-            case .disconnected:
-                disconnectExpectation.fulfill()
-            case .connected:
-                reconnectExpectation.fulfill()
-            default:
-                break
-            }
-        }
-        .store(in: &cancellables)
-
-        // Simulate NWPathMonitor firing .satisfied again after a path recycle.
-        // The scan seed in MockNetworkMonitor is (.online, .online), so sending
-        // .online produces exactly the (.online, .online) tuple from issue #3976.
-        await mockNetworkMonitor.updateState(.online)
-
-        await fulfillment(
-            of: [disconnectExpectation, reconnectExpectation],
-            timeout: timeout,
-            enforceOrder: true
-        )
-    }
-
-    /// Integration-level companion to the mock-based test above. Proves the
-    /// fix works with the real AmplifyNetworkMonitor's scan seed (.none, .none)
-    /// — i.e., the bug is not an artifact of MockNetworkMonitor's seeding.
-    /// Drives state through `updateState`, the same seam WebSocketClient
-    /// itself uses when reporting connectionLost. Tolerates spontaneous
-    /// NWPathMonitor firings on watchOS by counting `.connected` events
-    /// instead of using the strict verifyConnected helper.
-    ///
-    /// - Given:
-    ///    - A WebSocketClient wired to a real AmplifyNetworkMonitor with its
-    ///      natural scan seed (.none, .none).
-    ///    - A publisher sink attached before connect() so no events are lost
-    ///      while the client's internal sink is still attaching.
-    ///    - autoConnectOnNetworkStatusChange is true.
-    /// - When:
-    ///    - The monitor's updateState(.online) is called twice, producing
-    ///      (.none, .online) then (.online, .online) through the scan.
-    /// - Then:
-    ///    - A second `.connected` event is observed (initial connect + recycle
-    ///      reconnect), confirming the WebSocket was torn down and rebuilt.
-    func testWebSocketClient_withRealNetworkMonitor_whenPathChangesWhileOnline_shouldRecycle() async throws {
-        var cancellables = Set<AnyCancellable>()
-        guard let endpoint = try localWebSocketServer?.start() else {
-            XCTFail("Local WebSocket server failed to start")
-            return
-        }
-
-        let realNetworkMonitor = AmplifyNetworkMonitor()
-        let webSocketClient = WebSocketClient(url: endpoint, networkMonitor: realNetworkMonitor)
-
-        let initialConnect = expectation(description: "Initial WebSocket connect")
-        let reconnectAfterPathChange = expectation(description: "Reconnect after (.online, .online)")
-        let connectedCounter = AtomicInt()
-
-        await webSocketClient.publisher.sink { event in
-            if case .connected = event {
-                let count = connectedCounter.increment()
-                if count == 1 {
-                    initialConnect.fulfill()
-                } else if count == 2 {
-                    reconnectAfterPathChange.fulfill()
-                }
-            }
-            // Tolerate .disconnected / .error / .string / .data events,
-            // which can arrive from NWPathMonitor-driven recycling or from
-            // LocalWebSocketServer teardown.
-        }
-        .store(in: &cancellables)
-
-        await webSocketClient.connect(
-            autoConnectOnNetworkStatusChange: true,
-            autoRetryOnConnectionFailure: false
-        )
-        await fulfillment(of: [initialConnect], timeout: timeout)
-
-        // WebSocketClient.init spawns its sink via Task { startNetworkMonitor() };
-        // by the time initialConnect fulfils, the sink is attached.
-        // Prime the scan so (previous, next) reaches (.online, .online) on
-        // the second updateState — first reaches (.none, .online).
-        await realNetworkMonitor.updateState(.online)
-
-        // Second .online emission → scan produces (.online, .online) —
-        // the exact tuple from issue #3976. With the fix, this triggers a
-        // recycle that yields a second `.connected`.
-        await realNetworkMonitor.updateState(.online)
-
-        await fulfillment(of: [reconnectAfterPathChange], timeout: timeout)
-    }
-
-    /// Characterizes the input signal that drives issue #3976. Proves that
-    /// the real AmplifyNetworkMonitor.publisher emits the (.online, .online)
-    /// tuple when two .online states are sent consecutively — which is what
-    /// WebSocketClient.onNetworkStateChange receives during a scenePhase-
-    /// triggered NWPath recycle. Does not exercise the fix; passes both
-    /// before and after.
-    ///
-    /// - Given:
-    ///    - A fresh AmplifyNetworkMonitor instance.
-    ///    - A publisher sink that watches for (.online, .online) tuples.
-    /// - When:
-    ///    - updateState(.online) is called twice consecutively.
-    /// - Then:
-    ///    - The publisher emits an (.online, .online) tuple via its scan —
-    ///      confirming this is the exact signal WebSocketClient must handle.
-    func testAmplifyNetworkMonitor_whenOnlineEmittedTwice_publishesOnlineOnlineTuple() async throws {
-        var cancellables = Set<AnyCancellable>()
-        let monitor = AmplifyNetworkMonitor()
-
-        let expectOnlineOnline = expectation(description: "publisher emits (.online, .online)")
-        expectOnlineOnline.assertForOverFulfill = false
-
-        monitor.publisher.sink { tuple in
-            if tuple.0 == .online && tuple.1 == .online {
-                expectOnlineOnline.fulfill()
-            }
-        }
-        .store(in: &cancellables)
-
-        // Two consecutive .online emissions must produce an (.online, .online)
-        // tuple through the scan — the exact input that triggers issue #3976
-        // in WebSocketClient.onNetworkStateChange.
-        await monitor.updateState(.online)
-        await monitor.updateState(.online)
-
-        await fulfillment(of: [expectOnlineOnline], timeout: timeout)
-    }
-
     func testAutoRetry_whenReceiveTransientFailureFromServer() async throws {
         var cancellables = Set<AnyCancellable>()
         guard let endpoint = try localWebSocketServer?.start() else {
@@ -357,17 +195,6 @@ class WebSocketClientTests: XCTestCase {
 }
 
 
-private final class AtomicInt: @unchecked Sendable {
-    private var value: Int = 0
-    private let lock = NSLock()
-    func increment() -> Int {
-        lock.lock()
-        defer { lock.unlock() }
-        value += 1
-        return value
-    }
-}
-
 private class MockNetworkMonitor: WebSocketNetworkMonitorProtocol {
     typealias State = AmplifyNetworkMonitor.State
     let subject = PassthroughSubject<State, Never>()