keep upload alive for large files or slow connections

[DenysVuika]

Addresses issue #1374

Keeps upload alive for very large files or slow connections, resets deadline/timeout timers while data is still being sent over the wire.

I have tested the fix with network throttling using Chrome DevTools, also tested on a real project. However I cannot find a way to create a unit test that has slow upload with progress updates simulated.

[kornelski]

Thank you for the PR. However, I don't think that's appropriate.

I see what you want — monitoring whether connection is still alive and timing out only after data stops flowing. That would be useful. But that's not what current timers are supposed to do. The deadline's main purpose is to kill slow connections, even if they're still going.

If you can accept the upload taking half an hour, then you can't set the deadline (or you have to set it to half an hour).

The _responseTimeoutTimer is cleared in readyState change >= 2, which is fired when headers are received. Its not set again, so doesn't need to be cleared later.

So if you'd like to implement watchdog timer for connection stalling, then you need to add a third timer. Don't cancel the existing ones.

[DenysVuika]

I see what you mean, thanks. Will try to increase timeout settings at the client side.

[eromano]

@kornelski also if we add a third timer the deadline timeout will be still active .
Should I just disable the deadline timeout and other this third timeout?

[kornelski]

No, don't touch the other options. They are independent.

[eromano]

Sorry still don't clear to me. Basically we will have 3 timers:

1. Deadline timer
2. Response timer
3. Upload timer

Upload timer :The upload timer should trigger an error when between two progress chunk it last too much time. (is that right?)

The problem that I see that the deadline timer will still be triggered after 60 seconds (by default):

  // deadline, this timeout will always be triggered if we don't clear it
  if (this._timeout && !this._timer) {
    this._timer = setTimeout(function(){
      self._timeoutError('Timeout of ', self._timeout, 'ETIME');
    }, this._timeout);
    ...
    ...
    ...
    ...

even if we clear the upload timer :

// progress
  var handleProgress = function(direction, e) {
    clearTimeout(self._uploadTimeoutTimer);

    if (e.total > 0) {
      e.percent = e.loaded / e.total * 100;
    }
    e.direction = direction;
    self.emit('progress', e);
  };
  if (this.hasListeners('progress')) {

      try {
      xhr.onprogress = handleProgress.bind(null, 'download');
      if (xhr.upload) {
        xhr.upload.onprogress = handleProgress.bind(null, 'upload');
      }
    } catch(e) {
      // Accessing xhr.upload fails in IE from a web worker, so just pretend it doesn't exist.
      // Reported here:
      // https://connect.microsoft.com/IE/feedback/details/837245/xmlhttprequest-upload-throws-invalid-argument-when-used-from-web-worker-context
    }
  }

How can I avoid the deadline timeout to happen if I don't clear it?

[kornelski]

The current options are:

• deadline timer: kill the request if it doesn't fully finish entire upload and entire download before the time passes. Kills no matter what.

• response timer: kill the request if it doesn't fully finish entire upload and start some download before the time passes. Allows inifinite download time.

Both current options always limit upload time — on purpose. It's not a bug that these timers stop long uploads. That's their purpose. If you don't want uploads interrupted by a timeout, don't set these timeouts.

[eromano]

Thanks for the explanation of the current options .
My questions are related to the third timer that you suggest to implement and I was trying to figure out which was your proposal.

"So if you'd like to implement watchdog timer for connection stalling, then you need to add a third timer. Don't cancel the existing ones."

[kornelski]

From what I understand, you are setting the deadline timer, which sole purpose is to unconditionally abort the request, and then you don't like that the timer fires and unconditionally aborts the request.

There's nothing wrong with the deadline timer, it does exactly what it's supposed to do. It's just not a feature for you, so don't use it. Don't call .timeout({deadline:…}).

How can I avoid the deadline timeout to happen if I don't clear it?

By design, the deadline timer is supposed to be unconditional and intentionally impossible to avoid. If you change the implementation to clear the timer in some circumstances, then you just break that feature.

What I'm suggesting is that you add a third kind of timer, completely separate from the other two, and then:

• Do not set the deadline, at all. Never. Just forget it ever existed. The default behavior is no deadline, which allows infinitely long uploads.
• Use the third kind of timer instead.

In that situation you don't need to clear the deadline timer.

[eromano]

Ok I got it, thanks.
This timer will works only if the deadline timer is not set or it's value is minor of the deadline.
Do you would like to have this functionality in superagent or is better that we just overwrite it in our side?

[kornelski]

Yes, this functionality (timeout since last bit of data was sent or received) would be useful in superagent. Here's previous discussion of that feature: #1115

I've just remembered we've had reports that the upload events are buggy in some browsers: #1236 #1244

So this is going to be a serious problem for uploads, and will require special handling (e.g. find a way to detect when the browser fires events for buffering rather than actual sending, or support the timeout only in browsers tested and verified to work).

[eromano]

Ok thanks , I guess I will try to create a PR but I will sure need your help

[kornelski]

Sure! I'm happy to help.