fix(ui): keep peer-check state backend-driven
Downloading a game could keep showing "Checking peers" while the backend was already transferring files. The frontend owned a five-second fallback that could invent a no-peers error during a valid long download, then return the action to Download until install began. Remove that frontend timer and make the peer lifecycle authoritative instead. The UI now treats CheckingPeers as only an optimistic click response, ignores it if a real operation is already in progress, and switches to Downloading when the existing game-download-pre bridge reports that peer metadata was found. A review found one backend path that previously had no terminal event: candidate peers existed, but every peer detail request failed before GotGameFiles. That path now emits DownloadGameFilesFailed so the UI can leave CheckingPeers without falling back to a frontend guess. Test Plan: - just fmt - just clippy - just test - just build - git diff --check Refs: local review P2
This commit is contained in:
@@ -185,6 +185,9 @@ async fn fetch_game_details_from_peers<F, Fut>(
|
||||
}
|
||||
} else {
|
||||
log::warn!("Failed to retrieve game files for {id} from any peer");
|
||||
if let Err(e) = tx_notify_ui.send(PeerEvent::DownloadGameFilesFailed { id: id.clone() }) {
|
||||
log::error!("Failed to send DownloadGameFilesFailed event: {e}");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1171,6 +1174,48 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn failed_peer_detail_fetch_emits_terminal_download_failure() {
|
||||
let first_addr = addr(12_020);
|
||||
let second_addr = addr(12_021);
|
||||
let peer_game_db = Arc::new(RwLock::new(PeerGameDB::new()));
|
||||
let (tx, mut rx) = mpsc::unbounded_channel();
|
||||
let fetched_peers = Arc::new(Mutex::new(Vec::new()));
|
||||
|
||||
fetch_game_details_from_peers(
|
||||
vec![first_addr, second_addr],
|
||||
"game".to_string(),
|
||||
peer_game_db,
|
||||
tx.clone(),
|
||||
{
|
||||
let fetched_peers = fetched_peers.clone();
|
||||
move |peer_addr, _game_id, _peer_game_db| {
|
||||
let fetched_peers = fetched_peers.clone();
|
||||
async move {
|
||||
fetched_peers
|
||||
.lock()
|
||||
.expect("fetched peer list should not be poisoned")
|
||||
.push(peer_addr);
|
||||
Err::<Vec<GameFileDescription>, _>(eyre::eyre!("detail fetch failed"))
|
||||
}
|
||||
}
|
||||
},
|
||||
)
|
||||
.await;
|
||||
|
||||
assert_eq!(
|
||||
*fetched_peers
|
||||
.lock()
|
||||
.expect("fetched peer list should not be poisoned"),
|
||||
vec![first_addr, second_addr]
|
||||
);
|
||||
assert!(matches!(
|
||||
recv_event(&mut rx).await,
|
||||
PeerEvent::DownloadGameFilesFailed { id } if id == "game"
|
||||
));
|
||||
assert_no_event(&mut rx).await;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn update_request_skips_local_manifest_even_when_download_exists() {
|
||||
let temp = TempDir::new("lanspread-handler-latest-peer");
|
||||
|
||||
@@ -10,12 +10,11 @@ import {
|
||||
} from '../lib/types';
|
||||
import {
|
||||
activeStatusById,
|
||||
isInProgress,
|
||||
mergeGameUpdate,
|
||||
normalizeGamesListPayload,
|
||||
} from '../lib/gameState';
|
||||
|
||||
const CHECKING_PEERS_TIMEOUT_MS = 5000;
|
||||
|
||||
interface PendingPatch {
|
||||
install_status?: InstallStatus;
|
||||
downloaded?: boolean;
|
||||
@@ -47,8 +46,7 @@ const applyPatch = (game: Game, patch: PendingPatch): Game => {
|
||||
* Owns the games list and reflects every backend event (download/install/
|
||||
* uninstall/remove lifecycle, peer count) into local React state. Returns a
|
||||
* fire-and-forget `markChecking` helper so action calls can immediately show a
|
||||
* "Checking peers…" state with an automatic fall-back if the backend never
|
||||
* emits a follow-up event.
|
||||
* "Checking peers…" state until the backend emits the authoritative outcome.
|
||||
*/
|
||||
export interface UseGamesResult {
|
||||
games: Game[];
|
||||
@@ -57,57 +55,29 @@ export interface UseGamesResult {
|
||||
requestGames: () => Promise<void>;
|
||||
markChecking: (id: string) => void;
|
||||
markInstalling: (id: string) => void;
|
||||
cancelChecking: (id: string) => void;
|
||||
}
|
||||
|
||||
export const useGames = (rescanGameDir: () => void): UseGamesResult => {
|
||||
const [games, setGames] = useState<Game[]>([]);
|
||||
const [totalPeerCount, setTotalPeerCount] = useState(0);
|
||||
const checkingTimeouts = useRef<Record<string, ReturnType<typeof setTimeout>>>({});
|
||||
const rescanRef = useRef(rescanGameDir);
|
||||
rescanRef.current = rescanGameDir;
|
||||
|
||||
const cancelChecking = useCallback((id: string) => {
|
||||
const t = checkingTimeouts.current[id];
|
||||
if (t !== undefined) {
|
||||
clearTimeout(t);
|
||||
delete checkingTimeouts.current[id];
|
||||
}
|
||||
const markChecking = useCallback((id: string) => {
|
||||
setGames(prev => prev.map(item =>
|
||||
item.id === id && !isInProgress(item.install_status)
|
||||
? applyPatch(item, { install_status: InstallStatus.CheckingPeers, clearStatus: true })
|
||||
: item
|
||||
));
|
||||
}, []);
|
||||
|
||||
const markChecking = useCallback((id: string) => {
|
||||
cancelChecking(id);
|
||||
setGames(prev => prev.map(item =>
|
||||
item.id === id
|
||||
? { ...item, install_status: InstallStatus.CheckingPeers }
|
||||
: item,
|
||||
));
|
||||
checkingTimeouts.current[id] = setTimeout(() => {
|
||||
setGames(prev => prev.map(item => {
|
||||
if (item.id !== id || item.install_status !== InstallStatus.CheckingPeers) {
|
||||
return item;
|
||||
}
|
||||
return {
|
||||
...item,
|
||||
install_status: item.installed
|
||||
? InstallStatus.Installed
|
||||
: InstallStatus.NotInstalled,
|
||||
status_message: 'No peers currently have this game.',
|
||||
status_level: 'error',
|
||||
};
|
||||
}));
|
||||
delete checkingTimeouts.current[id];
|
||||
}, CHECKING_PEERS_TIMEOUT_MS);
|
||||
}, [cancelChecking]);
|
||||
|
||||
const markInstalling = useCallback((id: string) => {
|
||||
cancelChecking(id);
|
||||
setGames(prev => prev.map(item =>
|
||||
item.id === id
|
||||
? applyPatch(item, { install_status: InstallStatus.Installing, clearStatus: true })
|
||||
: item,
|
||||
));
|
||||
}, [cancelChecking]);
|
||||
}, []);
|
||||
|
||||
const requestGames = useCallback(async () => {
|
||||
try {
|
||||
@@ -130,7 +100,6 @@ export const useGames = (rescanGameDir: () => void): UseGamesResult => {
|
||||
message: string,
|
||||
{ triggerRescan = false }: { triggerRescan?: boolean } = {},
|
||||
) => {
|
||||
cancelChecking(id);
|
||||
setGames(prev => prev.map(item => item.id === id
|
||||
? {
|
||||
...item,
|
||||
@@ -163,15 +132,18 @@ export const useGames = (rescanGameDir: () => void): UseGamesResult => {
|
||||
});
|
||||
}));
|
||||
|
||||
unlisteners.push(await listen('game-download-pre', (e) => {
|
||||
const id = e.payload as string;
|
||||
updateById(id, { install_status: InstallStatus.Downloading, clearStatus: true });
|
||||
}));
|
||||
|
||||
unlisteners.push(await listen('game-download-begin', (e) => {
|
||||
const id = e.payload as string;
|
||||
cancelChecking(id);
|
||||
updateById(id, { install_status: InstallStatus.Downloading, clearStatus: true });
|
||||
}));
|
||||
|
||||
unlisteners.push(await listen('game-download-finished', (e) => {
|
||||
const id = e.payload as string;
|
||||
cancelChecking(id);
|
||||
updateById(id, { install_status: InstallStatus.Installing, clearStatus: true });
|
||||
}));
|
||||
|
||||
@@ -193,13 +165,11 @@ export const useGames = (rescanGameDir: () => void): UseGamesResult => {
|
||||
|
||||
unlisteners.push(await listen('game-install-begin', (e) => {
|
||||
const id = e.payload as string;
|
||||
cancelChecking(id);
|
||||
updateById(id, { install_status: InstallStatus.Installing, clearStatus: true });
|
||||
}));
|
||||
|
||||
unlisteners.push(await listen('game-install-finished', (e) => {
|
||||
const id = e.payload as string;
|
||||
cancelChecking(id);
|
||||
updateById(id, {
|
||||
install_status: InstallStatus.Installed,
|
||||
installed: true,
|
||||
@@ -274,10 +244,8 @@ export const useGames = (rescanGameDir: () => void): UseGamesResult => {
|
||||
return () => {
|
||||
cancelled = true;
|
||||
unlisteners.forEach(fn => fn());
|
||||
Object.values(checkingTimeouts.current).forEach(clearTimeout);
|
||||
checkingTimeouts.current = {};
|
||||
};
|
||||
}, [cancelChecking]);
|
||||
}, []);
|
||||
|
||||
return {
|
||||
games,
|
||||
@@ -286,6 +254,5 @@ export const useGames = (rescanGameDir: () => void): UseGamesResult => {
|
||||
requestGames,
|
||||
markChecking,
|
||||
markInstalling,
|
||||
cancelChecking,
|
||||
};
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user