Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't close socket while upload is still in progress #4479

Merged
merged 3 commits into from
Jun 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 11 additions & 7 deletions packages/@uppy/aws-s3-multipart/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -716,38 +716,40 @@ export default class AwsS3Multipart extends BasePlugin {
this.uploaderEvents[file.id] = new EventTracker(this.uppy)

this.onFileRemove(file.id, () => {
queuedRequest.abort()
socket.send('cancel', {})
queuedRequest.abort()
this.resetUploaderReferences(file.id, { abort: true })
resolve(`upload ${file.id} was removed`)
})

this.onFilePause(file.id, (isPaused) => {
if (isPaused) {
// Remove this file from the queue so another file can start in its place.
queuedRequest.abort()
socket.send('pause', {})
queuedRequest.abort()
} else {
// Resuming an upload should be queued, else you could pause and then
// resume a queued upload to make it skip the queue.
queuedRequest.abort()
queuedRequest = this.requests.run(() => {
socket.open()
socket.send('resume', {})
return () => socket.close()
return () => {}
})
}
})

this.onPauseAll(file.id, () => {
queuedRequest.abort()
// First send the message, then call .abort,
// just to make sure socket is not closed, which .abort used to do
socket.send('pause', {})
queuedRequest.abort()
})

this.onCancelAll(file.id, ({ reason } = {}) => {
if (reason === 'user') {
queuedRequest.abort()
socket.send('cancel', {})
queuedRequest.abort()
this.resetUploaderReferences(file.id)
}
resolve(`upload ${file.id} was canceled`)
Expand All @@ -762,7 +764,7 @@ export default class AwsS3Multipart extends BasePlugin {
socket.open()
socket.send('resume', {})

return () => socket.close()
return () => {}
})
})

Expand All @@ -789,6 +791,7 @@ export default class AwsS3Multipart extends BasePlugin {
socket.on('error', (errData) => {
this.uppy.emit('upload-error', file, new Error(errData.error))
this.resetUploaderReferences(file.id)
socket.close()
queuedRequest.done()
reject(new Error(errData.error))
})
Expand All @@ -800,6 +803,7 @@ export default class AwsS3Multipart extends BasePlugin {

this.uppy.emit('upload-success', file, uploadResp)
this.resetUploaderReferences(file.id)
socket.close()
queuedRequest.done()
resolve()
})
Expand All @@ -811,7 +815,7 @@ export default class AwsS3Multipart extends BasePlugin {
socket.open()
}

return () => socket.close()
return () => {}
})
})
}
Expand Down
16 changes: 8 additions & 8 deletions packages/@uppy/tus/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -498,17 +498,17 @@ export default class Tus extends BasePlugin {
let queuedRequest

this.onFileRemove(file.id, () => {
queuedRequest.abort()
socket.send('cancel', {})
queuedRequest.abort()
this.resetUploaderReferences(file.id)
resolve(`upload ${file.id} was removed`)
})

this.onPause(file.id, (isPaused) => {
if (isPaused) {
// Remove this file from the queue so another file can start in its place.
queuedRequest.abort()
socket.send('pause', {})
queuedRequest.abort()
} else {
// Resuming an upload should be queued, else you could pause and then
// resume a queued upload to make it skip the queue.
Expand All @@ -517,20 +517,20 @@ export default class Tus extends BasePlugin {
socket.open()
socket.send('resume', {})

return () => socket.close()
return () => {}
})
}
})

this.onPauseAll(file.id, () => {
queuedRequest.abort()
socket.send('pause', {})
queuedRequest.abort()
})

this.onCancelAll(file.id, ({ reason } = {}) => {
if (reason === 'user') {
queuedRequest.abort()
socket.send('cancel', {})
queuedRequest.abort()
this.resetUploaderReferences(file.id)
}
resolve(`upload ${file.id} was canceled`)
Expand All @@ -545,7 +545,7 @@ export default class Tus extends BasePlugin {
socket.open()
socket.send('resume', {})

return () => socket.close()
return () => {}
})
})

Expand Down Expand Up @@ -599,7 +599,7 @@ export default class Tus extends BasePlugin {
this.uppy.emit('upload-success', file, uploadResp)
this.resetUploaderReferences(file.id)
queuedRequest.done()

socket.close()
resolve()
})

Expand All @@ -616,7 +616,7 @@ export default class Tus extends BasePlugin {
// that point this cancellation function is not going to be called.
// Also, we need to remove the request from the queue _without_ destroying everything
// related to this upload to handle pauses.
return () => socket.close()
return () => {}
})
})
}
Expand Down
17 changes: 7 additions & 10 deletions packages/@uppy/xhr-upload/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,7 @@ export default class XHRUpload extends BasePlugin {
: Object.assign(new Error(errData.error.message), { cause: errData.error })
this.uppy.emit('upload-error', file, error)
queuedRequest.done() // eslint-disable-line no-use-before-define
socket.close()
if (this.uploaderEvents[file.id]) {
this.uploaderEvents[file.id].remove()
this.uploaderEvents[file.id] = null
Expand All @@ -451,11 +452,12 @@ export default class XHRUpload extends BasePlugin {
createSocket()
}

return () => socket.close()
return () => {}
})

this.onFileRemove(file.id, () => {
socket?.send('cancel', {})
socket.close()
queuedRequest.abort()
resolve(`upload ${file.id} was removed`)
})
Expand All @@ -464,6 +466,7 @@ export default class XHRUpload extends BasePlugin {
if (reason === 'user') {
socket?.send('cancel', {})
queuedRequest.abort()
// socket.close()
}
resolve(`upload ${file.id} was canceled`)
})
Expand All @@ -472,19 +475,13 @@ export default class XHRUpload extends BasePlugin {
if (socket == null) {
queuedRequest.abort()
} else {
socket.send('pause', {})
queuedRequest.done()
}
queuedRequest = this.requests.run(() => {
if (!file.isPaused) {
if (socket == null) {
createSocket()
} else {
socket.send('resume', {})
}
if (socket == null) {
createSocket()
}

return () => socket.close()
return () => {}
})
}
this.onRetry(file.id, onRetryRequest)
Expand Down