Skip to content

fix(api): emit disconnected directly from didCompleteWithError to trigger auto-retry - #4222

Open
thisisabhash wants to merge 7 commits into
mainfrom
fix/gh-4220
Open

fix(api): emit disconnected directly from didCompleteWithError to trigger auto-retry#4222
thisisabhash wants to merge 7 commits into
mainfrom
fix/gh-4220

Conversation

@thisisabhash

@thisisabhash thisisabhash commented May 11, 2026

Copy link
Copy Markdown
Member

Issue #

#4220

Description

fix(api): emit disconnected directly from didCompleteWithError to trigger auto-retry

Summary

Fixes #4220. Resolves GraphQL subscriptions getting permanently stuck in connecting state after repeated iOS background/foreground transitions.

Problem

When iOS suspends the app process, the kernel defuncts TCP flows bound to the WebSocket connection. On foreground return, didCompleteWithError fires with NSPOSIXErrorDomain Code=53 "Software caused connection abort". The old code handled this by calling networkMonitor.updateState(.offline), expecting NWPathMonitor to later fire .online to trigger reconnection. But NWPathMonitor only fires on path changes — if the network path never actually changed (the socket just died from process suspension), no .online event arrives and the client stays dead permanently.

Root Cause

Reconnection after connection abort depends on NWPathMonitor firing .online:

case (NSPOSIXErrorDomain.self, Int(ECONNABORTED)):
    subject.send(.error(WebSocketClient.Error.connectionLost))
    Task { await self?.networkMonitor.updateState(.offline) }

This pushes .offline to the network monitor scan, but reconnection only happens when NWPathMonitor fires .online to produce (.offline, .online). If the network path never changed (process suspension doesn't change the network path), no .online arrives and the client is permanently stuck.

Fix

File: WebSocketClient.swift

Instead of going through the network monitor, directly cancel the stale connection and emit .disconnected(.invalid, nil). This triggers the existing auto-retry mechanism (retryOnCloseCode(.invalid)) which creates a new connection — no dependency on NWPathMonitor.

case (NSPOSIXErrorDomain.self, Int(ECONNABORTED)),
     (NSPOSIXErrorDomain.self, 57):
    subject.send(.error(WebSocketClient.Error.connectionLost))
    Task { [weak self] in
        await connection?.cancel(with: .invalid, reason: nil)
        subject.send(.disconnected(.invalid, nil))
    }

Verification

Unit test — AWSPluginsCoreTests/WebSocketClientTests (1 new test)

Test What it verifies
testWebSocketClient_whenConnectionAborts_shouldEmitDisconnectedAndReconnect Connection abort emits .disconnected and auto-retry reconnects

Test plan

swift test --filter "WebSocketClientTests"  # 9/9 pass (8 existing + 1 new, no regressions)

@thisisabhash
thisisabhash requested a review from a team as a code owner May 11, 2026 22:09
@codecov

codecov Bot commented May 11, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 50.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.87%. Comparing base (7428f64) to head (6699e47).
⚠️ Report is 41 commits behind head on main.

Files with missing lines Patch % Lines
...ore/AWSPluginsCore/WebSocket/WebSocketClient.swift 50.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4222      +/-   ##
==========================================
+ Coverage   66.72%   66.87%   +0.14%     
==========================================
  Files        1151     1151              
  Lines       43817    43796      -21     
==========================================
+ Hits        29239    29289      +50     
+ Misses      14578    14507      -71     
Flag Coverage Δ
API_plugin_unit_test 68.33% <ø> (-0.07%) ⬇️
AWSPluginsCore 68.05% <50.00%> (-0.43%) ⬇️
Amplify 47.47% <ø> (ø)
Amplify_Foundation_Bridge_unit_test 62.28% <ø> (ø)
Amplify_Foundation_unit_test 67.64% <ø> (ø)
Analytics_plugin_unit_test 83.43% <ø> (ø)
Auth_plugin_unit_test 72.30% <ø> (+0.01%) ⬆️
DataStore_plugin_unit_test 82.71% <ø> (+1.01%) ⬆️
Firehose_plugin_unit_test 53.15% <ø> (ø)
Geo_plugin_unit_test 73.39% <ø> (ø)
Kinesis_plugin_unit_test 52.17% <ø> (ø)
Logging_plugin_unit_test 64.86% <ø> (ø)
Predictions_plugin_unit_test 33.89% <ø> (ø)
PushNotifications_plugin_unit_test 85.66% <ø> (ø)
RecordCache_unit_test 76.40% <ø> (ø)
Storage_plugin_unit_test 78.67% <ø> (ø)
unit_tests 66.87% <50.00%> (+0.14%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

jvh-aws
jvh-aws previously approved these changes Jun 1, 2026

@jvh-aws jvh-aws left a comment

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.

nit: can we run swiftformat?

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.

GraphQL subscriptions can get stuck in connecting after repeated iOS background/foreground transitions

2 participants