Ensure stallDetector is supplied for HEAD and POST requests

m7kvqbe1 · m7kvqbe1 · commit 44b87ab8bd15 · 2025-10-02T14:18:21.000+01:00
diff --git a/lib/upload.ts b/lib/upload.ts
@@ -407,7 +407,9 @@ export class BaseUpload {
 
     let res: HttpResponse
     try {
-      res = await this._sendRequest(req)
+      // Create stall detector for final concatenation POST request
+      const stallDetector = this._createStallDetector()
+      res = await this._sendRequest(req, undefined, stallDetector)
     } catch (err) {
       if (!(err instanceof Error)) {
         throw new Error(`tus: value thrown that is not an error: ${err}`)
@@ -638,7 +640,9 @@ export class BaseUpload {
         ) {
           req.setHeader('Upload-Complete', '?0')
         }
-        res = await this._sendRequest(req)
+        // Create stall detector for POST request
+        const stallDetector = this._createStallDetector()
+        res = await this._sendRequest(req, undefined, stallDetector)
       }
     } catch (err) {
       if (!(err instanceof Error)) {
@@ -700,7 +704,9 @@ export class BaseUpload {
 
     let res: HttpResponse
     try {
-      res = await this._sendRequest(req)
+      // Create stall detector for HEAD request
+      const stallDetector = this._createStallDetector()
+      res = await this._sendRequest(req, undefined, stallDetector)
     } catch (err) {
       if (!(err instanceof Error)) {
         throw new Error(`tus: value thrown that is not an error: ${err}`)
@@ -852,23 +858,15 @@ export class BaseUpload {
   }
 
   /**
-   * _addChunktoRequest reads a chunk from the source and sends it using the
-   * supplied request object. It will not handle the response.
+   * Create a stall detector if stall detection is enabled and supported.
    *
    * @api private
    */
-  private async _addChunkToRequest(req: HttpRequest): Promise<HttpResponse> {
-    const start = this._offset
-    let end = this._offset + this.options.chunkSize
-
-    // Create stall detector for this request if stall detection is enabled and supported
-    // but don't start it yet - we'll start it after onBeforeRequest completes
-    let stallDetector: StallDetector | undefined
-
+  private _createStallDetector(): StallDetector | undefined {
     if (this.options.stallDetection?.enabled) {
       // Only enable stall detection if the HTTP stack supports progress events
       if (this.options.httpStack.supportsProgressEvents()) {
-        stallDetector = new StallDetector(this.options.stallDetection, (reason: string) => {
+        return new StallDetector(this.options.stallDetection, (reason: string) => {
           // Handle stall by aborting the current request
           // The abort will cause the request to fail, which will be caught
           // in _performUpload and wrapped in a DetailedError for proper retry handling
@@ -878,13 +876,28 @@ export class BaseUpload {
           }
           // Don't call _retryOrEmitError here - let the natural error flow handle it
         })
-        // Don't start yet - will be started after onBeforeRequest
       } else {
         log(
           'tus: stall detection is enabled but the HTTP stack does not support progress events, it will be disabled for this upload',
         )
       }
     }
+    return undefined
+  }
+
+  /**
+   * _addChunktoRequest reads a chunk from the source and sends it using the
+   * supplied request object. It will not handle the response.
+   *
+   * @api private
+   */
+  private async _addChunkToRequest(req: HttpRequest): Promise<HttpResponse> {
+    const start = this._offset
+    let end = this._offset + this.options.chunkSize
+
+    // Create stall detector for this request if stall detection is enabled and supported
+    // but don't start it yet - we'll start it after onBeforeRequest completes
+    const stallDetector = this._createStallDetector()
 
     req.setProgressHandler((bytesSent) => {
       // Update per-request stall detector if active
diff --git a/test/spec/test-stall-detection.js b/test/spec/test-stall-detection.js
@@ -22,6 +22,7 @@ class StallTestHttpStack extends TestHttpStack {
     this.progressSequences = new Map()
     this.progressPromises = new Map()
     this.nextProgressSequence = null
+    this.methodsToStall = new Set()
   }
 
   /**
@@ -31,6 +32,14 @@ class StallTestHttpStack extends TestHttpStack {
     this.stallOnNextPatch = true
   }
 
+  /**
+   * Configure the stack to simulate a stall for a specific HTTP method
+   * @param {String} method - HTTP method to stall (e.g., 'POST', 'HEAD', 'PATCH')
+   */
+  simulateStallForMethod(method) {
+    this.methodsToStall.add(method)
+  }
+
   /**
    * Set a custom progress sequence for the next PATCH request
    * @param {Array} sequence - Array of {bytes: number, delay: number} objects
@@ -46,38 +55,62 @@ class StallTestHttpStack extends TestHttpStack {
   createRequest(method, url) {
     const req = super.createRequest(method, url)
 
-    if (method === 'PATCH') {
+    if (this.methodsToStall.has(method)) {
+      this._setupMethodStall(req, method)
+      this.methodsToStall.delete(method)
+    } else if (method === 'PATCH') {
       this._setupPatchRequest(req)
     }
 
     return req
   }
 
+  _setupMethodStall(req, method) {
+    const originalAbort = req.abort.bind(req)
+
+    req.send = async function (body) {
+      this.body = body
+      this._requestPromise = new Promise((resolve, reject) => {
+        this._rejectRequest = reject
+        this._resolveRequest = resolve
+      })
+
+      if (req._onRequestSend) {
+        req._onRequestSend(this)
+      }
+
+      return this._requestPromise
+    }
+
+    req.abort = function() {
+      if (this._rejectRequest) {
+        this._rejectRequest(new Error('request aborted'))
+      }
+      originalAbort()
+    }
+  }
+
   _setupPatchRequest(req) {
     const self = this
 
-    // Handle complete stalls
     if (this.stallOnNextPatch) {
       this.stallOnNextPatch = false
       req.send = async function (body) {
         this.body = body
         if (body) {
           this.bodySize = await getBodySize(body)
-          // Don't call progress handler to simulate a complete stall
         }
         this._onRequestSend(this)
         return this._requestPromise
       }
       return
     }
 
-    // Handle progress sequences
     if (this.nextProgressSequence) {
       this.progressSequences.set(req, this.nextProgressSequence)
       this.nextProgressSequence = null
     }
 
-    // Override respondWith to wait for progress events
     const originalRespondWith = req.respondWith.bind(req)
     req.respondWith = async (resData) => {
       const progressPromise = self.progressPromises.get(req)
@@ -88,7 +121,6 @@ class StallTestHttpStack extends TestHttpStack {
       originalRespondWith(resData)
     }
 
-    // Override send to handle progress sequences
     req.send = async function (body) {
       this.body = body
       if (body) {
@@ -115,7 +147,7 @@ class StallTestHttpStack extends TestHttpStack {
           progressHandler(event.bytes)
         }
         resolve()
-      }, 10) // Small delay to ensure stall detector is started
+      }, 10)
     })
     this.progressPromises.set(req, progressPromise)
   }
@@ -126,7 +158,7 @@ class StallTestHttpStack extends TestHttpStack {
         progressHandler(0)
         progressHandler(bodySize)
         resolve()
-      }, 10) // Small delay to ensure stall detector is started
+      }, 10)
     })
     this.progressPromises.set(req, progressPromise)
   }
@@ -166,6 +198,47 @@ async function handleUploadCreation(testStack, location = '/uploads/12345') {
   return req
 }
 
+/**
+ * Helper function to test stall detection for a specific request method
+ */
+async function testStallDetectionForMethod(method, uploadOptions = {}) {
+  const { enableDebugLog } = await import('tus-js-client')
+  enableDebugLog()
+
+  const testStack = new StallTestHttpStack()
+  testStack.simulateStallForMethod(method)
+
+  const options = {
+    httpStack: testStack,
+    stallDetection: {
+      enabled: true,
+      checkInterval: 50,
+      stallTimeout: 200,
+    },
+    retryDelays: null,
+    ...uploadOptions,
+  }
+
+  const { upload, options: testOptions } = createTestUpload(options)
+
+  const originalLog = console.log
+  let loggedMessage = ''
+  console.log = (message) => {
+    loggedMessage += message + '\n'
+  }
+
+  upload.start()
+
+  const request = await testStack.nextRequest()
+  expect(request.method).toBe(method)
+
+  const error = await testOptions.onError.toBeCalled()
+
+  console.log = originalLog
+
+  return { error, loggedMessage, request }
+}
+
 describe('tus-stall-detection', () => {
   describe('integration tests', () => {
     it("should not enable stall detection if HTTP stack doesn't support progress events", async () => {
@@ -393,5 +466,27 @@ describe('tus-stall-detection', () => {
       expect(error.message).toContain('stalled: no progress')
       expect(options.onProgress.calls.count()).toBeGreaterThan(0)
     })
+
+    it('should detect stalls during POST request (upload creation)', async () => {
+      const { error, loggedMessage, request } = await testStallDetectionForMethod('POST')
+
+      expect(request.url).toBe('https://tus.io/uploads')
+      expect(error.message).toContain('request aborted')
+      expect(error.message).toContain('POST')
+      expect(loggedMessage).toContain('starting stall detection')
+      expect(loggedMessage).toContain('upload stalled')
+    })
+
+    it('should detect stalls during HEAD request (resuming upload)', async () => {
+      const { error, loggedMessage, request } = await testStallDetectionForMethod('HEAD', {
+        uploadUrl: 'https://tus.io/uploads/existing',
+      })
+
+      expect(request.url).toBe('https://tus.io/uploads/existing')
+      expect(error.message).toContain('request aborted')
+      expect(error.message).toContain('HEAD')
+      expect(loggedMessage).toContain('starting stall detection')
+      expect(loggedMessage).toContain('upload stalled')
+    })
   })
 })
