From cfafcffde229c1bf6b48a4df9002369ed409dbce Mon Sep 17 00:00:00 2001 From: alltheseas Date: Fri, 13 Feb 2026 22:59:24 -0600 Subject: [PATCH] fix: wait for relay connection before loading nevent URLs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit LoadableNostrEventViewModel.load() now calls awaitConnection() before executeLoadingLogic(), preventing premature "not found" when opening nevent URLs or search results before relays finish connecting. Closes: https://github.com/damus-io/damus/pull/3559 Signed-off-by: alltheseas Tested-by: Daniel D’Aquino Reviewed-by: Daniel D’Aquino --- damus.xcodeproj/project.pbxproj | 4 + .../Models/LoadableNostrEventView.swift | 29 ++--- .../LoadableNostrEventViewModelTests.swift | 104 ++++++++++++++++++ 3 files changed, 115 insertions(+), 22 deletions(-) create mode 100644 damusTests/LoadableNostrEventViewModelTests.swift diff --git a/damus.xcodeproj/project.pbxproj b/damus.xcodeproj/project.pbxproj index f3e25429..6baa5010 100644 --- a/damus.xcodeproj/project.pbxproj +++ b/damus.xcodeproj/project.pbxproj @@ -41,6 +41,7 @@ 3A92C0FF2DE16E9800CEEBAC /* FaviconCache.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3A92C0FD2DE16E9800CEEBAC /* FaviconCache.swift */; }; 3A92C1002DE16E9800CEEBAC /* FaviconCache.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3A92C0FD2DE16E9800CEEBAC /* FaviconCache.swift */; }; 3A92C1022DE17ACA00CEEBAC /* NIP05DomainTimelineHeaderViewTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3A92C1012DE17ACA00CEEBAC /* NIP05DomainTimelineHeaderViewTests.swift */; }; + EBCC3486DE53D8DB2532B98E /* LoadableNostrEventViewModelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = C0AE7EE3216D7983A50BE2D9 /* LoadableNostrEventViewModelTests.swift */; }; 3A96E3FE2D6BCE3800AE1630 /* RepostedTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3A96E3FD2D6BCE3800AE1630 /* RepostedTests.swift */; }; 3AA247FF297E3D900090C62D /* RepostsView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3AA247FE297E3D900090C62D /* RepostsView.swift */; }; 3AA24802297E3DC20090C62D /* RepostView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3AA24801297E3DC20090C62D /* RepostView.swift */; }; @@ -2081,6 +2082,7 @@ 3A929C22297F2CF80090925E /* it-IT */ = {isa = PBXFileReference; lastKnownFileType = text.plist.stringsdict; name = "it-IT"; path = "it-IT.lproj/Localizable.stringsdict"; sourceTree = ""; }; 3A92C0FD2DE16E9800CEEBAC /* FaviconCache.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FaviconCache.swift; sourceTree = ""; }; 3A92C1012DE17ACA00CEEBAC /* NIP05DomainTimelineHeaderViewTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NIP05DomainTimelineHeaderViewTests.swift; sourceTree = ""; }; + C0AE7EE3216D7983A50BE2D9 /* LoadableNostrEventViewModelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LoadableNostrEventViewModelTests.swift; sourceTree = ""; }; 3A93342929884CA600D6A8F3 /* pl-PL */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = "pl-PL"; path = "pl-PL.lproj/InfoPlist.strings"; sourceTree = ""; }; 3A93342A29884CA600D6A8F3 /* pl-PL */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = "pl-PL"; path = "pl-PL.lproj/Localizable.strings"; sourceTree = ""; }; 3A93342B29884CA600D6A8F3 /* pl-PL */ = {isa = PBXFileReference; lastKnownFileType = text.plist.stringsdict; name = "pl-PL"; path = "pl-PL.lproj/Localizable.stringsdict"; sourceTree = ""; }; @@ -3931,6 +3933,7 @@ 64D0A2B0F048CC8D494945E6 /* RepostNotificationTests.swift */, 4C0ED07E2D7A1E260020D8A2 /* Benchmarking.swift */, 3A92C1012DE17ACA00CEEBAC /* NIP05DomainTimelineHeaderViewTests.swift */, + C0AE7EE3216D7983A50BE2D9 /* LoadableNostrEventViewModelTests.swift */, ); path = damusTests; sourceTree = ""; @@ -6400,6 +6403,7 @@ 4C684A552A7E91FE005E6031 /* LargeEventTests.swift in Sources */, E02B54182B4DFADA0077FF42 /* Bech32ObjectTests.swift in Sources */, 3A92C1022DE17ACA00CEEBAC /* NIP05DomainTimelineHeaderViewTests.swift in Sources */, + EBCC3486DE53D8DB2532B98E /* LoadableNostrEventViewModelTests.swift in Sources */, ); runOnlyForDeploymentPostprocessing = 0; }; diff --git a/damus/Features/Events/Models/LoadableNostrEventView.swift b/damus/Features/Events/Models/LoadableNostrEventView.swift index 632c4aad..6eae587e 100644 --- a/damus/Features/Events/Models/LoadableNostrEventView.swift +++ b/damus/Features/Events/Models/LoadableNostrEventView.swift @@ -21,34 +21,19 @@ class LoadableNostrEventViewModel: ObservableObject { let damus_state: DamusState let note_reference: NoteReference @Published var state: ThreadModelLoadingState = .loading - /// The time period after which it will give up loading the view. - /// Written in nanoseconds - let TIMEOUT: UInt64 = 10 * 1_000_000_000 // 10 seconds - + init(damus_state: DamusState, note_reference: NoteReference) { self.damus_state = damus_state self.note_reference = note_reference Task { await self.load() } } - - /// Starts loading the referenced Nostr event and updates the view model's `state` with the result or a timeout outcome. - /// - /// This launches a dedicated task that runs the loading logic and a separate timeout task that cancels the loader after `TIMEOUT`. If the timeout fires, `state` is set to `.not_found`. If the load finishes first, the timeout task is cancelled. - func load() async { - // Start the loading process in a separate task to manage the timeout independently. - let loadTask = Task { @MainActor in - self.state = await executeLoadingLogic(note_reference: self.note_reference) - } - // Setup a timer to cancel the load after the timeout period - let timeoutTask = Task { @MainActor in - try await Task.sleep(nanoseconds: TIMEOUT) - loadTask.cancel() // This sends a cancellation signal to the load task. - self.state = .not_found - } - - await loadTask.value - timeoutTask.cancel() // Cancel the timeout task if loading finishes earlier. + /// Waits for relay connection then loads the referenced event. + /// + /// Timeout is handled by `awaitConnection()` (30 s built-in). + func load() async { + await damus_state.nostrNetwork.awaitConnection() + self.state = await executeLoadingLogic(note_reference: self.note_reference) } /// Loads the Nostr event identified by `noteId`, optionally restricting the lookup to specific relays. diff --git a/damusTests/LoadableNostrEventViewModelTests.swift b/damusTests/LoadableNostrEventViewModelTests.swift new file mode 100644 index 00000000..7be0819b --- /dev/null +++ b/damusTests/LoadableNostrEventViewModelTests.swift @@ -0,0 +1,104 @@ +// +// LoadableNostrEventViewModelTests.swift +// damusTests +// +// Created by alltheseas on 2026-02-13. +// + +import XCTest +@testable import damus + +/// Tests for LoadableNostrEventViewModel, verifying that event loading +/// waits for relay connection before attempting network lookups. +/// +/// ## Bug replication (issue #3544) +/// +/// Before the fix, `load()` called `executeLoadingLogic()` immediately +/// without waiting for relay connection. When the app opened a nevent URL +/// or search result before relays connected, `findEvent` would fail and +/// the view would show "not found." +/// +/// The observable difference: +/// +/// - **Old code** (no `awaitConnection`): `executeLoadingLogic` runs +/// immediately → `findEvent` hits empty ndb and disconnected relays → +/// returns nil → state becomes `.not_found` within milliseconds. +/// +/// - **Fixed code** (with `awaitConnection`): `load()` blocks at +/// `awaitConnection()` → state stays `.loading` until relays connect +/// or the 30 s timeout fires. +/// +/// The fix adds `awaitConnection()` before loading, matching the pattern +/// established in `SearchHomeModel.load()` (commit fa4b7a75). +@MainActor +final class LoadableNostrEventViewModelTests: XCTestCase { + + /// Proves the fix: without a relay connection, `load()` blocks at + /// `awaitConnection()` and state remains `.loading`. + /// + /// **Fails with old code (the bug):** Without `awaitConnection()`, + /// `executeLoadingLogic` runs immediately on disconnected relays. + /// `findEvent` falls through to `streamExistingEvents` (10 s default + /// timeout), which eventually returns nil → state becomes `.not_found`. + /// + /// **Passes with fix:** `awaitConnection()` blocks for up to 30 s, + /// so state stays `.loading` well past the 11 s check window. + /// + /// The 11 s sleep exceeds the `streamExistingEvents` 10 s timeout, + /// ensuring the old code path has fully resolved to `.not_found`. + func testLoadBlocksUntilConnected() async throws { + let state = generate_test_damus_state(mock_profile_info: nil) + + // Do NOT call connect() — simulates opening a nevent URL + // before relays are ready (the exact bug scenario). + let vm = LoadableNostrEventViewModel( + damus_state: state, + note_reference: .note_id(test_note.id, relays: []) + ) + + // Sleep past the 10 s streamExistingEvents timeout so the old + // code path fully resolves, but under the 30 s awaitConnection + // timeout so the fix keeps state at .loading. + try await Task.sleep(for: .seconds(11)) + + // With the fix: awaitConnection() is still blocking → .loading + // Without the fix (bug): executeLoadingLogic completed → .not_found + switch vm.state { + case .loading: + break // Correct: awaitConnection is blocking as intended + case .not_found: + XCTFail("State is .not_found — load() bypassed awaitConnection and ran executeLoadingLogic on disconnected relays (bug #3544)") + case .loaded: + XCTFail("Should not load without a relay connection") + case .unknown_or_unsupported_kind: + XCTFail("Unexpected state") + } + } + + /// Verifies that `awaitConnection()` returns immediately when the + /// network is already connected, so `load()` proceeds without delay. + func testAwaitConnection_ReturnsImmediatelyWhenConnected() async throws { + let state = generate_test_damus_state(mock_profile_info: nil) + + try! await state.nostrNetwork.userRelayList.set(userRelayList: NIP65.RelayList()) + await state.nostrNetwork.connect() + + let start = ContinuousClock.now + await state.nostrNetwork.awaitConnection() + let elapsed = ContinuousClock.now - start + + XCTAssertLessThan(elapsed, .seconds(1), "awaitConnection should return immediately when already connected") + } + + /// Verifies that `awaitConnection()` respects its timeout and does + /// not block indefinitely when no connection is established. + func testAwaitConnectionTimeout_DoesNotBlockForever() async throws { + let state = generate_test_damus_state(mock_profile_info: nil) + + let start = ContinuousClock.now + await state.nostrNetwork.awaitConnection(timeout: .milliseconds(200)) + let elapsed = ContinuousClock.now - start + + XCTAssertLessThan(elapsed, .seconds(2), "awaitConnection should respect timeout") + } +}