fix: in case of resumable upload, split-upload was not temporarily creating a new resumable
Massimo Melina committed
Jan 2, 2025 at 13:59 UTC
dd846a41548f1ba86ee2cd8ba20490ceacaf42d7
3 files changed
+26
-9
frontend/src/uploadQueue.ts
+12
-3
@@ -1,7 +1,7 @@
1
import {
2
buildUrlQueryString, dirname, formatBytes, formatPerc, getHFS,
3
HTTP_CONFLICT, HTTP_MESSAGES, HTTP_PAYLOAD_TOO_LARGE, UPLOAD_RESUMABLE, UPLOAD_STATUS,
4
- pathEncode, pendingPromise, prefix, randomId, tryJson, with_
4
+ pathEncode, pendingPromise, prefix, randomId, tryJson, with_, wait
5
} from '@hfs/shared'
6
import { state } from './state'
7
import { getNotifications } from '@hfs/shared/api'
@@ -89,9 +89,11 @@ export function resetReloadOnClose() {
89
90
export async function startUpload(toUpload: ToUpload, to: string, resume=0) {
91
let resuming = false
92
+ let preserveTempFile = undefined
93
overrideStatus = 0
94
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
97
const splitSize = getHFS().splitUploads
98
const fullSize = toUpload.file.size
99
let offset = resume
@@ -152,12 +154,14 @@ export async function startUpload(toUpload: ToUpload, to: string, resume=0) {
154
req.open('PUT', to + pathEncode(uploadPath) + buildUrlQueryString({
155
notificationChannel,
156
...partial && { partial: 'y' },
155
- ...offset && { resume: String(offset) },
157
+ ...offset && { resume: offset, preserveTempFile },
158
...toUpload.comment && { comment: toUpload.comment },
159
...with_(state.uploadOnExisting, x => x !== 'rename' && { existing: x }), // rename is the default
160
}), true)
161
req.send(toUpload.file.slice(offset, splitSize ? offset + splitSize : undefined))
162
await finished
163
+ if (!resume)
164
+ await waitSecondChunk
165
} while (!stopLooping && offset < fullSize)
166
167
async function subscribeNotifications() {
@@ -167,8 +171,11 @@ export async function startUpload(toUpload: ToUpload, to: string, resume=0) {
171
const {uploading} = uploadState
172
if (!uploading) return
173
if (name === UPLOAD_RESUMABLE) {
174
+ waitSecondChunk.resolve()
175
const size = data?.[getFilePath(uploading.file)] //TODO use toUpload?
171
- if (!size || size > toUpload.file.size) return
176
+ if (!size) return
177
+ preserveTempFile = true // this is affecting only split-uploads, because is undefined on first chunk (or no chunking)
178
+ if (size > toUpload.file.size) return
179
const {expires} = data
180
const timeout = typeof expires !== 'number' ? 0
181
: (Number(new Date(expires)) - Date.now()) / 1000
@@ -183,7 +190,9 @@ export async function startUpload(toUpload: ToUpload, to: string, resume=0) {
190
if (!confirmed) return
191
if (uploading !== uploadState.uploading) return // too late
192
resuming = true
193
+ preserveTempFile = undefined
194
abortCurrentUpload()
195
+ 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
196
return startUpload(toUpload, to, size)
197
}
198
if (name === UPLOAD_STATUS) {
shared/index.ts
+1
-1
@@ -51,7 +51,7 @@ function getScriptAttr(k: string) {
51
}
52
53
export function buildUrlQueryString(params: Dict) { // not using URLSearchParams.toString as it doesn't work on firefox50
54
- return '?' + Object.entries(params).map(pair => pair.map(encodeURIComponent).join('=') ).join('&')
54
+ return '?' + Object.entries(params).filter(pair => pair[1] !== undefined).map(pair => pair.map(encodeURIComponent).join('=') ).join('&')
55
}
56
57
export function domOn<K extends keyof WindowEventMap>(eventName: K, cb: (ev: WindowEventMap[K]) => void, { target=window }={}) {
src/upload.ts
+13
-5
@@ -96,17 +96,23 @@ export function uploadWriter(base: VfsNode, baseUri: string, path: string, ctx:
96
setUploadMeta(dir, ctx)
97
// use temporary name while uploading
98
const keepName = basename(fullPath).slice(-200)
99
- let tempName = join(dir, 'hfs$upload-' + keepName)
100
- const resumableSize = try_(() => fs.statSync(tempName).size) || 0 // we use size to even when user has not required resume, yet, to notify frontend of the possibility
101
- const resumableTempName = resumableSize > 0 && tempName // resumableTempName is temp-file-1
99
+ const firstTempName = join(dir, 'hfs$upload-' + keepName)
100
+ const altTempName = join(dir, 'hfs$upload2-' + keepName)
101
+ const splitAndPreserving = ctx.query.preserveTempFile // frontend knows about existing temp that can be resumed, but it is not resuming that, but instead it is continuing split-uploading on alternative temp file
102
+ let tempName = splitAndPreserving ? altTempName : firstTempName
103
+ const stats = try_(() => fs.statSync(tempName))
104
+ const resumableSize = stats?.size || 0 // we use size to even when user has not required resume, yet, to notify frontend of the possibility
105
+ const resumableTempName = resumableSize > 0 && tempName
106
if (resumableTempName)
103
- tempName = join(dir, 'hfs$upload2-' + keepName)
107
+ tempName = altTempName
108
// checks for resume feature
109
let resume = Number(ctx.query.resume)
110
if (resume > resumableSize)
111
return fail(HTTP_RANGE_NOT_SATISFIABLE)
112
// warn frontend about resume possibility
113
let resumableLost = false
114
+ if (!resume && !resumableTempName)
115
+ notifyClient(ctx, UPLOAD_RESUMABLE, { [path]: 0 })
116
if (!resume && resumableTempName) {
117
const timeout = 30
118
notifyClient(ctx, UPLOAD_RESUMABLE, { [path]: resumableSize, expires: Date.now() + timeout * 1000 })
@@ -122,7 +128,7 @@ export function uploadWriter(base: VfsNode, baseUri: string, path: string, ctx:
128
if (!resuming)
129
resume = 0
130
const writeStream = createStreamLimiter(reqSize ?? Infinity)
125
- if (resuming) {
131
+ if (resuming && !splitAndPreserving) {
132
fs.rm(tempName, () => {})
133
tempName = resumableTempName
134
}
@@ -169,6 +175,8 @@ export function uploadWriter(base: VfsNode, baseUri: string, path: string, ctx:
175
try {
176
await rename(tempName, dest)
177
cancelDeletion(tempName) // not necessary, as deletion's failure is silent, but still
178
+ if (splitAndPreserving) // we've been using altTempName, but now we're done, so we can delete firstTempName
179
+ delayedDelete(firstTempName, 0)
180
ctx.state.uploadDestinationPath = dest
181
setUploadMeta(dest, ctx)
182
if (ctx.query.comment)