From 35c50d02f7fc67564660564ee7768dcb649161d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ferenc=20S=C3=A1rai?= Date: Wed, 22 Jul 2026 16:56:20 +0200 Subject: [PATCH] fix: replace stewardship sync check with local chunk probe (#299) * fix: replace stewardship sync check with local chunk probe Direct (non-deferred) uploads already push chunks to the network with a confirmed receipt before the upload call resolves, so the post-upload sync screen's isReferenceRetrievable check was re-verifying something already guaranteed - and, being a full per-chunk network re-traversal, its cost scaled with chunk count and routinely took minutes or timed out for anything beyond a handful of chunks, even on a healthy network. Replace it with a single local probeData (HEAD /bytes) call plus one retry, each bounded by an explicit AbortSignal timeout so a hung request can't stall the UI indefinitely. Add an indeterminate mode to LinearProgressWithLabel so the bar animates instead of sitting frozen at 0% while the check runs. Fixes SPDV-1413 Co-Authored-By: Claude Sonnet 5 * fix: reset sync state when reference changes syncProgress/probeFailed were never reset on a reference/beeApi change, so navigating between two uploaded files' Share pages without a full unmount could show a stale 100% or failure message carried over from the previous file. Addresses Copilot review feedback on #299. Co-Authored-By: Claude Sonnet 5 * fix: correctly apply linearProgressProps to LinearProgress linearProgressProps was declared but never actually forwarded - it was spread onto as a prop named "linearProgressProps" via {...props}, which also blocked value/variant from being overridden. Destructure it explicitly and spread it onto the underlying component instead. Addresses Copilot review feedback on #299. Co-Authored-By: Claude Sonnet 5 * fix: avoid misleading 0% label on probe failure When probeData failed, syncProgress stayed at 0 and indeterminate was false, so the bar showed "0%" right next to text saying the upload succeeded. Add a label override to LinearProgressWithLabel and use it to show "Unknown" instead of a percentage in that state. Addresses Copilot review feedback on #299. Co-Authored-By: Claude Sonnet 5 * fix: don't schedule retry after unmount/reference change The first-attempt failure branch scheduled a retry unconditionally, without checking isMounted, so a rejection arriving after unmount (or a reference/beeApi change) could still trigger an unnecessary extra probeData request. Guard the whole catch block on isMounted, and abort the in-flight request on cleanup so it doesn't keep running pointlessly either. Addresses Copilot review feedback on #299. Co-Authored-By: Claude Sonnet 5 --------- Co-authored-by: Claude Sonnet 5 --- src/components/ProgressBar.tsx | 16 +++- src/pages/files/AssetSyncing.tsx | 123 ++++++++++++++----------------- 2 files changed, 69 insertions(+), 70 deletions(-) diff --git a/src/components/ProgressBar.tsx b/src/components/ProgressBar.tsx index bb3aee80..8aabe0aa 100644 --- a/src/components/ProgressBar.tsx +++ b/src/components/ProgressBar.tsx @@ -6,16 +6,26 @@ import React, { ReactElement } from 'react' interface Props { linearProgressProps?: LinearProgressProps value: number + indeterminate?: boolean + label?: string } -export function LinearProgressWithLabel(props: Props): ReactElement { +export function LinearProgressWithLabel({ indeterminate, value, linearProgressProps, label }: Props): ReactElement { + const displayLabel = label ?? (indeterminate ? 'Syncing...' : `${Math.round(value)}%`) + return ( - + - {`${Math.round(props.value)}%`} + + {displayLabel} + ) diff --git a/src/pages/files/AssetSyncing.tsx b/src/pages/files/AssetSyncing.tsx index 65129695..31457382 100644 --- a/src/pages/files/AssetSyncing.tsx +++ b/src/pages/files/AssetSyncing.tsx @@ -1,6 +1,5 @@ -import { Tag } from '@ethersphere/bee-js' import { Box } from '@mui/material' -import { ReactElement, useCallback, useContext, useEffect, useRef, useState } from 'react' +import { ReactElement, useContext, useEffect, useState } from 'react' import { DocumentationText } from '../../components/DocumentationText' import { LinearProgressWithLabel } from '../../components/ProgressBar' @@ -10,88 +9,66 @@ interface Props { reference?: string } -const SYNC_CHECK_INTERVAL_MS = 2000 +const PROBE_RETRY_DELAY_MS = 500 +const PROBE_TIMEOUT_MS = 10 * 1000 export function AssetSyncing({ reference }: Props): ReactElement { const { beeApi } = useContext(SettingsContext) - const syncTimer = useRef | null>(null) - const retrieveCheckRef = useRef(false) - const [isRetrieveChecking, setIsRetrieveChecking] = useState(false) const [syncProgress, setSyncProgress] = useState(0) + const [probeFailed, setProbeFailed] = useState(false) + + useEffect(() => { + setSyncProgress(0) + setProbeFailed(false) - const syncCheck = useCallback(async () => { if (!beeApi || !reference) return - let allTags: Tag[] = [] - let offset = 0 - const limit = 1000 - let tagsBatch: Tag[] - - do { - tagsBatch = await beeApi.getAllTags({ limit, offset }) - allTags = allTags.concat(tagsBatch) - offset += limit - } while (tagsBatch.length === limit) // Continue if the batch is full, stop if fewer than the limit - - const tag = allTags.find(t => t.address === reference) - - if (tag && tag.split > 0) { - const progress = ((tag.seen + tag.synced) / tag.split) * 100 - setSyncProgress(progress) - } else if (!tag && !retrieveCheckRef.current) { - // Direct (non-deferred) uploads do not create a tag on the Bee node, - // so verify network availability with the stewardship endpoint instead - retrieveCheckRef.current = true + let isMounted = true + let retryTimer: ReturnType | null = null + let currentAbortController: AbortController | null = null + + // deferred: false already guarantees the upload was pushed to and acknowledged by the + // network before the upload call resolved. This is just a cheap local sanity check + // (HEAD on the root chunk, served from local storage) - not a network-wide verification. + const check = async (isRetry: boolean) => { + // fetch() ignores requestOptions.timeout, so bound the request explicitly - otherwise a + // hung request never resolves or rejects, and neither the retry nor the failure state + // would ever trigger. + const abortController = new AbortController() + currentAbortController = abortController + const abortTimer = setTimeout(() => abortController.abort(), PROBE_TIMEOUT_MS) + try { - if (await beeApi.isReferenceRetrievable(reference)) { - setSyncProgress(100) - } + await beeApi.probeData(reference, { signal: abortController.signal }) + + if (isMounted) setSyncProgress(100) } catch { - // Transient error, the next interval tick will retry + // Bail out entirely once unmounted/reference changed, so a rejection from an + // in-flight first attempt can't schedule an unnecessary extra retry request. + if (!isMounted) return + + if (!isRetry) { + retryTimer = setTimeout(() => check(true), PROBE_RETRY_DELAY_MS) + } else { + setProbeFailed(true) + } } finally { - retrieveCheckRef.current = false + clearTimeout(abortTimer) } } - }, [beeApi, reference]) - useEffect(() => { - syncTimer.current = setInterval(syncCheck, SYNC_CHECK_INTERVAL_MS) + check(false) return () => { - if (syncTimer.current) { - clearInterval(syncTimer.current) - syncTimer.current = null - } - } - }, [reference, syncCheck]) + isMounted = false + currentAbortController?.abort() - useEffect(() => { - if (syncProgress === 100 && syncTimer.current) { - clearInterval(syncTimer.current) - syncTimer.current = null - } - }, [syncProgress]) - - useEffect(() => { - /* - There are instances when it seems that the content isn't synchronized, despite being already available. - To ensure it's not due to invalid synchronization data, - verify availability from at least 70% using one of the stewardship endpoints. - */ - if (beeApi && reference && !isRetrieveChecking && syncProgress > 10 && syncProgress < 100) { - // It's a long running task make sure only one run occurs at a time. - setIsRetrieveChecking(true) - - beeApi.isReferenceRetrievable(reference).then(isRetriavable => { - if (isRetriavable) { - setSyncProgress(100) - } - - setIsRetrieveChecking(false) - }) + if (retryTimer) { + clearTimeout(retryTimer) + } } - }, [syncProgress, isRetrieveChecking, beeApi, reference]) + }, [beeApi, reference]) return ( <> @@ -104,8 +81,20 @@ export function AssetSyncing({ reference }: Props): ReactElement { - + + {probeFailed && ( + + + Upload succeeded, but we couldn't confirm it locally. Try refreshing this page — your file has very + likely still been uploaded successfully. + + + )} ) }