fix: wait for relay connection before loading nevent URLs
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 <alltheseas@users.noreply.github.com> Tested-by: Daniel D’Aquino <daniel@daquino.me> Reviewed-by: Daniel D’Aquino <daniel@daquino.me>
This commit is contained in:
committed by
Daniel D’Aquino
parent
4099827169
commit
cfafcffde2
@@ -41,6 +41,7 @@
|
|||||||
3A92C0FF2DE16E9800CEEBAC /* FaviconCache.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3A92C0FD2DE16E9800CEEBAC /* FaviconCache.swift */; };
|
3A92C0FF2DE16E9800CEEBAC /* FaviconCache.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3A92C0FD2DE16E9800CEEBAC /* FaviconCache.swift */; };
|
||||||
3A92C1002DE16E9800CEEBAC /* 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 */; };
|
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 */; };
|
3A96E3FE2D6BCE3800AE1630 /* RepostedTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3A96E3FD2D6BCE3800AE1630 /* RepostedTests.swift */; };
|
||||||
3AA247FF297E3D900090C62D /* RepostsView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3AA247FE297E3D900090C62D /* RepostsView.swift */; };
|
3AA247FF297E3D900090C62D /* RepostsView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3AA247FE297E3D900090C62D /* RepostsView.swift */; };
|
||||||
3AA24802297E3DC20090C62D /* RepostView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3AA24801297E3DC20090C62D /* RepostView.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 = "<group>"; };
|
3A929C22297F2CF80090925E /* it-IT */ = {isa = PBXFileReference; lastKnownFileType = text.plist.stringsdict; name = "it-IT"; path = "it-IT.lproj/Localizable.stringsdict"; sourceTree = "<group>"; };
|
||||||
3A92C0FD2DE16E9800CEEBAC /* FaviconCache.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FaviconCache.swift; sourceTree = "<group>"; };
|
3A92C0FD2DE16E9800CEEBAC /* FaviconCache.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FaviconCache.swift; sourceTree = "<group>"; };
|
||||||
3A92C1012DE17ACA00CEEBAC /* NIP05DomainTimelineHeaderViewTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NIP05DomainTimelineHeaderViewTests.swift; sourceTree = "<group>"; };
|
3A92C1012DE17ACA00CEEBAC /* NIP05DomainTimelineHeaderViewTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NIP05DomainTimelineHeaderViewTests.swift; sourceTree = "<group>"; };
|
||||||
|
C0AE7EE3216D7983A50BE2D9 /* LoadableNostrEventViewModelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LoadableNostrEventViewModelTests.swift; sourceTree = "<group>"; };
|
||||||
3A93342929884CA600D6A8F3 /* pl-PL */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = "pl-PL"; path = "pl-PL.lproj/InfoPlist.strings"; sourceTree = "<group>"; };
|
3A93342929884CA600D6A8F3 /* pl-PL */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = "pl-PL"; path = "pl-PL.lproj/InfoPlist.strings"; sourceTree = "<group>"; };
|
||||||
3A93342A29884CA600D6A8F3 /* pl-PL */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = "pl-PL"; path = "pl-PL.lproj/Localizable.strings"; sourceTree = "<group>"; };
|
3A93342A29884CA600D6A8F3 /* pl-PL */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = "pl-PL"; path = "pl-PL.lproj/Localizable.strings"; sourceTree = "<group>"; };
|
||||||
3A93342B29884CA600D6A8F3 /* pl-PL */ = {isa = PBXFileReference; lastKnownFileType = text.plist.stringsdict; name = "pl-PL"; path = "pl-PL.lproj/Localizable.stringsdict"; sourceTree = "<group>"; };
|
3A93342B29884CA600D6A8F3 /* pl-PL */ = {isa = PBXFileReference; lastKnownFileType = text.plist.stringsdict; name = "pl-PL"; path = "pl-PL.lproj/Localizable.stringsdict"; sourceTree = "<group>"; };
|
||||||
@@ -3931,6 +3933,7 @@
|
|||||||
64D0A2B0F048CC8D494945E6 /* RepostNotificationTests.swift */,
|
64D0A2B0F048CC8D494945E6 /* RepostNotificationTests.swift */,
|
||||||
4C0ED07E2D7A1E260020D8A2 /* Benchmarking.swift */,
|
4C0ED07E2D7A1E260020D8A2 /* Benchmarking.swift */,
|
||||||
3A92C1012DE17ACA00CEEBAC /* NIP05DomainTimelineHeaderViewTests.swift */,
|
3A92C1012DE17ACA00CEEBAC /* NIP05DomainTimelineHeaderViewTests.swift */,
|
||||||
|
C0AE7EE3216D7983A50BE2D9 /* LoadableNostrEventViewModelTests.swift */,
|
||||||
);
|
);
|
||||||
path = damusTests;
|
path = damusTests;
|
||||||
sourceTree = "<group>";
|
sourceTree = "<group>";
|
||||||
@@ -6400,6 +6403,7 @@
|
|||||||
4C684A552A7E91FE005E6031 /* LargeEventTests.swift in Sources */,
|
4C684A552A7E91FE005E6031 /* LargeEventTests.swift in Sources */,
|
||||||
E02B54182B4DFADA0077FF42 /* Bech32ObjectTests.swift in Sources */,
|
E02B54182B4DFADA0077FF42 /* Bech32ObjectTests.swift in Sources */,
|
||||||
3A92C1022DE17ACA00CEEBAC /* NIP05DomainTimelineHeaderViewTests.swift in Sources */,
|
3A92C1022DE17ACA00CEEBAC /* NIP05DomainTimelineHeaderViewTests.swift in Sources */,
|
||||||
|
EBCC3486DE53D8DB2532B98E /* LoadableNostrEventViewModelTests.swift in Sources */,
|
||||||
);
|
);
|
||||||
runOnlyForDeploymentPostprocessing = 0;
|
runOnlyForDeploymentPostprocessing = 0;
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -21,34 +21,19 @@ class LoadableNostrEventViewModel: ObservableObject {
|
|||||||
let damus_state: DamusState
|
let damus_state: DamusState
|
||||||
let note_reference: NoteReference
|
let note_reference: NoteReference
|
||||||
@Published var state: ThreadModelLoadingState = .loading
|
@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) {
|
init(damus_state: DamusState, note_reference: NoteReference) {
|
||||||
self.damus_state = damus_state
|
self.damus_state = damus_state
|
||||||
self.note_reference = note_reference
|
self.note_reference = note_reference
|
||||||
Task { await self.load() }
|
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
|
/// Waits for relay connection then loads the referenced event.
|
||||||
let timeoutTask = Task { @MainActor in
|
///
|
||||||
try await Task.sleep(nanoseconds: TIMEOUT)
|
/// Timeout is handled by `awaitConnection()` (30 s built-in).
|
||||||
loadTask.cancel() // This sends a cancellation signal to the load task.
|
func load() async {
|
||||||
self.state = .not_found
|
await damus_state.nostrNetwork.awaitConnection()
|
||||||
}
|
self.state = await executeLoadingLogic(note_reference: self.note_reference)
|
||||||
|
|
||||||
await loadTask.value
|
|
||||||
timeoutTask.cancel() // Cancel the timeout task if loading finishes earlier.
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Loads the Nostr event identified by `noteId`, optionally restricting the lookup to specific relays.
|
/// Loads the Nostr event identified by `noteId`, optionally restricting the lookup to specific relays.
|
||||||
|
|||||||
104
damusTests/LoadableNostrEventViewModelTests.swift
Normal file
104
damusTests/LoadableNostrEventViewModelTests.swift
Normal file
@@ -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")
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user