Missing default timeout in NodeHttpStack causes uploads to hang indefinitely

[m7kvqbe1]

Issue

When using tus-js-client in Node.js environments, I've discovered that uploads can hang indefinitely with no retry mechanism being triggered, particularly in poor or unreliable network conditions (like satellite connections).

After investigating, I found that NodeHttpStack does not specify any default timeout for HTTP requests. Since Node.js defaults to no timeout (0 = infinite), this means requests can remain in a "stalled" state forever without ever triggering an error that would initiate the retry mechanism.

Impact

This is a critical issue for resumable uploads, especially in environments with unreliable connections:

• Uploads silently hang with no errors or retries
• Even when network conditions improve, stalled uploads remain stalled due to TCP backoff
• Resources remain tied up indefinitely
• The retry mechanism (a key feature of the library) is never triggered for these cases

Reproduction

1. Create a Node.js service using tus-js-client
2. Initiate an upload over an unreliable connection
3. During upload, introduce severe packet loss or connection degradation
4. Observe that the upload stalls indefinitely with no retry

Workaround

I've had to copy paste the NodeHttpStack module into our own codebase and modify it to include timeouts. I would have preferred to import the module directly, but it's not properly exported from your library in a way that makes it easily accessible.

The fix was simple:

const httpStack = new NodeHttpStack({
  timeout: 30000, // 30 seconds
})

Proposed Solution

A simple fix would be to:

1. Add a reasonable default timeout to the NodeHttpStack constructor
2. Properly export the NodeHttpStack class so it can be imported directly (you let us inject a custom implementation)

This would ensure that hanging connections eventually trigger the retry mechanism, which is essential for a resumable upload library designed for unreliable networks.

Questions

1. Was this omission intentional, or was it an oversight?
2. Are there any technical reasons why a default timeout wasn't included?
3. Would you consider adding a reasonable default timeout in a future release?

[Acconut]

tus-js-client doesn't set default timeout because it's not possible to choose a good default value. Small uploads might be finished in a few seconds, so a 1min timeout seems good. Large uploads might run for multiple hours and 1min would interrupt the data transfer unnecessarily. In addition, some server might not save uploaded data if requests get interrupted. In these cases, uploads might not progress at all if timeouts constantly prevent requests from finishing.

A better solution would be to monitor the upload progress events and determine if requests have stalled. A bit similar to curl's speed limits options: https://daniel.haxx.se/blog/2020/05/11/curl-ootw-y-speed-limit/ Such logic could be part of tus-js-client and then user can configure it as they want. One caveat is that not every HTTP stack supports upload progress events. It's available in XMLHttpRequest and Node.js' http module, but not for the Fetch API.

[m7kvqbe1]

Thanks for your response!

Just to clarify a key point:

I think socket timeouts only trigger when no data flows for the specified period - they don't limit total upload duration. A 10-hour upload works fine with a 30-second timeout as long as data continues to transfer.

I like your progress-based stall detection idea even better, we were also thinking about something along those lines.

Here's a rough implementation sketch:

export class NodeHttpStack implements HttpStack {
  constructor(options = {}) {
    this._stallDetectionOptions = {
      enabled: options.stallDetection?.enabled ?? true,
      stallTimeout: options.stallDetection?.stallTimeout ?? 30000, // 30s with no progress = stalled
      checkInterval: options.stallDetection?.checkInterval ?? 5000, // Check every 5s
    };
    this._requestOptions = options.requestOptions || {};
  }
}

// In Request.send() method
send(body?: FileSliceTypes): Promise<HttpResponse> {
  return new Promise((resolve, reject) => {
    // Setup stall detection
    let lastBytesSent = 0;
    let lastProgressTime = Date.now();
    
    req.setProgressHandler((bytesSent) => {
      this._progressHandler(bytesSent); // Original handler
      
      if (bytesSent > lastBytesSent) {
        lastBytesSent = bytesSent;
        lastProgressTime = Date.now();
      }
    });
    
    // Check for stalls periodically
    const stallCheckInterval = setInterval(() => {
      const stallTime = Date.now() - lastProgressTime;
      if (stallTime > this._stallDetectionOptions.stallTimeout) {
        clearInterval(stallCheckInterval);
        req.abort();
        reject(new Error('Upload stalled: No progress for 30 seconds'));
      }
    }, this._stallDetectionOptions.checkInterval);
    
    // Clear interval on completion/error
    req.on('response', () => clearInterval(stallCheckInterval));
    req.on('error', () => clearInterval(stallCheckInterval));
    
    // Rest of implementation...
  });
}

I assume that error would propagate and trigger a retry?

Would you be open to a PR implementing something like this?

[Acconut]

I think socket timeouts only trigger when no data flows for the specified period - they don't limit total upload duration. A 10-hour upload works fine with a 30-second timeout as long as data continues to transfer.

Oh, I didn't realize this is a socket timeout not a request timeout. Having a default socket timeout makes sense! Feel free to open a PR to add this.

I assume that error would propagate and trigger a retry?

Yes, it should

Would you be open to a PR implementing something like this?

Yes, very much! Although I think a generic implementation in the Upload class (or some helper function) would be preferable, so we don't have to duplicate the logic in every implementation of HttpStack. Does that make sense?

[m7kvqbe1]

Ok great sounds good I will investigate, thanks!

[m7kvqbe1]

Draft PR here: #775

Let me know what you think about the direction of this @Acconut

Feel free to push stuff to that branch if you like.

❤

[m7kvqbe1]

Feel free to take another look at this now @Acconut ❤

#775