From e02168854cafa047214de7516463f5df4599996e Mon Sep 17 00:00:00 2001 From: James Magahern Date: Sun, 3 May 2026 16:42:49 -0700 Subject: [PATCH] ios: better network handling --- .../Sybil/Sources/Sybil/SplitView.swift | 7 +- .../Sources/Sybil/SybilMarkdownTheme.swift | 2 +- .../Sources/Sybil/SybilPhoneShellView.swift | 7 +- .../Sybil/Sources/Sybil/SybilViewModel.swift | 196 ++++++++++++++++-- .../Sybil/Tests/SybilTests/SybilTests.swift | 104 ++++++++++ 5 files changed, 292 insertions(+), 24 deletions(-) diff --git a/ios/Packages/Sybil/Sources/Sybil/SplitView.swift b/ios/Packages/Sybil/Sources/Sybil/SplitView.swift index 7d187c2..ad8969f 100644 --- a/ios/Packages/Sybil/Sources/Sybil/SplitView.swift +++ b/ios/Packages/Sybil/Sources/Sybil/SplitView.swift @@ -72,19 +72,22 @@ public struct SplitView: View { switch nextPhase { case .background: shouldRefreshOnForeground = true + viewModel.markAppInactiveForNetwork() case .active: + viewModel.markAppActiveForNetwork() guard shouldRefreshOnForeground, horizontalSizeClass != .compact else { return } shouldRefreshOnForeground = false Task { - await viewModel.refreshVisibleContent( + await viewModel.refreshAfterAppBecameActive( refreshCollections: true, refreshSelection: viewModel.hasRefreshableSelection ) } case .inactive: - break + shouldRefreshOnForeground = true + viewModel.markAppInactiveForNetwork() @unknown default: break } diff --git a/ios/Packages/Sybil/Sources/Sybil/SybilMarkdownTheme.swift b/ios/Packages/Sybil/Sources/Sybil/SybilMarkdownTheme.swift index db10a5b..369bed8 100644 --- a/ios/Packages/Sybil/Sources/Sybil/SybilMarkdownTheme.swift +++ b/ios/Packages/Sybil/Sources/Sybil/SybilMarkdownTheme.swift @@ -85,7 +85,7 @@ extension Theme { .paragraph { configuration in configuration.label .fixedSize(horizontal: false, vertical: true) - .relativeLineSpacing(.em(0.36)) + .relativeLineSpacing(.em(0.46)) .markdownMargin(top: .zero, bottom: .em(0.82)) } .blockquote { configuration in diff --git a/ios/Packages/Sybil/Sources/Sybil/SybilPhoneShellView.swift b/ios/Packages/Sybil/Sources/Sybil/SybilPhoneShellView.swift index 7e5ae7f..90c1527 100644 --- a/ios/Packages/Sybil/Sources/Sybil/SybilPhoneShellView.swift +++ b/ios/Packages/Sybil/Sources/Sybil/SybilPhoneShellView.swift @@ -51,19 +51,22 @@ struct SybilPhoneShellView: View { switch nextPhase { case .background: shouldRefreshOnForeground = true + viewModel.markAppInactiveForNetwork() case .active: + viewModel.markAppActiveForNetwork() guard shouldRefreshOnForeground else { return } shouldRefreshOnForeground = false Task { - await viewModel.refreshVisibleContent( + await viewModel.refreshAfterAppBecameActive( refreshCollections: path.isEmpty, refreshSelection: !path.isEmpty && viewModel.hasRefreshableSelection ) } case .inactive: - break + shouldRefreshOnForeground = true + viewModel.markAppInactiveForNetwork() @unknown default: break } diff --git a/ios/Packages/Sybil/Sources/Sybil/SybilViewModel.swift b/ios/Packages/Sybil/Sources/Sybil/SybilViewModel.swift index 7c1ed46..8cdafa7 100644 --- a/ios/Packages/Sybil/Sources/Sybil/SybilViewModel.swift +++ b/ios/Packages/Sybil/Sources/Sybil/SybilViewModel.swift @@ -104,6 +104,10 @@ final class SybilViewModel { @ObservationIgnored private var chatBackgroundTask: SybilBackgroundTaskAssertion? @ObservationIgnored + private var isAppActive = true + @ObservationIgnored + private var appLifecycleGeneration = 0 + @ObservationIgnored private let clientFactory: (APIConfiguration) -> any SybilAPIClienting private let fallbackModels: [Provider: [String]] = [ @@ -502,6 +506,38 @@ final class SybilViewModel { await reconnect() } + func markAppInactiveForNetwork() { + guard isAppActive else { + return + } + + isAppActive = false + appLifecycleGeneration += 1 + SybilLog.debug(SybilLog.app, "App became inactive for network lifecycle generation \(appLifecycleGeneration)") + } + + func markAppActiveForNetwork() { + isAppActive = true + } + + func refreshAfterAppBecameActive(refreshCollections shouldRefreshCollections: Bool, refreshSelection shouldRefreshSelection: Bool) async { + markAppActiveForNetwork() + + guard isAuthenticated, !isCheckingSession else { + return + } + + guard shouldRefreshCollections || shouldRefreshSelection else { + return + } + + try? await Task.sleep(for: .milliseconds(150)) + await refreshVisibleContent( + refreshCollections: shouldRefreshCollections, + refreshSelection: shouldRefreshSelection + ) + } + func refreshVisibleContent(refreshCollections shouldRefreshCollections: Bool, refreshSelection shouldRefreshSelection: Bool) async { guard isAuthenticated, !isCheckingSession else { return @@ -729,6 +765,7 @@ final class SybilViewModel { SybilLog.app, "Refreshed collections: \(nextChats.count) chats, \(nextSearches.count) searches" ) + errorMessage = nil if case .settings = selectedItem { isLoadingCollections = false @@ -749,8 +786,12 @@ final class SybilViewModel { await refreshSelectionIfNeeded() } } catch { - errorMessage = normalizeAPIError(error) - SybilLog.error(SybilLog.app, "Refresh collections failed", error: error) + if shouldSuppressInactiveTransportError(error) { + SybilLog.info(SybilLog.app, "Suppressing collection refresh transport interruption while app is inactive") + } else { + errorMessage = normalizeAPIError(error) + SybilLog.error(SybilLog.app, "Refresh collections failed", error: error) + } } isLoadingCollections = false @@ -789,9 +830,12 @@ final class SybilViewModel { case .settings: break } + errorMessage = nil } catch { if isCancellation(error) { SybilLog.debug(SybilLog.app, "Selection refresh cancelled for \(selectedItem.id)") + } else if shouldSuppressInactiveTransportError(error) { + SybilLog.info(SybilLog.app, "Suppressing selection refresh transport interruption while app is inactive") } else { errorMessage = normalizeAPIError(error) SybilLog.error(SybilLog.app, "Selection refresh failed", error: error) @@ -882,6 +926,8 @@ final class SybilViewModel { } + [CompletionRequestMessage(role: .user, content: content, attachments: attachments.isEmpty ? nil : attachments)] let streamStatus = CompletionStreamStatus() + let streamLifecycleGeneration = appLifecycleGeneration + let streamStartedWhileInactive = !isAppActive if isUntitledChat(chatID: chatID, detail: selectedChat) { Task { [weak self] in @@ -917,24 +963,63 @@ final class SybilViewModel { chatBackgroundTask = nil } - try await client.runCompletionStream( - body: CompletionStreamRequest( - chatId: chatID, - provider: provider, - model: selectedModel, - messages: requestMessages - ) - ) { [weak self] event in - guard let self else { return } - await self.applyCompletionEvent(event, streamStatus: streamStatus) + do { + try await client.runCompletionStream( + body: CompletionStreamRequest( + chatId: chatID, + provider: provider, + model: selectedModel, + messages: requestMessages + ) + ) { [weak self] event in + guard let self else { return } + await self.applyCompletionEvent(event, streamStatus: streamStatus) + } + } catch { + if shouldSuppressLifecycleTransportError( + error, + startedAt: streamLifecycleGeneration, + startedWhileInactive: streamStartedWhileInactive + ) { + SybilLog.info(SybilLog.app, "Suppressing chat stream transport interruption after app lifecycle change") + pendingChatState = nil + if isAppActive { + await refreshInterruptedStream(preferredSelection: .chat(chatID)) + } + return + } + + throw error } if let streamError = await streamStatus.error() { throw APIError.httpError(statusCode: 502, message: streamError) } + guard isAppActive else { + pendingChatState = nil + return + } + await refreshCollections(preferredSelection: .chat(chatID)) - selectedChat = try await client.getChat(chatID: chatID) + do { + selectedChat = try await client.getChat(chatID: chatID) + } catch { + if shouldSuppressLifecycleTransportError( + error, + startedAt: streamLifecycleGeneration, + startedWhileInactive: streamStartedWhileInactive + ) { + SybilLog.info(SybilLog.app, "Suppressing chat refresh transport interruption after app lifecycle change") + pendingChatState = nil + if isAppActive { + await refreshInterruptedStream(preferredSelection: .chat(chatID)) + } + return + } + + throw error + } pendingChatState = nil } @@ -1003,19 +1088,41 @@ final class SybilViewModel { ) let streamStatus = SearchStreamStatus() + let streamLifecycleGeneration = appLifecycleGeneration + let streamStartedWhileInactive = !isAppActive - try await client.runSearchStream( - searchID: searchID, - body: SearchRunRequest(query: query, title: String(query.prefix(80)), type: "auto", numResults: 10) - ) { [weak self] event in - guard let self else { return } - await self.applySearchEvent(event, searchID: searchID, streamStatus: streamStatus) + do { + try await client.runSearchStream( + searchID: searchID, + body: SearchRunRequest(query: query, title: String(query.prefix(80)), type: "auto", numResults: 10) + ) { [weak self] event in + guard let self else { return } + await self.applySearchEvent(event, searchID: searchID, streamStatus: streamStatus) + } + } catch { + if shouldSuppressLifecycleTransportError( + error, + startedAt: streamLifecycleGeneration, + startedWhileInactive: streamStartedWhileInactive + ) { + SybilLog.info(SybilLog.app, "Suppressing search stream transport interruption after app lifecycle change") + if isAppActive { + await refreshInterruptedStream(preferredSelection: .search(searchID)) + } + return + } + + throw error } if let streamError = await streamStatus.error() { throw APIError.httpError(statusCode: 502, message: streamError) } + guard isAppActive else { + return + } + await refreshCollections(preferredSelection: .search(searchID)) } @@ -1250,6 +1357,57 @@ final class SybilViewModel { #endif } + private func refreshInterruptedStream(preferredSelection: SidebarSelection) async { + try? await Task.sleep(for: .milliseconds(150)) + await refreshCollections(preferredSelection: preferredSelection) + } + + private func shouldSuppressLifecycleTransportError( + _ error: Error, + startedAt generation: Int, + startedWhileInactive: Bool + ) -> Bool { + guard generation != appLifecycleGeneration || startedWhileInactive || !isAppActive else { + return false + } + + return isTransientTransportInterruption(error) + } + + private func shouldSuppressInactiveTransportError(_ error: Error) -> Bool { + !isAppActive && isTransientTransportInterruption(error) + } + + private func isTransientTransportInterruption(_ error: Error) -> Bool { + if isCancellation(error) { + return true + } + + if let apiError = error as? APIError, + case let .networkError(message) = apiError { + let lowercased = message.lowercased() + return lowercased.contains("network error -999") + || lowercased.contains("network error -1005") + || lowercased.contains("network connection was lost") + || lowercased.contains("software caused connection abort") + || lowercased.contains("socket is not connected") + } + + let nsError = error as NSError + if nsError.domain == NSURLErrorDomain { + return nsError.code == URLError.cancelled.rawValue + || nsError.code == URLError.networkConnectionLost.rawValue + || nsError.code == URLError.notConnectedToInternet.rawValue + || nsError.code == URLError.timedOut.rawValue + } + + if nsError.domain == NSPOSIXErrorDomain { + return nsError.code == 53 || nsError.code == 57 + } + + return false + } + private func isCancellation(_ error: Error) -> Bool { if error is CancellationError { return true diff --git a/ios/Packages/Sybil/Tests/SybilTests/SybilTests.swift b/ios/Packages/Sybil/Tests/SybilTests/SybilTests.swift index 9bb6634..748161a 100644 --- a/ios/Packages/Sybil/Tests/SybilTests/SybilTests.swift +++ b/ios/Packages/Sybil/Tests/SybilTests/SybilTests.swift @@ -19,6 +19,10 @@ private actor MockSybilClient: SybilAPIClienting { private let searchDetails: [String: SearchDetail] private var snapshot = MockClientCallSnapshot() + private var completionStreamNetworkErrorMessage: String? + private var completionStreamDelayNanoseconds: UInt64 = 0 + private var searchStreamNetworkErrorMessage: String? + private var searchStreamDelayNanoseconds: UInt64 = 0 init( chatsResponse: [ChatSummary] = [], @@ -36,6 +40,16 @@ private actor MockSybilClient: SybilAPIClienting { snapshot } + func setCompletionStreamNetworkError(_ message: String, delayNanoseconds: UInt64 = 0) { + completionStreamNetworkErrorMessage = message + completionStreamDelayNanoseconds = delayNanoseconds + } + + func setSearchStreamNetworkError(_ message: String, delayNanoseconds: UInt64 = 0) { + searchStreamNetworkErrorMessage = message + searchStreamDelayNanoseconds = delayNanoseconds + } + func verifySession() async throws -> AuthSession { AuthSession(authenticated: true, mode: "open") } @@ -98,6 +112,12 @@ private actor MockSybilClient: SybilAPIClienting { body: CompletionStreamRequest, onEvent: @escaping @Sendable (CompletionStreamEvent) async -> Void ) async throws { + if completionStreamDelayNanoseconds > 0 { + try await Task.sleep(nanoseconds: completionStreamDelayNanoseconds) + } + if let completionStreamNetworkErrorMessage { + throw APIError.networkError(message: completionStreamNetworkErrorMessage) + } throw UnexpectedClientCall() } @@ -106,6 +126,12 @@ private actor MockSybilClient: SybilAPIClienting { body: SearchRunRequest, onEvent: @escaping @Sendable (SearchStreamEvent) async -> Void ) async throws { + if searchStreamDelayNanoseconds > 0 { + try await Task.sleep(nanoseconds: searchStreamDelayNanoseconds) + } + if let searchStreamNetworkErrorMessage { + throw APIError.networkError(message: searchStreamNetworkErrorMessage) + } throw UnexpectedClientCall() } } @@ -267,6 +293,84 @@ private func makeSearchDetail(id: String, date: Date, answer: String) -> SearchD #expect(viewModel.selectedSearch?.answerText == "fresh answer") } +@MainActor +@Test func backgroundChatStreamInterruptionIsSuppressedUntilForegroundRefresh() async throws { + let date = Date(timeIntervalSince1970: 1_700_000_300) + let chat = makeChatSummary(id: "chat-3", date: date) + let initialDetail = makeChatDetail(id: "chat-3", date: date, body: "stale transcript") + let refreshedDetail = makeChatDetail(id: "chat-3", date: date, body: "fresh transcript") + let client = MockSybilClient( + chatsResponse: [chat], + chatDetails: ["chat-3": refreshedDetail] + ) + await client.setCompletionStreamNetworkError( + "Network error -1005 while requesting POST: The network connection was lost.", + delayNanoseconds: 50_000_000 + ) + let viewModel = SybilViewModel(settings: testSettings(named: #function)) { _ in client } + viewModel.isAuthenticated = true + viewModel.isCheckingSession = false + viewModel.selectedItem = .chat("chat-3") + viewModel.selectedChat = initialDetail + viewModel.composer = "continue" + + let sendTask = Task { + await viewModel.sendComposer() + } + try await Task.sleep(nanoseconds: 10_000_000) + viewModel.markAppInactiveForNetwork() + await sendTask.value + + #expect(viewModel.errorMessage == nil) + #expect(viewModel.composer.isEmpty) + #expect(!viewModel.isSending) + #expect(viewModel.selectedChat?.messages.first?.content == "stale transcript") + + await viewModel.refreshAfterAppBecameActive(refreshCollections: false, refreshSelection: true) + + let snapshot = await client.currentSnapshot() + #expect(snapshot.getChat == 1) + #expect(viewModel.errorMessage == nil) + #expect(viewModel.selectedChat?.messages.first?.content == "fresh transcript") +} + +@MainActor +@Test func backgroundSearchStreamInterruptionIsSuppressedUntilForegroundRefresh() async throws { + let date = Date(timeIntervalSince1970: 1_700_000_400) + let refreshedDetail = makeSearchDetail(id: "search-3", date: date, answer: "fresh answer") + let client = MockSybilClient( + searchDetails: ["search-3": refreshedDetail] + ) + await client.setSearchStreamNetworkError( + "Network error -1005 while requesting POST: The network connection was lost.", + delayNanoseconds: 50_000_000 + ) + let viewModel = SybilViewModel(settings: testSettings(named: #function)) { _ in client } + viewModel.isAuthenticated = true + viewModel.isCheckingSession = false + viewModel.selectedItem = .search("search-3") + viewModel.selectedSearch = makeSearchDetail(id: "search-3", date: date, answer: "stale answer") + viewModel.composer = "refresh me" + + let sendTask = Task { + await viewModel.sendComposer() + } + try await Task.sleep(nanoseconds: 10_000_000) + viewModel.markAppInactiveForNetwork() + await sendTask.value + + #expect(viewModel.errorMessage == nil) + #expect(viewModel.composer.isEmpty) + #expect(!viewModel.isSending) + + await viewModel.refreshAfterAppBecameActive(refreshCollections: false, refreshSelection: true) + + let snapshot = await client.currentSnapshot() + #expect(snapshot.getSearch == 1) + #expect(viewModel.errorMessage == nil) + #expect(viewModel.selectedSearch?.answerText == "fresh answer") +} + @Test func newChatSwipeMetricsClampProgressAndLatch() async throws { let width: CGFloat = 390 let maxTravel = NewChatSwipeMetrics.maxTravel(for: width)