From f9169334e669f3442fc545c35b7cbb982d2dd7a3 Mon Sep 17 00:00:00 2001 From: Adron Hall Date: Sun, 2 Aug 2026 11:01:07 -0700 Subject: [PATCH] fix(dm): stop showing "Network error: cancelled" on message send MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A cancelled URLSession request (URLError.cancelled, thrown when a SwiftUI .task is torn down on navigation/teardown) was flattened at the APIClient boundary into APIError.transport(message: "cancelled") and rendered as a red "Network error: cancelled" banner — even though the message actually sent. The banner was also sticky: only a successful load/send cleared it, while the poll loop that keeps the thread live swallowed errors and never cleared it. Fixes: - APIClient.performOnce: normalise URLError(.cancelled)/CancellationError to CancellationError instead of laundering it into a transport error, so the cancellation signal survives the boundary. Genuine transport failures are unchanged. - DMThreadViewModel.load()/send(), DirectMessagesListViewModel.load()/ loadMore(), NewMessageViewModel.send(): ignore CancellationError instead of pinning it to the visible error. - DMThreadViewModel.pollOnce(): clear a stale error on a successful poll so a one-off cancellation self-heals. Tests: - InterlinedKit APIClientTests: a cancelled request throws CancellationError, never APIError.transport. - App DMThreadViewModelTests: cancelled load leaves error nil / not-loaded; cancelled send restores the draft without a banner; a successful poll clears a stale error. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../DirectMessages/DMThreadViewModel.swift | 15 ++++++ .../DirectMessagesListViewModel.swift | 5 ++ .../DirectMessages/NewMessageViewModel.swift | 4 ++ AppTests/DMThreadViewModelTests.swift | 46 +++++++++++++++++++ .../InterlinedKit/APIClient/APIClient.swift | 13 ++++++ .../InterlinedKitTests/APIClientTests.swift | 18 ++++++++ 6 files changed, 101 insertions(+) diff --git a/App/Features/DirectMessages/DMThreadViewModel.swift b/App/Features/DirectMessages/DMThreadViewModel.swift index 91b3143..1cf7faf 100644 --- a/App/Features/DirectMessages/DMThreadViewModel.swift +++ b/App/Features/DirectMessages/DMThreadViewModel.swift @@ -162,6 +162,11 @@ final class DMThreadViewModel { apply(thread, replacing: true) error = nil hasLoadedOnce = true + } catch is CancellationError { + // The load was cancelled (view teardown / navigation), not + // failed. Leave `error` and `hasLoadedOnce` untouched so no + // banner shows and no "loaded" flags flip prematurely — the + // surviving poll (or a fresh load) repopulates the thread. } catch { self.error = error hasLoadedOnce = true @@ -218,6 +223,12 @@ final class DMThreadViewModel { } error = nil bus?.post(.messageSent(recipientUsername: username, message: sent)) + } catch is CancellationError { + // Cancelled mid-send — not a failure. Drop the optimistic bubble + // and restore the draft so the user can retry; a live poll + // reconciles any message that did reach the server. No banner. + messages.removeAll { $0.id == tempId } + draft = priorDraft } catch { messages.removeAll { $0.id == tempId } draft = priorDraft @@ -274,6 +285,10 @@ final class DMThreadViewModel { func pollOnce() async { do { let update = try await service.threadUpdates(username: username, since: newestId) + // A successful round-trip proves the thread is live — clear any + // stale error (e.g. a cancelled initial load) so a one-off + // cancellation banner self-heals instead of pinning forever. + error = nil guard !update.messages.isEmpty || update.otherUser != nil else { return } mergeUpdates(update) await markInboundRead() diff --git a/App/Features/DirectMessages/DirectMessagesListViewModel.swift b/App/Features/DirectMessages/DirectMessagesListViewModel.swift index 5041abd..49f39c2 100644 --- a/App/Features/DirectMessages/DirectMessagesListViewModel.swift +++ b/App/Features/DirectMessages/DirectMessagesListViewModel.swift @@ -129,6 +129,9 @@ final class DirectMessagesListViewModel { regroup() error = nil hasLoadedOnce = true + } catch is CancellationError { + // Cancelled (view teardown / folder switch superseded), not + // failed. Leave state untouched so no spurious error banner shows. } catch { self.error = error hasLoadedOnce = true @@ -150,6 +153,8 @@ final class DirectMessagesListViewModel { nextCursor = page.nextCursor regroup() error = nil + } catch is CancellationError { + // Cancelled pagination — not a failure; leave the list as-is. } catch { self.error = error } diff --git a/App/Features/DirectMessages/NewMessageViewModel.swift b/App/Features/DirectMessages/NewMessageViewModel.swift index 32c330a..c9df982 100644 --- a/App/Features/DirectMessages/NewMessageViewModel.swift +++ b/App/Features/DirectMessages/NewMessageViewModel.swift @@ -119,6 +119,10 @@ final class NewMessageViewModel { if let username = recipients.first(where: { $0.id == recipientId })?.username { bus?.post(.messageSent(recipientUsername: username, message: sent)) } + } catch is CancellationError { + // Cancelled mid-send — not a failure. Leave `error` nil so the + // sheet shows no spurious "Network error" banner; the user can + // retry (a persisted message reconciles via the list poll). } catch { self.error = error } diff --git a/AppTests/DMThreadViewModelTests.swift b/AppTests/DMThreadViewModelTests.swift index 70b5ef1..8db737f 100644 --- a/AppTests/DMThreadViewModelTests.swift +++ b/AppTests/DMThreadViewModelTests.swift @@ -181,6 +181,52 @@ final class DMThreadViewModelTests: XCTestCase { XCTAssertTrue(recorded.isEmpty, "An all-outbound thread marks nothing") } + // MARK: - Cancellation is not an error + + func test_givenLoadCancelled_whenLoading_thenLeavesErrorNilAndNotLoaded() async { + // A cancelled thread load (view teardown / navigation) surfaces as + // CancellationError from the client. It must not pin an error banner + // nor flip `hasLoadedOnce`, which would flash a premature "Not mutual". + let (vm, service, _) = makeViewModel() + await service.enqueueThread(failure: CancellationError()) + + await vm.load() + + XCTAssertNil(vm.error, "A cancelled load must not surface an error") + XCTAssertFalse(vm.hasLoadedOnce, "A cancelled load did not complete") + } + + func test_givenSendCancelled_whenSending_thenRestoresDraftWithoutError() async { + // A cancelled send drops the optimistic bubble and restores the draft + // (so the user can retry) but shows no error banner. + let (vm, service, _) = makeViewModel() + vm.seedForTest(messages: [inbound("m1", read: true, at: 100)], otherUser: ada, isMutual: true) + await service.enqueueSend(failure: CancellationError()) + + vm.draft = "hello" + await vm.send() + + XCTAssertEqual(vm.messages.map(\.id), ["m1"], "Optimistic placeholder removed on cancel") + XCTAssertEqual(vm.draft, "hello", "Draft restored for retry") + XCTAssertNil(vm.error, "A cancelled send must not surface an error") + } + + func test_givenStaleError_whenPollSucceeds_thenErrorIsCleared() async { + // A genuine earlier failure pins the banner; once a poll round-trips + // successfully the thread is proven live and the banner self-heals. + let (vm, service, _) = makeViewModel() + vm.seedForTest(messages: [inbound("m1", read: true, at: 100)], otherUser: ada, isMutual: true) + await service.enqueueSend(failure: TestError.upstream("offline")) + vm.draft = "hi" + await vm.send() + XCTAssertEqual(vm.error as? TestError, .upstream("offline"), "Precondition: banner is showing") + + await service.enqueueThreadUpdates(success: DMThread(messages: [], otherUser: ada, isMutual: true)) + await vm.pollOnce() + + XCTAssertNil(vm.error, "A successful poll clears the stale error banner") + } + // MARK: - Poll cycle + cancellation func test_givenPollCycle_whenNewMessageArrives_thenMergesIntoThread() async { diff --git a/Packages/InterlinedKit/Sources/InterlinedKit/APIClient/APIClient.swift b/Packages/InterlinedKit/Sources/InterlinedKit/APIClient/APIClient.swift index cedff0a..22484f6 100644 --- a/Packages/InterlinedKit/Sources/InterlinedKit/APIClient/APIClient.swift +++ b/Packages/InterlinedKit/Sources/InterlinedKit/APIClient/APIClient.swift @@ -206,6 +206,19 @@ public final class APIClient: APIClientProtocol { ) } catch let error as APIError { throw error + } catch is CancellationError { + // Cooperative task cancellation (a SwiftUI `.task` torn down on + // view teardown / navigation) is not a network failure. Propagate + // it as-is so callers can ignore it instead of surfacing a + // spurious "Network error: cancelled" banner. + throw CancellationError() + } catch let error as URLError where error.code == .cancelled { + // URLSession's async API reports task cancellation as + // `URLError(.cancelled)`. Normalise it to `CancellationError` so + // the `.cancelled` signal survives the boundary (it would + // otherwise be flattened into `.transport(message: "cancelled")` + // and be indistinguishable from a genuine transport failure). + throw CancellationError() } catch { throw APIError.transport(message: error.localizedDescription) } diff --git a/Packages/InterlinedKit/Tests/InterlinedKitTests/APIClientTests.swift b/Packages/InterlinedKit/Tests/InterlinedKitTests/APIClientTests.swift index 7f8678b..8d10a88 100644 --- a/Packages/InterlinedKit/Tests/InterlinedKitTests/APIClientTests.swift +++ b/Packages/InterlinedKit/Tests/InterlinedKitTests/APIClientTests.swift @@ -175,6 +175,24 @@ final class APIClientTests: XCTestCase { } } + func test_givenCancelledRequest_whenSent_thenThrowsCancellationErrorNotTransport() async throws { + // Given — URLSession's async API reports task cancellation (a SwiftUI + // `.task` torn down on navigation) as `URLError(.cancelled)`. + let (client, transport, _) = makeClient() + await transport.enqueueError(URLError(.cancelled)) + + // When / Then — it must surface as `CancellationError`, never as a + // user-facing `APIError.transport` ("Network error: cancelled"). + do { + _ = try await client.send(Request(method: .get, path: "/x", auth: .none)) + XCTFail("Expected cancellation") + } catch is CancellationError { + // Expected. + } catch let error as APIError { + XCTFail("Cancellation must not be mapped to APIError, got \(error)") + } + } + // MARK: - Query parameters func test_givenOptionalQueryItems_whenSent_thenSkipsNilParameters() async throws {