Skip to content

Commit 6913195

Browse files
committed
Ensure stallDetector is supplied for HEAD and POST requests
1 parent e4175c3 commit 6913195

File tree

2 files changed

+141
-23
lines changed

2 files changed

+141
-23
lines changed

lib/upload.ts

Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -407,7 +407,9 @@ export class BaseUpload {
407407

408408
let res: HttpResponse
409409
try {
410-
res = await this._sendRequest(req)
410+
// Create stall detector for final concatenation POST request
411+
const stallDetector = this._createStallDetector()
412+
res = await this._sendRequest(req, undefined, stallDetector)
411413
} catch (err) {
412414
if (!(err instanceof Error)) {
413415
throw new Error(`tus: value thrown that is not an error: ${err}`)
@@ -638,7 +640,9 @@ export class BaseUpload {
638640
) {
639641
req.setHeader('Upload-Complete', '?0')
640642
}
641-
res = await this._sendRequest(req)
643+
// Create stall detector for POST request
644+
const stallDetector = this._createStallDetector()
645+
res = await this._sendRequest(req, undefined, stallDetector)
642646
}
643647
} catch (err) {
644648
if (!(err instanceof Error)) {
@@ -700,7 +704,9 @@ export class BaseUpload {
700704

701705
let res: HttpResponse
702706
try {
703-
res = await this._sendRequest(req)
707+
// Create stall detector for HEAD request
708+
const stallDetector = this._createStallDetector()
709+
res = await this._sendRequest(req, undefined, stallDetector)
704710
} catch (err) {
705711
if (!(err instanceof Error)) {
706712
throw new Error(`tus: value thrown that is not an error: ${err}`)
@@ -852,23 +858,15 @@ export class BaseUpload {
852858
}
853859

854860
/**
855-
* _addChunktoRequest reads a chunk from the source and sends it using the
856-
* supplied request object. It will not handle the response.
861+
* Create a stall detector if stall detection is enabled and supported.
857862
*
858863
* @api private
859864
*/
860-
private async _addChunkToRequest(req: HttpRequest): Promise<HttpResponse> {
861-
const start = this._offset
862-
let end = this._offset + this.options.chunkSize
863-
864-
// Create stall detector for this request if stall detection is enabled and supported
865-
// but don't start it yet - we'll start it after onBeforeRequest completes
866-
let stallDetector: StallDetector | undefined
867-
865+
private _createStallDetector(): StallDetector | undefined {
868866
if (this.options.stallDetection?.enabled) {
869867
// Only enable stall detection if the HTTP stack supports progress events
870868
if (this.options.httpStack.supportsProgressEvents()) {
871-
stallDetector = new StallDetector(this.options.stallDetection, (reason: string) => {
869+
return new StallDetector(this.options.stallDetection, (reason: string) => {
872870
// Handle stall by aborting the current request
873871
// The abort will cause the request to fail, which will be caught
874872
// in _performUpload and wrapped in a DetailedError for proper retry handling
@@ -878,13 +876,28 @@ export class BaseUpload {
878876
}
879877
// Don't call _retryOrEmitError here - let the natural error flow handle it
880878
})
881-
// Don't start yet - will be started after onBeforeRequest
882879
} else {
883880
log(
884881
'tus: stall detection is enabled but the HTTP stack does not support progress events, it will be disabled for this upload',
885882
)
886883
}
887884
}
885+
return undefined
886+
}
887+
888+
/**
889+
* _addChunktoRequest reads a chunk from the source and sends it using the
890+
* supplied request object. It will not handle the response.
891+
*
892+
* @api private
893+
*/
894+
private async _addChunkToRequest(req: HttpRequest): Promise<HttpResponse> {
895+
const start = this._offset
896+
let end = this._offset + this.options.chunkSize
897+
898+
// Create stall detector for this request if stall detection is enabled and supported
899+
// but don't start it yet - we'll start it after onBeforeRequest completes
900+
const stallDetector = this._createStallDetector()
888901

889902
req.setProgressHandler((bytesSent) => {
890903
// Update per-request stall detector if active

test/spec/test-stall-detection.js

Lines changed: 113 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ class StallTestHttpStack extends TestHttpStack {
2222
this.progressSequences = new Map()
2323
this.progressPromises = new Map()
2424
this.nextProgressSequence = null
25+
this.methodsToStall = new Set()
2526
}
2627

2728
/**
@@ -31,6 +32,19 @@ class StallTestHttpStack extends TestHttpStack {
3132
this.stallOnNextPatch = true
3233
}
3334

35+
/**
36+
* Configure the stack to simulate a stall for a specific HTTP method
37+
*
38+
* When this is called, the specified HTTP method will stall on the next request.
39+
* The stall is created by returning a promise that never resolves or rejects,
40+
* simulating a network request that starts but never receives any data.
41+
*
42+
* @param {String} method - HTTP method to stall (e.g., 'POST', 'HEAD', 'PATCH')
43+
*/
44+
simulateStallForMethod(method) {
45+
this.methodsToStall.add(method)
46+
}
47+
3448
/**
3549
* Set a custom progress sequence for the next PATCH request
3650
* @param {Array} sequence - Array of {bytes: number, delay: number} objects
@@ -46,38 +60,67 @@ class StallTestHttpStack extends TestHttpStack {
4660
createRequest(method, url) {
4761
const req = super.createRequest(method, url)
4862

49-
if (method === 'PATCH') {
63+
if (this.methodsToStall.has(method)) {
64+
this._setupMethodStall(req, method)
65+
this.methodsToStall.delete(method)
66+
} else if (method === 'PATCH') {
5067
this._setupPatchRequest(req)
5168
}
5269

5370
return req
5471
}
5572

73+
_setupMethodStall(req, method) {
74+
const originalAbort = req.abort.bind(req)
75+
76+
req.send = async function (body) {
77+
this.body = body
78+
79+
// We create a promise but never resolve or reject it, this
80+
// simulates a network request that starts but never completes
81+
this._requestPromise = new Promise((resolve, reject) => {
82+
this._rejectRequest = reject
83+
this._resolveRequest = resolve
84+
})
85+
86+
if (req._onRequestSend) {
87+
req._onRequestSend(this)
88+
}
89+
90+
// Return the hanging promise - the caller will await this forever
91+
// (until StallDetector calls abort after timeout)
92+
return this._requestPromise
93+
}
94+
95+
req.abort = function() {
96+
if (this._rejectRequest) {
97+
this._rejectRequest(new Error('request aborted'))
98+
}
99+
originalAbort()
100+
}
101+
}
102+
56103
_setupPatchRequest(req) {
57104
const self = this
58105

59-
// Handle complete stalls
60106
if (this.stallOnNextPatch) {
61107
this.stallOnNextPatch = false
62108
req.send = async function (body) {
63109
this.body = body
64110
if (body) {
65111
this.bodySize = await getBodySize(body)
66-
// Don't call progress handler to simulate a complete stall
67112
}
68113
this._onRequestSend(this)
69114
return this._requestPromise
70115
}
71116
return
72117
}
73118

74-
// Handle progress sequences
75119
if (this.nextProgressSequence) {
76120
this.progressSequences.set(req, this.nextProgressSequence)
77121
this.nextProgressSequence = null
78122
}
79123

80-
// Override respondWith to wait for progress events
81124
const originalRespondWith = req.respondWith.bind(req)
82125
req.respondWith = async (resData) => {
83126
const progressPromise = self.progressPromises.get(req)
@@ -88,7 +131,6 @@ class StallTestHttpStack extends TestHttpStack {
88131
originalRespondWith(resData)
89132
}
90133

91-
// Override send to handle progress sequences
92134
req.send = async function (body) {
93135
this.body = body
94136
if (body) {
@@ -115,7 +157,7 @@ class StallTestHttpStack extends TestHttpStack {
115157
progressHandler(event.bytes)
116158
}
117159
resolve()
118-
}, 10) // Small delay to ensure stall detector is started
160+
}, 10)
119161
})
120162
this.progressPromises.set(req, progressPromise)
121163
}
@@ -126,7 +168,7 @@ class StallTestHttpStack extends TestHttpStack {
126168
progressHandler(0)
127169
progressHandler(bodySize)
128170
resolve()
129-
}, 10) // Small delay to ensure stall detector is started
171+
}, 10)
130172
})
131173
this.progressPromises.set(req, progressPromise)
132174
}
@@ -166,6 +208,47 @@ async function handleUploadCreation(testStack, location = '/uploads/12345') {
166208
return req
167209
}
168210

211+
/**
212+
* Helper function to test stall detection for a specific request method
213+
*/
214+
async function testStallDetectionForMethod(method, uploadOptions = {}) {
215+
const { enableDebugLog } = await import('tus-js-client')
216+
enableDebugLog()
217+
218+
const testStack = new StallTestHttpStack()
219+
testStack.simulateStallForMethod(method)
220+
221+
const options = {
222+
httpStack: testStack,
223+
stallDetection: {
224+
enabled: true,
225+
checkInterval: 50,
226+
stallTimeout: 200,
227+
},
228+
retryDelays: null,
229+
...uploadOptions,
230+
}
231+
232+
const { upload, options: testOptions } = createTestUpload(options)
233+
234+
const originalLog = console.log
235+
let loggedMessage = ''
236+
console.log = (message) => {
237+
loggedMessage += message + '\n'
238+
}
239+
240+
upload.start()
241+
242+
const request = await testStack.nextRequest()
243+
expect(request.method).toBe(method)
244+
245+
const error = await testOptions.onError.toBeCalled()
246+
247+
console.log = originalLog
248+
249+
return { error, loggedMessage, request }
250+
}
251+
169252
describe('tus-stall-detection', () => {
170253
describe('integration tests', () => {
171254
it("should not enable stall detection if HTTP stack doesn't support progress events", async () => {
@@ -393,5 +476,27 @@ describe('tus-stall-detection', () => {
393476
expect(error.message).toContain('stalled: no progress')
394477
expect(options.onProgress.calls.count()).toBeGreaterThan(0)
395478
})
479+
480+
it('should detect stalls during POST request (upload creation)', async () => {
481+
const { error, loggedMessage, request } = await testStallDetectionForMethod('POST')
482+
483+
expect(request.url).toBe('https://tus.io/uploads')
484+
expect(error.message).toContain('request aborted')
485+
expect(error.message).toContain('POST')
486+
expect(loggedMessage).toContain('starting stall detection')
487+
expect(loggedMessage).toContain('upload stalled')
488+
})
489+
490+
it('should detect stalls during HEAD request (resuming upload)', async () => {
491+
const { error, loggedMessage, request } = await testStallDetectionForMethod('HEAD', {
492+
uploadUrl: 'https://tus.io/uploads/existing',
493+
})
494+
495+
expect(request.url).toBe('https://tus.io/uploads/existing')
496+
expect(error.message).toContain('request aborted')
497+
expect(error.message).toContain('HEAD')
498+
expect(loggedMessage).toContain('starting stall detection')
499+
expect(loggedMessage).toContain('upload stalled')
500+
})
396501
})
397502
})

0 commit comments

Comments
 (0)