fix: download and upload files (#223)

* fix: download for unknown mime types
* fix: async process two files from the queue
* refactor: upload functions

src/modules/filemanager/components/VersionHistoryModal/VersionHistoryModal.tsx

@@ -7,7 +7,6 @@ import HistoryIcon from 'remixicon-react/HistoryLineIcon'
 import { Context as FMContext } from '../../../../providers/FileManager'
 import { TOOLTIPS } from '../../constants/tooltips'
 import { ActionTag, DownloadProgress, TrackDownloadProps } from '../../constants/transfers'
-import { useTransfers } from '../../hooks/useTransfers'
 import { ConflictAction, useUploadConflictDialog } from '../../hooks/useUploadConflictDialog'
 import { verifyDriveSpace } from '../../utils/bee'
 import { indexStrToBigint, truncateNameMiddle } from '../../utils/common'
@@ -31,15 +30,12 @@ type RenameConfirmState = {
 interface VersionHistoryModalProps {
   fileInfo: FileInfo
   onCancelClick: () => void
-  onDownload?: (props: TrackDownloadProps) => (dp: DownloadProgress) => void
+  onDownload: (props: TrackDownloadProps) => (dp: DownloadProgress) => void
 }
 
 export function VersionHistoryModal({ fileInfo, onCancelClick, onDownload }: VersionHistoryModalProps): ReactElement {
   const { fm, files, currentDrive, currentStamp, refreshStamp } = useContext(FMContext)
 
-  const localTransfers = useTransfers({})
-  const trackDownload = onDownload ?? localTransfers.trackDownload
-
   const [openConflict, conflictPortal] = useUploadConflictDialog()
   const modalRoot = document.querySelector('.fm-main') || document.body
 
@@ -345,7 +341,7 @@ export function VersionHistoryModal({ fileInfo, onCancelClick, onDownload }: Ver
               versions={!error && !loading ? pageVersions : []}
               headFi={fileInfo}
               restoreVersion={restoreVersion}
-              onDownload={trackDownload}
+              onDownload={onDownload}
             />
           </div>
         </div>

src/modules/filemanager/constants/transfers.ts

@@ -39,7 +39,6 @@ export enum FileAction {
 
 export enum DownloadState {
   InProgress = 'in-progress',
-  Completed = 'completed',
   Cancelled = 'cancelled',
   Error = 'error',
 }

src/modules/filemanager/hooks/useTransfers.ts

@@ -22,6 +22,7 @@ import { ConflictAction, useUploadConflictDialog } from './useUploadConflictDial
 const SAMPLE_WINDOW_MS = 500
 const ETA_SMOOTHING = 0.3
 const MAX_UPLOAD_FILES = 10
+const MAX_PARALLEL_UPLOAD_FILES = 2
 const ABORT_EVENT = 'abort'
 
 type ResolveResult = {
@@ -156,6 +157,19 @@ const updateTransferItems = <T extends TransferItem>(items: T[], uuid: string, u
   })
 }
 
+const getUploadCancelPromise = (signal: AbortSignal | undefined): { promise: Promise<never>; cleanup: () => void } => {
+  let reject: (reason?: Error) => void
+  const abortHandler = () => {
+    reject(new Error('Upload cancelled'))
+  }
+  const promise = new Promise<never>((_, rej) => {
+    reject = rej
+    signal?.addEventListener(ABORT_EVENT, abortHandler)
+  })
+
+  return { promise, cleanup: () => signal?.removeEventListener(ABORT_EVENT, abortHandler) }
+}
+
 const createTransferItem = (
   uuid: string,
   name: string,
@@ -185,10 +199,10 @@ export function useTransfers({ setErrorMessage }: TransferProps) {
   const { beeApi } = useContext(SettingsContext)
   const [openConflict, conflictPortal] = useUploadConflictDialog()
 
-  const isMountedRef = useRef(true)
+  const isMountedRef = useRef<boolean>(true)
   const uploadAbortsRef = useRef<AbortManager>(new AbortManager())
   const uploadTaskQueueRef = useRef<UploadTask[]>([])
-  const runningRef = useRef(false)
+  const runningRef = useRef<boolean>(false)
   const cancelledQueuedRef = useRef<Set<string>>(new Set())
   const cancelledUploadingRef = useRef<Set<string>>(new Set())
   const cancelledDownloadingRef = useRef<Set<string>>(new Set())
@@ -221,11 +235,10 @@ export function useTransfers({ setErrorMessage }: TransferProps) {
         const idx = prev.findIndex(p => p.uuid === uuid && p.status !== TransferStatus.Done)
         const base = createTransferItem(uuid, name, size, kind, driveName, TransferStatus.Queued)
 
-        if (idx !== -1) {
-          clearAllFlagsFor(uuid)
-        }
-
         if (idx === -1) return [...prev, base]
+
+        clearAllFlagsFor(uuid)
+
         const copy = [...prev]
         copy[idx] = base
 
@@ -378,14 +391,6 @@ export function useTransfers({ setErrorMessage }: TransferProps) {
         topic: task.isReplace ? task.replaceTopic : undefined,
       }
 
-      const progressCb = trackUpload(
-        task.uuid,
-        task.finalName,
-        task.prettySize,
-        task.isReplace ? FileTransferType.Update : FileTransferType.Upload,
-        taskDrive.name,
-      )
-
       safeSetState(
         isMountedRef,
         setUploadItems,
@@ -400,33 +405,29 @@ export function useTransfers({ setErrorMessage }: TransferProps) {
       uploadAbortsRef.current.create(task.uuid)
       const signal = uploadAbortsRef.current.getSignal(task.uuid)
 
-      let reject: (reason?: Error) => void
+      const { promise: checkCancellation, cleanup: cleanupCancelPromise } = getUploadCancelPromise(signal)
-      const abortHandler = () => {
-        reject(new Error('Upload cancelled'))
-      }
-
-      const checkCancellation = new Promise<never>((_, rej) => {
-        reject = rej
-        signal?.addEventListener(ABORT_EVENT, abortHandler)
-      })
 
       try {
         if (signal?.aborted) {
           throw new Error('Upload cancelled')
         }
 
+        const onUploadProgress = trackUpload(
+          task.uuid,
+          task.finalName,
+          task.prettySize,
+          task.isReplace ? FileTransferType.Update : FileTransferType.Upload,
+          taskDrive.name,
+        )
+
         const uploadPromise = fm.upload(
           taskDrive,
-          { ...info, onUploadProgress: progressCb },
+          { ...info, onUploadProgress },
           { actHistoryAddress: task.isReplace ? task.replaceHistory : undefined },
           { signal },
         )
 
         await Promise.race([uploadPromise, checkCancellation])
-
-        if (currentStamp) {
-          await refreshStamp(currentStamp.batchID.toString())
-        }
       } catch (error) {
         const wasCancelled = cancelledUploadingRef.current.has(task.uuid) || signal?.aborted
 
@@ -445,7 +446,7 @@ export function useTransfers({ setErrorMessage }: TransferProps) {
           }),
         )
       } finally {
-        signal?.removeEventListener(ABORT_EVENT, abortHandler)
+        cleanupCancelPromise()
 
         const wasCancelled = cancelledUploadingRef.current.has(task.uuid) || signal?.aborted
 
@@ -456,7 +457,7 @@ export function useTransfers({ setErrorMessage }: TransferProps) {
         }
       }
     },
-    [fm, files, currentStamp, trackUpload, refreshStamp, setShowError, setErrorMessage],
+    [fm, files, trackUpload, setShowError, setErrorMessage],
   )
 
   const trackDownload = useCallback((props: TrackDownloadProps) => {
@@ -686,37 +687,65 @@ export function useTransfers({ setErrorMessage }: TransferProps) {
     if (runningRef.current) return
     runningRef.current = true
 
-    while (uploadTaskQueueRef.current.length > 0) {
+    const processNextTask = async (): Promise<void> => {
-      const task = uploadTaskQueueRef.current[0]
+      while (uploadTaskQueueRef.current.length > 0) {
+        const task = uploadTaskQueueRef.current.shift()
 
-      if (!task) break
+        if (!task) break
 
-      if (cancelledQueuedRef.current.has(task.uuid)) {
+        if (cancelledQueuedRef.current.has(task.uuid)) {
-        safeSetState(
+          safeSetState(
-          isMountedRef,
+            isMountedRef,
-          setUploadItems,
+            setUploadItems,
-        )(prev => updateTransferItems(prev, task.uuid, { status: TransferStatus.Cancelled }))
+          )(prev => updateTransferItems(prev, task.uuid, { status: TransferStatus.Cancelled }))
-        cancelledQueuedRef.current.delete(task.uuid)
+          cancelledQueuedRef.current.delete(task.uuid)
-      } else {
+        } else {
-        await executeUploadTask(task)
+          await executeUploadTask(task)
+        }
       }
-
-      uploadTaskQueueRef.current.shift()
     }
 
+    const workers: Promise<void>[] = []
+    for (let i = 0; i < MAX_PARALLEL_UPLOAD_FILES; i++) {
+      workers.push(processNextTask())
+    }
+
+    await Promise.all(workers)
+
     runningRef.current = false
 
     // Race guard: uploadFiles may have appended tasks and called runUploadQueue() again
     if (uploadTaskQueueRef.current.length > 0) {
-      runUploadQueue()
+      void runUploadQueue()
     }
-  }, [executeUploadTask])
 
-  const uploadFiles = useCallback(
+    if (currentStamp) {
-    (picked: FileList | File[]): void => {
+      await refreshStamp(currentStamp.batchID.toString())
-      const filesArr = Array.from(picked)
+    }
+  }, [currentStamp, executeUploadTask, refreshStamp])
 
-      if (filesArr.length === 0 || !fm || !currentDrive || !currentStamp) return
+  const verifyUploadConditions = useCallback(
+    async (filesArr: File[]): Promise<boolean> => {
+      if (filesArr.length === 0) {
+        setErrorMessage?.('Nothing to upload.')
+        setShowError(true)
+
+        return false
+      }
+
+      if (!fm || !currentDrive) {
+        setErrorMessage?.('File manager is not ready or no drive is selected.')
+        setShowError(true)
+
+        return false
+      }
+
+      if (!currentStamp || !currentStamp.usable) {
+        setErrorMessage?.('Stamp is not usable.')
+        setShowError(true)
+
+        return false
+      }
 
       const currentlyQueued = uploadTaskQueueRef.current.length
       const newFilesCount = filesArr.length
@@ -728,36 +757,41 @@ export function useTransfers({ setErrorMessage }: TransferProps) {
         )
         setShowError(true)
 
+        return false
+      }
+
+      if (beeApi) {
+        const batchID = currentStamp.batchID
+        const stampValid = await validateStampStillExists(beeApi, batchID)
+
+        if (!stampValid) {
+          setErrorMessage?.(
+            `The selected stamp ${batchID.toString().slice(0, 4)} is no longer valid or has been deleted. Please select a different stamp.`,
+          )
+          setShowError(true)
+
+          return false
+        }
+      }
+
+      return true
+    },
+    [fm, currentStamp, currentDrive, beeApi, setShowError, setErrorMessage],
+  )
+
+  const uploadFiles = useCallback(
+    async (picked: FileList | File[]): Promise<void> => {
+      const filesArr = Array.from(picked)
+
+      if (!(await verifyUploadConditions(filesArr))) {
         return
       }
 
-      void (async () => {
+      const tasks = await preflight(filesArr)
-        if (!currentStamp || !currentStamp.usable) {
+      uploadTaskQueueRef.current = uploadTaskQueueRef.current.concat(tasks)
-          setErrorMessage?.('Stamp is not usable.')
+      runUploadQueue()
-          setShowError(true)
-
-          return
-        }
-
-        if (beeApi) {
-          const stampValid = await validateStampStillExists(beeApi, currentStamp.batchID)
-
-          if (!stampValid) {
-            setErrorMessage?.(
-              'The selected stamp is no longer valid or has been deleted. Please select a different stamp.',
-            )
-            setShowError(true)
-
-            return
-          }
-        }
-
-        const tasks = await preflight(filesArr)
-        uploadTaskQueueRef.current = uploadTaskQueueRef.current.concat(tasks)
-        runUploadQueue()
-      })()
     },
-    [fm, currentStamp, currentDrive, beeApi, setShowError, setErrorMessage, preflight, runUploadQueue],
+    [verifyUploadConditions, preflight, runUploadQueue],
   )
 
   const cancelOrDismissUpload = useCallback(
@@ -865,13 +899,13 @@ export function useTransfers({ setErrorMessage }: TransferProps) {
   }, [])
 
   return {
-    uploadFiles,
     isUploading,
     uploadItems,
-    trackDownload,
     isDownloading,
     downloadItems,
     conflictPortal,
+    uploadFiles,
+    trackDownload,
     cancelOrDismissUpload,
     cancelOrDismissDownload,
     dismissAllUploads,

src/modules/filemanager/utils/download.ts

@@ -6,6 +6,8 @@ import { AbortManager } from './abortManager'
 import { isDirectoryPickerSupported, isPickerSupported } from './fileOperations'
 import { guessMime, VIEWERS } from './view'
 
+const DefaultDownloadFolder = 'downloads'
+
 const downloadAborts = new AbortManager()
 
 enum Errors {
@@ -148,10 +150,7 @@ const isUserCancellation = (error: unknown): boolean => {
   return errName === Errors.AbortError || errName === Errors.NotAllowedError || errName === Errors.SecurityError
 }
 
-const getSingleFileHandle = async (
+const getSingleFileHandle = async (infoWithId: FileInfoWithUUID): Promise<FileInfoWithHandle | undefined> => {
-  infoWithId: FileInfoWithUUID,
-  defaultDownloadFolder: string,
-): Promise<FileInfoWithHandle[] | undefined> => {
   const { mime, ext } = guessMime(infoWithId.info.name, infoWithId.info.customMetadata)
 
   const pickerOptions: {
@@ -160,7 +159,7 @@ const getSingleFileHandle = async (
     types?: Array<{ accept: Record<string, string[]> }>
   } = {
     suggestedName: infoWithId.info.name,
-    startIn: defaultDownloadFolder,
+    startIn: DefaultDownloadFolder,
   }
 
   if (ext) {
@@ -171,24 +170,24 @@ const getSingleFileHandle = async (
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
     const handle = (await (window as any).showSaveFilePicker(pickerOptions)) as FileSystemFileHandle
 
-    return [{ infoWithId, handle }]
+    return { infoWithId, handle }
   } catch (error: unknown) {
-    return isUserCancellation(error) ? [{ infoWithId, cancelled: true }] : undefined
+    return isUserCancellation(error) ? { infoWithId, cancelled: true } : undefined
   }
 }
 
 const getMultipleFileHandles = async (
   infoWithIdList: FileInfoWithUUID[],
-  defaultDownloadFolder: string,
 ): Promise<FileInfoWithHandle[] | undefined> => {
   if (!isDirectoryPickerSupported()) {
     const handles: FileInfoWithHandle[] = []
 
     for (const info of infoWithIdList) {
-      const result = await getSingleFileHandle(info, defaultDownloadFolder)
+      const result = await getSingleFileHandle(info)
 
       if (!result) return undefined
-      handles.push(result[0])
+
+      handles.push(result)
     }
 
     return handles
@@ -198,7 +197,7 @@ const getMultipleFileHandles = async (
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
     const dirHandle = (await (window as any).showDirectoryPicker({
       mode: 'readwrite',
-      startIn: defaultDownloadFolder,
+      startIn: DefaultDownloadFolder,
     })) as FileSystemDirectoryHandle
 
     const handles: FileInfoWithHandle[] = []
@@ -224,16 +223,16 @@ const getMultipleFileHandles = async (
   }
 }
 
-const getFileHandles = (infoWithIdList: FileInfoWithUUID[]): Promise<FileInfoWithHandle[] | undefined> => {
+const getFileHandles = async (infoWithIdList: FileInfoWithUUID[]): Promise<FileInfoWithHandle[] | undefined> => {
-  const defaultDownloadFolder = 'downloads'
-
   if (!isPickerSupported()) return Promise.resolve(infoWithIdList.map(infoWithId => ({ infoWithId })))
 
   if (infoWithIdList.length === 1) {
-    return getSingleFileHandle(infoWithIdList[0], defaultDownloadFolder)
+    const fh = await getSingleFileHandle(infoWithIdList[0])
+
+    return fh ? [fh] : undefined
   }
 
-  return getMultipleFileHandles(infoWithIdList, defaultDownloadFolder)
+  return getMultipleFileHandles(infoWithIdList)
 }
 
 const downloadToDisk = async (
@@ -258,20 +257,25 @@ const downloadToDisk = async (
   }
 }
 
+interface BlobDownloadResult {
+  success: boolean
+  cancelled: boolean
+}
+
 const downloadToBlob = async (
   streams: ReadableStream<Uint8Array>[],
   info: FileInfo,
   onDownloadProgress?: (progress: DownloadProgress) => void,
   isOpenWindow?: boolean,
   signal?: AbortSignal,
-): Promise<boolean> => {
+): Promise<BlobDownloadResult> => {
   try {
     for (const stream of streams) {
       const { mime } = guessMime(info.name, info.customMetadata)
       const blob = await streamToBlob(stream, mime, onDownloadProgress, signal)
 
       if (!blob) {
-        return false
+        return { success: false, cancelled: false }
       }
 
       const url = URL.createObjectURL(blob)
@@ -282,50 +286,104 @@ const downloadToBlob = async (
       }
 
       if (!opened) {
+        if (isOpenWindow && isPickerSupported()) {
+          const result = await saveBlobWithPicker(blob, info, onDownloadProgress, signal)
+          URL.revokeObjectURL(url)
+
+          return result
+        }
+
         downloadFromUrl(url, info.name)
       }
     }
 
-    return true
+    return { success: true, cancelled: false }
   } catch (error: unknown) {
     if ((error as { name?: string }).name !== Errors.AbortError) {
       // eslint-disable-next-line no-console
       console.error('Error during download and open: ', error)
     }
 
-    return false
+    return { success: false, cancelled: false }
   }
 }
 
 const openNewWindow = (name: string, mime: string, url: string): boolean => {
   const viewer = VIEWERS.find(v => v.test(mime))
+
+  if (!viewer) return false
+
   const win = window.open('', '_blank')
 
-  if (viewer && win) {
+  if (!win) return false
-    viewer.render(win, url, mime, name)
 
-    return true
+  try {
+    viewer.render(win, url, mime, name)
+  } catch (err: unknown) {
+    win.close()
+    // eslint-disable-next-line no-console
+    console.error('Failed to render file in a new window: ', err)
+
+    return false
   }
 
-  win?.close()
+  return true
-
-  return false
 }
 
+const saveBlobWithPicker = async (
+  blob: Blob,
+  info: FileInfo,
+  onDownloadProgress?: (progress: DownloadProgress) => void,
+  signal?: AbortSignal,
+): Promise<BlobDownloadResult> => {
+  const infoWithId: FileInfoWithUUID = { uuid: '', info }
+
+  try {
+    if (signal?.aborted) {
+      throw new DOMException('Aborted', Errors.AbortError)
+    }
+
+    const fh = await getSingleFileHandle(infoWithId)
+
+    if (!fh || !fh.handle) {
+      return { success: false, cancelled: false }
+    }
+
+    if (fh.cancelled) {
+      return { success: false, cancelled: true }
+    }
+
+    await processStream(blob.stream(), fh.handle, onDownloadProgress, signal)
+
+    return { success: true, cancelled: false }
+  } catch (err: unknown) {
+    if (isUserCancellation(err)) {
+      return { success: false, cancelled: true }
+    }
+
+    // eslint-disable-next-line no-console
+    console.error('Failed to save blob using file picker: ', err)
+
+    return { success: false, cancelled: false }
+  }
+}
+
+const RevokeUrlTimeout = 1000
+
 const downloadFromUrl = (url: string, fileName: string): void => {
   const a = document.createElement('a')
   a.href = url
   a.download = fileName
   document.body.appendChild(a)
   a.click()
-  window.URL.revokeObjectURL(url)
+  window.setTimeout(() => window.URL.revokeObjectURL(url), RevokeUrlTimeout)
   document.body.removeChild(a)
 }
 
 export const startDownloadingQueue = async (
   fm: FileManager,
   infoListWithIds: FileInfoWithUUID[],
-  trackers?: Array<(progress: DownloadProgress) => void>,
+  trackers: Array<(progress: DownloadProgress) => void>,
   isOpenWindow?: boolean,
 ): Promise<void> => {
   if (!infoListWithIds.length || (trackers && trackers.length !== infoListWithIds.length)) return
@@ -339,7 +397,7 @@ export const startDownloadingQueue = async (
 
     await Promise.all(
       fileHandles.map(async (fh, i) => {
-        const tracker = trackers ? trackers[i] : undefined
+        const tracker = trackers[i]
 
         const uuid = fh.infoWithId.uuid
         createDownloadAbort(uuid)
@@ -347,7 +405,7 @@ export const startDownloadingQueue = async (
 
         try {
           if (fh.cancelled) {
-            tracker?.({ progress: 0, isDownloading: false, state: DownloadState.Cancelled })
+            tracker({ progress: 0, isDownloading: false, state: DownloadState.Cancelled })
 
             return
           }
@@ -359,23 +417,29 @@ export const startDownloadingQueue = async (
           if (!dataStreams || dataStreams.length === 0) {
             // eslint-disable-next-line no-console
             console.error(`No data streams returned for ${fh.infoWithId.info.name}`)
-            tracker?.({ progress: 0, isDownloading: false, state: DownloadState.Error })
+            tracker({ progress: 0, isDownloading: false, state: DownloadState.Error })
 
             return
           }
 
           let success = false
+          let userCancelled = false
 
           if (isOpenWindow || !fh.handle) {
-            success = await downloadToBlob(dataStreams, fh.infoWithId.info, tracker, isOpenWindow, signal)
+            const { success: saved, cancelled } = await downloadToBlob(
+              dataStreams,
+              fh.infoWithId.info,
+              tracker,
+              isOpenWindow,
+              signal,
+            )
+
+            success = saved
+            userCancelled = cancelled
           } else {
             success = await downloadToDisk(dataStreams, fh.handle, tracker, signal)
           }
 
-          if (!tracker) {
-            return
-          }
-
           if (success) {
             const size = fh.infoWithId.info.customMetadata?.size
             const finalProgress = size ? Number(size) : 0
@@ -385,18 +449,22 @@ export const startDownloadingQueue = async (
           }
 
           if (!signal?.aborted) {
-            tracker({ progress: 0, isDownloading: false, state: DownloadState.Error })
+            tracker({
+              progress: 0,
+              isDownloading: false,
+              state: userCancelled ? DownloadState.Cancelled : DownloadState.Error,
+            })
           }
         } catch (error: unknown) {
           const isAbortError = (error as { name?: string }).name === Errors.AbortError
 
           if (!isAbortError) {
-            tracker?.({ progress: 0, isDownloading: false, state: DownloadState.Error })
+            tracker({ progress: 0, isDownloading: false, state: DownloadState.Error })
 
             // eslint-disable-next-line no-console
             console.error('download queue error: ', error)
           } else {
-            tracker?.({ progress: 0, isDownloading: false, state: DownloadState.Cancelled })
+            tracker({ progress: 0, isDownloading: false, state: DownloadState.Cancelled })
           }
         } finally {
           downloadAborts.abort(uuid)

src/pages/filemanager/index.tsx

@@ -186,12 +186,9 @@ export function FileManagerPage(): ReactElement {
   }, [])
 
   useEffect(() => {
-    if (status.all !== CheckState.OK) {
+    const isApiError = status.apiConnection.checkState !== CheckState.OK || !status.apiConnection.isEnabled
-      setShowConnectionError(true)
+    setShowConnectionError(isApiError)
-    } else {
+  }, [status.apiConnection])
-      setShowConnectionError(false)
-    }
-  }, [status.all])
 
   useEffect(() => {
     if (!beeApi) {