@samitouri / QOSami-HFS / commits / 71b5e506

fix: upload could get stuck in case of disconnection, or 2 files with split-uploads

Massimo Melina committed May 5, 2025 at 23:44 UTC 71b5e506db2e77c37b31b6ea36286f88b7f249c7
4 files changed +104 -90
admin/src/api.ts
+1 -1
@@ -148,7 +148,7 @@ export function useApiList<T=any, S=T>(cmd:string|Falsy, params: Dict={}, { map,
148 }
149 return ret
150 })
151 - if (src?.readyState === src?.CLOSED)
151 + if (src.readyState === src.CLOSED)
152 stop()
153 }
154 })
frontend/src/uploadQueue.ts
+101 -87
@@ -1,6 +1,6 @@
1 import {
2 HTTP_CONFLICT, HTTP_MESSAGES, HTTP_PAYLOAD_TOO_LARGE, UPLOAD_RESUMABLE, UPLOAD_REQUEST_STATUS, UPLOAD_RESUMABLE_HASH,
3 - buildUrlQueryString, dirname, getHFS, pathEncode, pendingPromise, prefix, randomId, tryJson, with_, wait
3 + buildUrlQueryString, dirname, getHFS, pathEncode, pendingPromise, prefix, randomId, tryJson, with_, wait, waitFor,
4 } from '@hfs/shared'
5 import { state } from './state'
6 import { getNotifications } from '@hfs/shared/api'
@@ -60,9 +60,9 @@ setInterval(() => {
60 const now = Date.now()
61 const passed = (now - bytesSentTimestamp) / 1000
62 uploadState.speed = bytesSent / passed
63 - if (now - stuckSince >= 10_000) { // this will normally cause the upload to be retried after 10+10 seconds of no progress
63 + if (currentReq && now - stuckSince >= 10_000) { // this will normally cause the upload to be retried after 10+10 seconds of no progress
64 overrideStatus = RETRY_UPLOAD // try again
65 - abortCurrentUpload()
65 + currentReq.abort()
66 }
67 bytesSent = 0 // reset counter
68 bytesSentTimestamp = now
@@ -76,8 +76,11 @@ setInterval(() => {
76 let currentReq: XMLHttpRequest | undefined
77 let overrideStatus = 0
78 let notificationChannel = ''
79 +let userAborted = false
80 let notificationSource: EventSource | undefined
81 let closeLastDialog: undefined | (() => void)
82 +let currentNotificationHandler: (name: string, data: any) => void
83 +const { OPEN } = EventSource
84
85 let reloadOnClose = false
86 export function resetReloadOnClose() {
@@ -87,13 +90,15 @@ export function resetReloadOnClose() {
90 }
91
92 export async function startUpload(toUpload: ToUpload, to: string, resume=0) {
90 - console.debug('start upload', getFilePath(toUpload.file))
93 + console.debug('start upload', getFilePath(toUpload.file), resume)
94 let resuming = false
95 let preserveTempFile = undefined
96 overrideStatus = 0
97 uploadState.uploading = toUpload
95 - await subscribeNotifications()
96 - const waitSecondChunk = pendingPromise() // this will avoid race condition, in case the notification arrives after the first chunk is finished
98 + uploadState.progress = 0
99 + userAborted = false
100 + await subscribeNotifications() // subscribe before the request
101 + const waitSecondChunk = pendingPromise() // to avoid race condition in case the notification arrives after the first chunk is finished
102 const splitSize = getHFS().splitUploads
103 const fullSize = toUpload.file.size
104 let offset = resume
@@ -102,46 +107,49 @@ export async function startUpload(toUpload: ToUpload, to: string, resume=0) {
107 const req = currentReq = new XMLHttpRequest()
108 req.timeout = 10_000
109 const finished = pendingPromise()
105 - let aborted = false
106 - req.onabort = () => aborted = true
107 - req.onloadend = () => {
108 - finished.resolve()
109 - currentReq = undefined
110 - if (overrideStatus === RETRY_UPLOAD) {
111 - overrideStatus = 0
112 - stopLooping = true
113 - startUpload(toUpload, to, offset)
114 - return
115 - }
116 - const status = overrideStatus || req.status
117 - if (!partial) // if the upload ends here, the offer for resuming must stop
118 - closeLastDialog?.()
119 - if (resuming) { // resuming requested
120 - resuming = false // this behavior is only for once, for cancellation of the upload that is in the background while resume is confirmed
121 - stopLooping = true
122 - return
123 - }
124 - if (aborted || status === HTTP_CONFLICT) // 0 = user-aborted, HTTP_CONFLICT = skipped because existing
125 - uploadState.skipped.push(toUpload)
126 - else if (status >= 400)
127 - error(status)
128 - else if (!status) // since no aborted, the request failed at a network level, so try again
129 - return
130 - else {
131 - if (splitSize) {
132 - offset += splitSize
133 - if (offset < fullSize) return // continue looping
110 + stuckSince = Date.now()
111 + let errored = false
112 + req.onerror = () => {
113 + errored = true
114 + setTimeout(() => finished.resolve(), 2000) // retry on error, but not too often
115 + }
116 + req.onloadend = async () => {
117 + if (errored && !userAborted) return
118 + try {
119 + currentReq = undefined
120 + if (overrideStatus === RETRY_UPLOAD) {
121 + finished.resolve()
122 + overrideStatus = 0
123 + stopLooping = true
124 + startUpload(toUpload, to, offset)
125 + return
126 + }
127 + const status = overrideStatus || req.status
128 + if (resuming) { // resuming requested
129 + finished.resolve()
130 + resuming = false // this behavior is only for once, for cancellation of the upload that is in the background while resume is confirmed
131 + stopLooping = true
132 + return
133 + }
134 + if (userAborted || status === HTTP_CONFLICT) // HTTP_CONFLICT = skipped because existing
135 + uploadState.skipped.push(toUpload)
136 + else if (status >= 400)
137 + error(status)
138 + else if (!status) // request failed at a network level, so try again, but not too often
139 + return await wait(2000)
140 + else {
141 + offset += splitSize || Infinity
142 + if (offset < fullSize) return // go on with the next chunk
143 + waitSecondChunk.resolve() // finished, there's no second chunk
144 + uploadState.done.push({ ...toUpload, res: tryJson(req.responseText) })
145 + uploadState.doneByte += toUpload!.file.size
146 + reloadOnClose = true
147 }
135 - uploadState.done.push({ ...toUpload, res: tryJson(req.responseText) })
136 - uploadState.doneByte += toUpload!.file.size
137 - reloadOnClose = true
148 + finished.then(next)
149 + }
150 + finally {
151 + finished.resolve()
152 }
139 - next()
140 - }
141 - req.onerror = () => {
142 - error(0)
143 - finished.resolve()
144 - stopLooping = true
153 }
154 let lastProgress = 0
155 req.upload.onprogress = (e:any) => {
@@ -166,58 +174,63 @@ export async function startUpload(toUpload: ToUpload, to: string, resume=0) {
174 }), true)
175 req.send(toUpload.file.slice(offset, splitSize ? offset + splitSize : undefined))
176 await finished
169 - if (!resume)
177 + if (!resume && notificationSource?.readyState === OPEN) // wait only if notifications are currently available
178 await waitSecondChunk
179 } while (!stopLooping && offset < fullSize)
180
181 async function subscribeNotifications() {
174 - if (notificationChannel) return
182 + // we want to getNotifications once, as establishing a connection is slow in the case of many small files;
183 + // since our handler refers to closures and needs updates, we use indirection via currentNotificationHandler
184 + currentNotificationHandler = notificationHandler
185 + if (notificationChannel) // already subscribed
186 + return waitFor(() => userAborted || notificationSource?.readyState === OPEN) // ensure the system is still alive
187 notificationChannel = 'upload-' + randomId()
188 + notificationSource = await getNotifications(notificationChannel, async (name, data) => currentNotificationHandler(name, data))
189 + }
190 +
191 + async function notificationHandler(name: string, data: any) {
192 + const {uploading} = uploadState
193 + if (!uploading) return
194 const PREFIX = 'resume hash/'
177 - notificationSource = await getNotifications(notificationChannel, async (name, data) => {
178 - const {uploading} = uploadState
179 - if (!uploading) return
180 - if (name === UPLOAD_RESUMABLE_HASH)
181 - return hfsEvent(PREFIX + data.path, data.hash)
182 - if (name === UPLOAD_RESUMABLE) {
183 - waitSecondChunk.resolve()
184 - const path = getFilePath(uploading.file)
185 - if (path !== data.path) return // is it about current file?
186 - const {size} = data //TODO use toUpload?
187 - if (!size) {
188 - preserveTempFile = undefined
189 - return
190 - }
191 - preserveTempFile = true // this is affecting only split-uploads, because is undefined on first chunk (or no chunking)
192 - if (size > toUpload.file.size) return
193 - if (data.giveBack) {
194 - // lastModified doesn't necessarily mean the file has changed, but it seems ok for the time being
195 - if (data.giveBack !== String(toUpload.file.lastModified)) // query params are always string
196 - return console.debug('upload timestamp changed')
197 - console.debug('upload unchanged')
198 - }
199 - else { // timestamp may miss if the file is left by old version, or HFS was killed
200 - const hashFromServer = new Promise<any>(res => onHfsEvent(PREFIX + path, res))
201 - const hashed = await calcHash(uploading.file, size) // therefore, we attempt a check using the hash
202 - if (!hashed) return // too late, we are working on another file
203 - if (hashed !== await hashFromServer) return console.debug('upload hash mismatch')
204 - console.debug('upload hash is matching')
205 - }
206 - closeLastDialog?.()
207 - resuming = true
208 - console.debug('resuming upload', size.toLocaleString())
195 + if (name === UPLOAD_RESUMABLE_HASH)
196 + return hfsEvent(PREFIX + data.path, data.hash)
197 + if (name === UPLOAD_RESUMABLE) {
198 + waitSecondChunk.resolve()
199 + const path = getFilePath(uploading.file)
200 + if (path !== data.path) return // is it about current file?
201 + const {size} = data //TODO use toUpload?
202 + if (!size) {
203 preserveTempFile = undefined
210 - abortCurrentUpload()
211 - await wait(500) // be sure the server had the time to react to the abort() and unlocked the file, or our next request will fail
212 - return startUpload(toUpload, to, size)
213 - }
214 - if (name === UPLOAD_REQUEST_STATUS) {
215 - overrideStatus = data?.[getFilePath(uploading.file)]
216 - if (overrideStatus >= 400)
217 - abortCurrentUpload()
204 return
205 }
220 - })
206 + preserveTempFile = true // this is affecting only split-uploads, because is undefined on first chunk (or no chunking)
207 + if (size > toUpload.file.size) return
208 + if (data.giveBack) {
209 + // lastModified doesn't necessarily mean the file has changed, but it seems ok for the time being
210 + if (data.giveBack !== String(toUpload.file.lastModified)) // query params are always string
211 + return console.debug('upload timestamp changed')
212 + console.debug('upload unchanged')
213 + }
214 + else { // timestamp may miss if the file is left by old version, or HFS was killed
215 + const hashFromServer = new Promise<any>(res => onHfsEvent(PREFIX + path, res))
216 + const hashed = await calcHash(uploading.file, size) // therefore, we attempt a check using the hash
217 + if (!hashed) return // too late, we are working on another file
218 + if (hashed !== await hashFromServer) return console.debug('upload hash mismatch')
219 + console.debug('upload hash is matching')
220 + }
221 + resuming = true
222 + console.debug('resuming upload', size.toLocaleString())
223 + preserveTempFile = undefined
224 + abortCurrentUpload()
225 + await wait(500) // be sure the server had the time to react to the abort() and unlocked the file, or our next request will fail
226 + return startUpload(toUpload, to, size)
227 + }
228 + if (name === UPLOAD_REQUEST_STATUS) {
229 + overrideStatus = data?.[getFilePath(uploading.file)]
230 + if (overrideStatus >= 400)
231 + abortCurrentUpload()
232 + return
233 + }
234 }
235
236 function error(status: number) {
@@ -259,6 +272,7 @@ export async function startUpload(toUpload: ToUpload, to: string, resume=0) {
272 }
273
274 export function abortCurrentUpload() {
275 + userAborted = true // must track because abort event isn't trigger if connection isn't established yet
276 currentReq?.abort()
277 }
278 subscribe(uploadState, () => {
frontend/src/useFetchList.ts
+1 -1
@@ -146,7 +146,7 @@ export default function useFetchList() {
146 if (op === LIST.add)
147 buffer.push(new DirEntry(par.n, par))
148 }
149 - if (src?.readyState === src?.CLOSED)
149 + if (src.readyState === src.CLOSED)
150 return state.stopSearch?.()
151 }
152 })
shared/api.ts
+1 -1
@@ -158,7 +158,7 @@ export function useApiEvents<T=any>(cmd: string, params: Dict={}) {
158 case 'closed':
159 return stop()
160 case 'msg':
161 - if (src?.readyState === src?.CLOSED)
161 + if (src.readyState === src.CLOSED)
162 return stop()
163 return setData(data)
164 }