diff --git a/Bitkit/AppScene.swift b/Bitkit/AppScene.swift index 328c244d9..5b229a06d 100644 --- a/Bitkit/AppScene.swift +++ b/Bitkit/AppScene.swift @@ -249,6 +249,19 @@ struct AppScene: View { @State private var didWalletBackupRestoreFail = false @State private var isPinVerified: Bool = false @State private var showRecoveryScreen = false + /// Lets only a return from the background retry a failed node start, not a brief inactive phase. + @State private var foregroundReturnTracker = ForegroundReturnTracker() + + private var nodeRestarter: NodeRestarter { + NodeRestarter( + nodeState: { wallet.nodeLifecycleState }, + isConnected: { network.isConnected }, + walletExists: { wallet.walletExists }, + isRecoveryShown: { showRecoveryScreen }, + start: { playsErrorHaptic in await startWallet(playsErrorHaptic: playsErrorHaptic) }, + stop: { try await wallet.stopLightningNode() } + ) + } /// Check if there's a critical update available private var hasCriticalUpdate: Bool { @@ -765,7 +778,7 @@ struct AppScene: View { } } - private func startWallet(completingBackupRestore: Bool = false) async { + private func startWallet(completingBackupRestore: Bool = false, playsErrorHaptic: Bool = true) async { let hasPendingRestore = BackupService.shared.hasPendingWalletRestore() guard !WalletBackupRestoreGate.blocksWalletStart( isRestoreRunning: isWalletBackupRestoreRunning, @@ -806,7 +819,9 @@ struct AppScene: View { await BackupService.shared.scheduleFullBackup() } catch { Logger.error(error, context: "Failed to start wallet") - Haptics.notify(.error) + if playsErrorHaptic { + Haptics.notify(.error) + } if MigrationsService.shared.isShowingMigrationLoading { await MainActor.run { @@ -1050,6 +1065,8 @@ struct AppScene: View { private func handleScenePhaseChange(_ newPhase: ScenePhase) { Logger.info("Scene phase changed: \(newPhase)", context: "AppScene") + let returnedFromBackground = foregroundReturnTracker.scenePhaseChanged(to: newPhase) + if newPhase == .background { if settings.pinEnabled { // If PIN is enabled, lock the app when the app goes to the background @@ -1070,6 +1087,7 @@ struct AppScene: View { if retryPendingWalletRestoreIfNeeded() { return } + nodeRestarter.retryOnForeground(returnedFromBackground: returnedFromBackground) Task { if pubkyProfile.isInitialized { await pubkyProfile.checkAdoptedSource() @@ -1561,10 +1579,7 @@ struct AppScene: View { // Restart node if necessary (e.g. create/restore was skipped due to offline) switch wallet.nodeLifecycleState { case .stopped, .initializing, .errorStarting: - Logger.info("Network restored, retrying wallet start...", context: "AppScene") - Task { - await startWallet() - } + nodeRestarter.restart(reason: "Network restored") default: break } diff --git a/Bitkit/Utilities/NodeRestarter.swift b/Bitkit/Utilities/NodeRestarter.swift new file mode 100644 index 000000000..031a51b6d --- /dev/null +++ b/Bitkit/Utilities/NodeRestarter.swift @@ -0,0 +1,68 @@ +import SwiftUI + +/// Tells a return from the background apart from a brief `.inactive` phase (Control Center, notification shade, +/// Face ID prompt) that never left the foreground. +struct ForegroundReturnTracker { + private var wasInBackground = false + + /// Records the phase and returns true when it is an `.active` phase that follows a `.background` one. + mutating func scenePhaseChanged(to phase: ScenePhase) -> Bool { + switch phase { + case .background: + wasInBackground = true + return false + case .active: + defer { wasInBackground = false } + return wasInBackground + default: + return false + } + } +} + +/// Restarts a node that failed to start on behalf of the app lifecycle, without leaving it running under recovery mode. +@MainActor +struct NodeRestarter { + var nodeState: () -> NodeLifecycleState + var isConnected: () -> Bool + var walletExists: () -> Bool? + var isRecoveryShown: () -> Bool + /// Starts the wallet; the flag says whether a failed start plays the error haptic. + var start: (_ playsErrorHaptic: Bool) async -> Void + var stop: () async throws -> Void + + /// Schedules a restart when the app returned from the background while a wallet exists, the network is connected, + /// the node is in the error-starting state and the Recovery screen is not shown. + @discardableResult + func retryOnForeground(returnedFromBackground: Bool) -> Task? { + guard returnedFromBackground, isConnected(), walletExists() == true, !isRecoveryShown(), case .errorStarting = nodeState() else { + return nil + } + return restart(reason: "App returned to foreground") + } + + /// Starts the wallet unless Recovery is shown. A start the user did not trigger is silent on failure, and a node it + /// started while Recovery opened is stopped once the start completes. + @discardableResult + func restart(reason: String) -> Task { + Task { + // Checked when the task runs, because the Recovery quick action can be handled after the caller decided to restart. + guard !isRecoveryShown() else { + Logger.info("\(reason), skipping wallet start in recovery mode", context: "NodeRestarter") + return + } + Logger.info("\(reason), retrying wallet start...", context: "NodeRestarter") + await start(false) + + // Recovery can open while the start is in flight; stop the node this restart started instead of leaving it running. + if isRecoveryShown(), nodeState() == .running { + Logger.info("\(reason), stopping the node started while recovery mode opened", context: "NodeRestarter") + do { + try await stop() + } catch { + Logger.warn("Failed to stop the node under recovery mode: \(error)", context: "NodeRestarter") + } + } + } + } +} diff --git a/BitkitTests/NodeStartRetryTests.swift b/BitkitTests/NodeStartRetryTests.swift new file mode 100644 index 000000000..e704e0157 --- /dev/null +++ b/BitkitTests/NodeStartRetryTests.swift @@ -0,0 +1,159 @@ +@testable import Bitkit +import SwiftUI +import XCTest + +@MainActor +final class NodeStartRetryTests: XCTestCase { + private struct StartFailure: Error {} + + /// Stands in for the wallet, the network and the Recovery screen behind a `NodeRestarter`. + private final class Harness { + var state = NodeLifecycleState.errorStarting(cause: StartFailure()) + var isConnected = true + var walletExists: Bool? = true + var isRecoveryShown = false + var startHaptics: [Bool] = [] + var stopCalls = 0 + var stopError: Error? + /// Runs inside the start, to change the world while the start is in flight. + var duringStart: (() -> Void)? + + func makeRestarter() -> NodeRestarter { + NodeRestarter( + nodeState: { self.state }, + isConnected: { self.isConnected }, + walletExists: { self.walletExists }, + isRecoveryShown: { self.isRecoveryShown }, + start: { playsErrorHaptic in + self.startHaptics.append(playsErrorHaptic) + self.duringStart?() + }, + stop: { + self.stopCalls += 1 + if let error = self.stopError { + throw error + } + } + ) + } + } + + // MARK: Retry on returning to the foreground + + func testRetriesAFailedStartAfterReturningFromTheBackground() async { + let harness = Harness() + await harness.makeRestarter().retryOnForeground(returnedFromBackground: true)?.value + XCTAssertEqual(harness.startHaptics.count, 1) + } + + func testDoesNotRetryOnAnInactiveBlipThatNeverEnteredTheBackground() async { + let harness = Harness() + let task = harness.makeRestarter().retryOnForeground(returnedFromBackground: false) + await task?.value + XCTAssertNil(task) + XCTAssertTrue(harness.startHaptics.isEmpty) + } + + func testDoesNotRetryWhileOffline() async { + let harness = Harness() + harness.isConnected = false + await harness.makeRestarter().retryOnForeground(returnedFromBackground: true)?.value + XCTAssertTrue(harness.startHaptics.isEmpty) + } + + func testDoesNotRetryWithoutAWallet() async { + for walletExists in [false, nil] { + let harness = Harness() + harness.walletExists = walletExists + await harness.makeRestarter().retryOnForeground(returnedFromBackground: true)?.value + XCTAssertTrue(harness.startHaptics.isEmpty, "\(String(describing: walletExists))") + } + } + + func testDoesNotRetryWhileTheRecoveryScreenIsShown() async { + let harness = Harness() + harness.isRecoveryShown = true + await harness.makeRestarter().retryOnForeground(returnedFromBackground: true)?.value + XCTAssertTrue(harness.startHaptics.isEmpty) + } + + func testDoesNotRetryStatesThatAreNotAFailedStart() async { + for state in [NodeLifecycleState.stopped, .starting, .running, .stopping, .initializing] { + let harness = Harness() + harness.state = state + await harness.makeRestarter().retryOnForeground(returnedFromBackground: true)?.value + XCTAssertTrue(harness.startHaptics.isEmpty, "\(state)") + } + } + + // MARK: Error haptic + + func testRestartsTriggeredByTheLifecycleDoNotPlayTheErrorHaptic() async { + let foreground = Harness() + await foreground.makeRestarter().retryOnForeground(returnedFromBackground: true)?.value + XCTAssertEqual(foreground.startHaptics, [false]) + + let networkRestored = Harness() + await networkRestored.makeRestarter().restart(reason: "Network restored").value + XCTAssertEqual(networkRestored.startHaptics, [false]) + } + + // MARK: Recovery + + func testRestartDoesNotStartTheNodeWhileTheRecoveryScreenIsShown() async { + let harness = Harness() + harness.isRecoveryShown = true + await harness.makeRestarter().restart(reason: "Network restored").value + XCTAssertTrue(harness.startHaptics.isEmpty) + XCTAssertEqual(harness.stopCalls, 0) + } + + func testStopsANodeThatStartedWhileRecoveryOpened() async { + let harness = Harness() + harness.duringStart = { + harness.state = .running + harness.isRecoveryShown = true + } + await harness.makeRestarter().retryOnForeground(returnedFromBackground: true)?.value + XCTAssertEqual(harness.stopCalls, 1) + } + + func testLeavesTheNodeAloneWhenRecoveryOpenedButTheStartFailed() async { + let harness = Harness() + harness.duringStart = { harness.isRecoveryShown = true } + await harness.makeRestarter().restart(reason: "Network restored").value + XCTAssertEqual(harness.stopCalls, 0) + } + + func testLeavesARunningNodeAloneOutsideRecovery() async { + let harness = Harness() + harness.duringStart = { harness.state = .running } + await harness.makeRestarter().restart(reason: "Network restored").value + XCTAssertEqual(harness.stopCalls, 0) + } + + func testAFailedStopDoesNotEndTheRestartWithAnError() async { + let harness = Harness() + harness.stopError = StartFailure() + harness.duringStart = { + harness.state = .running + harness.isRecoveryShown = true + } + await harness.makeRestarter().restart(reason: "Network restored").value + XCTAssertEqual(harness.stopCalls, 1) + } + + // MARK: Scene phases + + func testOnlyAnActivePhaseAfterTheBackgroundIsAReturnFromTheBackground() { + var tracker = ForegroundReturnTracker() + XCTAssertFalse(tracker.scenePhaseChanged(to: .active)) + XCTAssertFalse(tracker.scenePhaseChanged(to: .inactive)) + XCTAssertFalse(tracker.scenePhaseChanged(to: .active), "an inactive blip is not a return") + XCTAssertFalse(tracker.scenePhaseChanged(to: .inactive)) + XCTAssertFalse(tracker.scenePhaseChanged(to: .background)) + XCTAssertFalse(tracker.scenePhaseChanged(to: .inactive)) + XCTAssertTrue(tracker.scenePhaseChanged(to: .active)) + XCTAssertFalse(tracker.scenePhaseChanged(to: .active), "the return counts once") + } +} diff --git a/changelog.d/next/848.fixed.md b/changelog.d/next/848.fixed.md new file mode 100644 index 000000000..7dda770d6 --- /dev/null +++ b/changelog.d/next/848.fixed.md @@ -0,0 +1 @@ +Fixed the wallet staying on "Connection issues" after a failed node start until the app was relaunched; it now retries when the app returns to the foreground.