From d263e2c7f7126b9a5caf1507140bb32e2c4e0ea3 Mon Sep 17 00:00:00 2001 From: Tom Brow Date: Thu, 29 Oct 2026 17:34:12 -0700 Subject: [PATCH] fix(mobile): resolve iOS pushes on a fresh connection and retry a stale one (#8722) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary iOS users were seeing a run of push notifications that just said "Reconnect to your relay now". That is the push gateway's fixed APNs body. It shows when the Notification Service Extension (NSE) can't fetch the message from the relay. On-device logs from one affected iPhone over a full day showed 12 of 385 NSE runs fail to resolve. Each failed relay `/query`: 10 hit the 8 second timeout and 2 got `NSURLErrorNetworkConnectionLost` (-1005). All 12 ran on a reused pooled connection from `URLSession.shared` in a long-lived NSE process. That process lived for hours while the network path changed between VPN and cellular. Of the fresh connections, 0 of 473 failed. This change: - Gives each notification its own ephemeral session, so it never inherits a pooled connection. The session is cancelled when the extension finishes or expires. - Retries `/query` once if it times out or loses its connection mid-request. The retry uses a new session and a freshly signed NIP-98 proof, because the relay rejects a replayed proof. Other failures (HTTP errors, offline, cancellation) are not retried. The retry is bounded to one attempt, so the worst case is about 16 seconds, inside the NSE's roughly 30 second budget. - Replaces the gateway body with "You have a new message." when the NSE can't resolve the push. The restricted fallback ("Open Buzz to view this message.") is unchanged, and so is the gateway payload. ### Related issue N/A. Reported in Buzz #buzz-mobile. ### Validation - Added tests in `BuzzPushNotificationResolverTests`: - A timeout or -1005 is retried once, on a new session and with a different `Authorization` proof, and resolves. - Repeated timeouts stop after 2 attempts. - `notConnectedToInternet` is not retried. - The retry is skipped if the account key changed during the first attempt. - `cancel()` (called when the NSE finishes or expires) stops a pending retry. - Each new test fails when the behavior it covers is removed: the retry, the new session, the account recheck, or the cancel guard. - The full BuzzPushKit `swift test` suite passes. - A debug simulator build of Runner and the NotificationService extension succeeded. - An agent review of the diff found no blockers. Its suggestions to create the session per `didReceive` and to cancel it on expiry are included. - Limitations: - The NSE target has no unit tests, so the fallback text and the session lifecycle are covered only by build and review. - I couldn't send a real push through the NSE. Test phones run a personal-team bundle that isn't the gateway's APNs topic. In the simulator, `simctl push` needs notification permission, which the app only requests after joining a community. - Under the mobile testing exception, the human-test step does not apply. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: Tom Brow Co-authored-by: Tom Brow Co-authored-by: studio Co-authored-by: Claude Opus 5.5 --- .../BuzzPushNotificationResolver.swift | 88 ++++++++++++++--- .../BuzzPushNotificationResolverTests.swift | 98 ++++++++++++++++++- .../NotificationService.swift | 22 ++++- 3 files changed, 187 insertions(+), 21 deletions(-) diff --git a/mobile/ios/BuzzPushKit/Sources/BuzzPushKit/BuzzPushNotificationResolver.swift b/mobile/ios/BuzzPushKit/Sources/BuzzPushKit/BuzzPushNotificationResolver.swift index 1428d3fa5f..989693c010 100644 --- a/mobile/ios/BuzzPushKit/Sources/BuzzPushKit/BuzzPushNotificationResolver.swift +++ b/mobile/ios/BuzzPushKit/Sources/BuzzPushKit/BuzzPushNotificationResolver.swift @@ -50,6 +50,8 @@ public struct BuzzPushResolution: Decodable, Equatable, Sendable { /// Resolves the content used to mutate a generic Buzz push notification. public protocol BuzzPushNotificationResolving { func resolve(completion: @escaping (BuzzPushResolution?) -> Void) + /// Cancels any retry in flight and prevents new ones. Call when the notification is delivered. + func cancel() } /// Reads configured Buzz communities and resolves their newest unread event. @@ -60,6 +62,12 @@ public final class BuzzPushNotificationResolver: BuzzPushNotificationResolving { static let maximumPresentationResponseBytes = 2 * 1_024 * 1_024 private let session: URLSession + private let makeRetrySession: () -> URLSession + // Recursive because a retry that fails synchronously completes while the lock is held, + // and that completion may finish the notification and call cancel(). + private let retryLock = NSRecursiveLock() + private var isCancelled = false + private var retrySessions: [URLSession] = [] private let loadCommunitiesData: () -> Data? private let loadPrivateKey: (String) -> String? private let now: () -> Date @@ -73,8 +81,10 @@ public final class BuzzPushNotificationResolver: BuzzPushNotificationResolving { loadPrivateKey: @escaping (String) -> String?, notificationCache: BuzzNotificationCache? = nil, avatarLoader: BuzzNotificationAvatarLoader? = nil, - now: @escaping () -> Date = Date.init + now: @escaping () -> Date = Date.init, + makeRetrySession: (() -> URLSession)? = nil ) { + self.makeRetrySession = makeRetrySession ?? { URLSession(configuration: session.configuration) } self.notificationCache = notificationCache self.avatarLoader = avatarLoader self.session = session @@ -154,21 +164,10 @@ public final class BuzzPushNotificationResolver: BuzzPushNotificationResolving { completion(nil) return } - var request = URLRequest(url: url) - request.httpMethod = "POST" - request.httpBody = body - request.timeoutInterval = 8 - request.setValue("application/json", forHTTPHeaderField: "Content-Type") - guard - let auth = try? NostrHTTPAuth.authorizationHeader( - url: url, method: "POST", body: body, privateKeyHex: privateKey - ) - else { - completion(nil) - return - } - request.setValue(auth, forHTTPHeaderField: "Authorization") - session.dataTask(with: request) { data, response, _ in + send( + url: url, body: body, privateKey: privateKey, session: session, + canRetry: { self.matchingPrivateKey(for: community) == privateKey } + ) { data, response in guard self.matchingPrivateKey(for: community) != nil, let response = response as? HTTPURLResponse, (200..<300).contains(response.statusCode), let data, let events = try? JSONDecoder().decode([VerifiedNostrEvent].self, from: data) @@ -186,6 +185,63 @@ public final class BuzzPushNotificationResolver: BuzzPushNotificationResolving { community: community ) completion(candidate.map { ($0, community) }) + } + } + + /// Sends one signed `/query`. A timeout or dropped connection is retried once on a new + /// session, because the failed attempt may have used a pooled connection that went stale + /// when the network path changed. Each attempt is signed afresh so the relay's NIP-98 + /// replay guard accepts it. `canRetry` rechecks the account first; nil means no retry. + public func cancel() { + retryLock.lock() + isCancelled = true + let sessions = retrySessions + retrySessions = [] + retryLock.unlock() + sessions.forEach { $0.invalidateAndCancel() } + } + + private func send( + url: URL, body: Data, privateKey: String, session: URLSession, canRetry: (() -> Bool)?, + completion: @escaping (Data?, URLResponse?) -> Void + ) { + var request = URLRequest(url: url) + request.httpMethod = "POST" + request.httpBody = body + request.timeoutInterval = 8 + request.setValue("application/json", forHTTPHeaderField: "Content-Type") + guard + let auth = try? NostrHTTPAuth.authorizationHeader( + url: url, method: "POST", body: body, privateKeyHex: privateKey + ) + else { + completion(nil, nil) + return + } + request.setValue(auth, forHTTPHeaderField: "Authorization") + session.dataTask(with: request) { data, response, error in + guard let error = error as? URLError, + [.timedOut, .networkConnectionLost].contains(error.code), canRetry?() == true + else { + completion(data, response) + return + } + // Creating the retry under the lock means cancel() either stops it or sees its session. + self.retryLock.lock() + guard !self.isCancelled else { + self.retryLock.unlock() + completion(data, response) + return + } + defer { self.retryLock.unlock() } + let retrySession = self.makeRetrySession() + self.retrySessions.append(retrySession) + self.send( + url: url, body: body, privateKey: privateKey, session: retrySession, canRetry: nil + ) { data, response in + retrySession.finishTasksAndInvalidate() + completion(data, response) + } }.resume() } diff --git a/mobile/ios/BuzzPushKit/Tests/BuzzPushKitTests/BuzzPushNotificationResolverTests.swift b/mobile/ios/BuzzPushKit/Tests/BuzzPushKitTests/BuzzPushNotificationResolverTests.swift index 366812bf91..f0df68a479 100644 --- a/mobile/ios/BuzzPushKit/Tests/BuzzPushKitTests/BuzzPushNotificationResolverTests.swift +++ b/mobile/ios/BuzzPushKit/Tests/BuzzPushKitTests/BuzzPushNotificationResolverTests.swift @@ -138,6 +138,98 @@ final class BuzzPushNotificationResolverTests: XCTestCase { XCTAssertEqual(URLProtocolStub.requests.count, 1) } + func testResolveRetriesStaleConnectionOnceWithFreshProof() throws { + let event = try JSONDecoder().decode( + VerifiedNostrEvent.self, + from: Data(Self.fixtureEvent.utf8) + ) + for code in [URLError.Code.timedOut, .networkConnectionLost] { + URLProtocolStub.reset() + var failed = false + URLProtocolStub.handler = { request in + if !failed { + failed = true + throw URLError(code) + } + return Self.response(request, status: 200, data: try JSONEncoder().encode([event])) + } + + var retrySessions = 0 + let result = try XCTUnwrap( + resolve( + makeResolver( + communitiesData: try snapshotData([community()]), + makeRetrySession: { + retrySessions += 1 + let configuration = URLSessionConfiguration.ephemeral + configuration.protocolClasses = [URLProtocolStub.self] + configuration.httpAdditionalHeaders = ["X-Test-Session": "retry"] + return URLSession(configuration: configuration) + })), "\(code)") + + XCTAssertEqual(result.body, "Hello Buzz") + XCTAssertEqual(retrySessions, 1) + let queries = URLProtocolStub.requests.prefix(2) + XCTAssertEqual(queries.map { $0.url?.path }, ["/query", "/query"]) + XCTAssertEqual( + queries.map { $0.value(forHTTPHeaderField: "X-Test-Session") }, [nil, "retry"], + "The retry must not reuse the session whose connection failed.") + XCTAssertNotEqual( + queries.first?.value(forHTTPHeaderField: "Authorization"), + queries.last?.value(forHTTPHeaderField: "Authorization"), + "The relay rejects a replayed NIP-98 proof.") + } + } + + func testResolveRetriesStaleConnectionOnlyOnce() throws { + URLProtocolStub.handler = { _ in throw URLError(.timedOut) } + + XCTAssertNil(resolve(makeResolver(communitiesData: try snapshotData([community()])))) + XCTAssertEqual(URLProtocolStub.requests.count, 2) + } + + func testResolveDoesNotRetryAfterAccountKeyChanges() throws { + let data = try snapshotData([community()]) + let lock = NSLock() + var key = Self.privateKey + let configuration = URLSessionConfiguration.ephemeral + configuration.protocolClasses = [URLProtocolStub.self] + URLProtocolStub.handler = { _ in + lock.lock() + key = Self.profilePrivateKey + lock.unlock() + throw URLError(.timedOut) + } + let resolver = BuzzPushNotificationResolver( + session: URLSession(configuration: configuration), loadCommunitiesData: { data }, + loadPrivateKey: { _ in + lock.lock() + defer { lock.unlock() } + return key + }) + + XCTAssertNil(resolve(resolver)) + XCTAssertEqual(URLProtocolStub.requests.count, 1) + } + + func testCancelStopsAPendingRetry() throws { + let resolver = makeResolver(communitiesData: try snapshotData([community()])) + URLProtocolStub.handler = { _ in + resolver.cancel() + throw URLError(.timedOut) + } + + XCTAssertNil(resolve(resolver)) + XCTAssertEqual(URLProtocolStub.requests.count, 1) + } + + func testResolveDoesNotRetryOtherTransportFailures() throws { + URLProtocolStub.handler = { _ in throw URLError(.notConnectedToInternet) } + + XCTAssertNil(resolve(makeResolver(communitiesData: try snapshotData([community()])))) + XCTAssertEqual(URLProtocolStub.requests.count, 1) + } + func testResolveReturnsNilForUndecodableRelayResponse() throws { URLProtocolStub.handler = { request in Self.response(request, status: 200, data: Data("not events".utf8)) @@ -898,7 +990,8 @@ final class BuzzPushNotificationResolverTests: XCTestCase { privateKeys: [String: String] = ["community-id": privateKey], presentationCacheData: Data? = nil, cacheOverride: BuzzNotificationCache? = nil, - now: Date = Date() + now: Date = Date(), + makeRetrySession: (() -> URLSession)? = nil ) -> BuzzPushNotificationResolver { let configuration = URLSessionConfiguration.ephemeral configuration.protocolClasses = [URLProtocolStub.self] @@ -907,7 +1000,8 @@ final class BuzzPushNotificationResolverTests: XCTestCase { loadCommunitiesData: { communitiesData }, loadPrivateKey: { privateKeys[$0] }, notificationCache: cacheOverride ?? makeNotificationCache(communitiesData: communitiesData, fixture: presentationCacheData, now: now), - now: { now } + now: { now }, + makeRetrySession: makeRetrySession ) } diff --git a/mobile/ios/NotificationService/NotificationService.swift b/mobile/ios/NotificationService/NotificationService.swift index c10bb0f3fd..84b6106009 100644 --- a/mobile/ios/NotificationService/NotificationService.swift +++ b/mobile/ios/NotificationService/NotificationService.swift @@ -34,13 +34,18 @@ final class NotificationService: UNNotificationServiceExtension { Bundle.main.object( forInfoDictionaryKey: "BuzzAppGroupIdentifier" ) as? String - private lazy var resolver: BuzzPushNotificationResolving = { + // A session per notification keeps a long-lived extension process from reusing + // connections that went stale when the network path changed (for example a VPN + // coming up or dropping). + private var session: URLSession? + private var resolver: BuzzPushNotificationResolving? + private func makeResolver(session: URLSession) -> BuzzPushNotificationResolving { let keychainAccessGroup = Bundle.main.object( forInfoDictionaryKey: "BuzzKeychainAccessGroup" ) as? String return BuzzPushNotificationResolver( - session: .shared, + session: session, loadCommunitiesData: { [self] in Self.loadPushSnapshotData(appGroupIdentifier: self.appGroupIdentifier) }, @@ -53,7 +58,7 @@ final class NotificationService: UNNotificationServiceExtension { notificationCache: notificationCache, avatarLoader: notificationAvatarLoader ) - }() + } private var notificationCacheRoot: URL? { FileManager.default.urls(for: .cachesDirectory, in: .userDomainMask).first? @@ -107,12 +112,19 @@ final class NotificationService: UNNotificationServiceExtension { } // Workaround until https://github.com/block/buzz/pull/8431 lands. if content.sound == nil { content.sound = .default } + // Shown if the relay can't be reached in time. The gateway's fixed body is meant + // to be replaced and reads like an instruction to the user. + content.body = "You have a new message." restrictedFallbackContent = Self.restrictedFallback(from: content) bestAttemptContent = content var cleanUserInfo = content.userInfo cleanUserInfo.removeValue(forKey: BuzzPushNavigationTarget.userInfoKey) content.userInfo = cleanUserInfo + let session = URLSession(configuration: .ephemeral) + self.session = session + let resolver = makeResolver(session: session) + self.resolver = resolver resolver.resolve { [weak self] resolution in guard let self else { return } if let resolution { @@ -161,6 +173,10 @@ final class NotificationService: UNNotificationServiceExtension { } private func finish(_ content: UNNotificationContent) { + // Cancelling on expiry also stops a retry that is running or about to start. + resolver?.cancel() + session?.invalidateAndCancel() + session = nil handoff?.finish( resolvedScope.map { isCurrent($0) } == false ? Self.restrictedFallback(from: content) : content,