Skip to content

Commit 2200fab

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

File tree

2 files changed

+150
-16
lines changed

2 files changed

+150
-16
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: 122 additions & 1 deletion
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,14 @@ 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+
* @param {String} method - HTTP method to stall (e.g., 'POST', 'HEAD', 'PATCH')
38+
*/
39+
simulateStallForMethod(method) {
40+
this.methodsToStall.add(method)
41+
}
42+
3443
/**
3544
* Set a custom progress sequence for the next PATCH request
3645
* @param {Array} sequence - Array of {bytes: number, delay: number} objects
@@ -46,13 +55,48 @@ class StallTestHttpStack extends TestHttpStack {
4655
createRequest(method, url) {
4756
const req = super.createRequest(method, url)
4857

49-
if (method === 'PATCH') {
58+
// Handle stalls for specific methods
59+
if (this.methodsToStall.has(method)) {
60+
this._setupMethodStall(req, method)
61+
this.methodsToStall.delete(method)
62+
} else if (method === 'PATCH') {
5063
this._setupPatchRequest(req)
5164
}
5265

5366
return req
5467
}
5568

69+
_setupMethodStall(req, method) {
70+
// Store original abort method
71+
const originalAbort = req.abort.bind(req)
72+
73+
// Override send to simulate a stall - the request starts but never progresses
74+
req.send = async function (body) {
75+
this.body = body
76+
// Set up the promise that will be rejected when abort is called
77+
this._requestPromise = new Promise((resolve, reject) => {
78+
this._rejectRequest = reject
79+
this._resolveRequest = resolve
80+
})
81+
82+
// Notify that request was sent (for nextRequest to resolve)
83+
if (req._onRequestSend) {
84+
req._onRequestSend(this)
85+
}
86+
87+
// Don't emit any progress events - simulating a complete stall
88+
return this._requestPromise
89+
}
90+
91+
// Ensure abort can reject the promise
92+
req.abort = function() {
93+
if (this._rejectRequest) {
94+
this._rejectRequest(new Error('request aborted'))
95+
}
96+
originalAbort()
97+
}
98+
}
99+
56100
_setupPatchRequest(req) {
57101
const self = this
58102

@@ -166,6 +210,52 @@ async function handleUploadCreation(testStack, location = '/uploads/12345') {
166210
return req
167211
}
168212

213+
/**
214+
* Helper function to test stall detection for a specific request method
215+
*/
216+
async function testStallDetectionForMethod(method, uploadOptions = {}) {
217+
const { enableDebugLog } = await import('tus-js-client')
218+
enableDebugLog()
219+
220+
const testStack = new StallTestHttpStack()
221+
testStack.simulateStallForMethod(method)
222+
223+
const options = {
224+
httpStack: testStack,
225+
stallDetection: {
226+
enabled: true,
227+
checkInterval: 50,
228+
stallTimeout: 200,
229+
},
230+
retryDelays: null,
231+
...uploadOptions,
232+
}
233+
234+
const { upload, options: testOptions } = createTestUpload(options)
235+
236+
// Capture console output to verify stall detection messages
237+
const originalLog = console.log
238+
let loggedMessage = ''
239+
console.log = (message) => {
240+
loggedMessage += message + '\n'
241+
}
242+
243+
upload.start()
244+
245+
// Wait for the expected request
246+
const request = await testStack.nextRequest()
247+
expect(request.method).toBe(method)
248+
249+
// Wait for stall detection to trigger and abort the request
250+
const error = await testOptions.onError.toBeCalled()
251+
252+
// Restore console
253+
console.log = originalLog
254+
255+
// Return results for assertions
256+
return { error, loggedMessage, request }
257+
}
258+
169259
describe('tus-stall-detection', () => {
170260
describe('integration tests', () => {
171261
it("should not enable stall detection if HTTP stack doesn't support progress events", async () => {
@@ -393,5 +483,36 @@ describe('tus-stall-detection', () => {
393483
expect(error.message).toContain('stalled: no progress')
394484
expect(options.onProgress.calls.count()).toBeGreaterThan(0)
395485
})
486+
487+
it('should detect stalls during POST request (upload creation)', async () => {
488+
const { error, loggedMessage, request } = await testStallDetectionForMethod('POST')
489+
490+
expect(request.url).toBe('https://tus.io/uploads')
491+
492+
// Verify the error is related to request abortion due to stall
493+
expect(error.message).toContain('request aborted')
494+
expect(error.message).toContain('POST')
495+
496+
// Verify stall detection was started and detected
497+
expect(loggedMessage).toContain('starting stall detection')
498+
expect(loggedMessage).toContain('upload stalled')
499+
})
500+
501+
it('should detect stalls during HEAD request (resuming upload)', async () => {
502+
const { error, loggedMessage, request } = await testStallDetectionForMethod('HEAD', {
503+
// Provide an uploadUrl to simulate resuming
504+
uploadUrl: 'https://tus.io/uploads/existing',
505+
})
506+
507+
expect(request.url).toBe('https://tus.io/uploads/existing')
508+
509+
// Verify the error is related to request abortion due to stall
510+
expect(error.message).toContain('request aborted')
511+
expect(error.message).toContain('HEAD')
512+
513+
// Verify stall detection was started and detected
514+
expect(loggedMessage).toContain('starting stall detection')
515+
expect(loggedMessage).toContain('upload stalled')
516+
})
396517
})
397518
})

0 commit comments

Comments
 (0)