segmentio / superagent-retry

Retry superagent requests for common hangups
85 stars 35 forks source link

Suggest adding a ECONNABORTED retry #25

Open garygchai opened 7 years ago

garygchai commented 7 years ago

The superagent lib will abort the request before retry, and the error code will be ECONNABORTED. So I suggest that add a retry at the retries.js. This error code was added at the superagent source code. https://github.com/visionmedia/superagent/blob/master/lib/should-retry.js

/**
 * Request be aborted
 */
function econnabort (err, res) {
  return err && err.code === 'ECONNABORTED';
}

In addition, if ECONNABORTED is added, one issue should be attention.

Some code like following:

request.get('xxxx')
       .timeout(5000)
       .retry(2)
       .end((err, res) => {
            // TODO
       })

when first time the request is aborted, the this._aborted will be true, and next time retry request, the request will be return. Show the code.

// https://github.com/visionmedia/superagent/blob/master/lib/request-base.js
RequestBase.prototype._timeoutError = function(reason, timeout, errno){
  if (this._aborted) { // The second time here is true.
    return;
  }
  var err = new Error(reason + timeout + 'ms exceeded');
  err.timeout = timeout;
  err.code = 'ECONNABORTED';
  err.errno = errno;
  this.timedout = true;
  this.abort();
  this.callback(err);
};

So we can fix it at reset function.

// https://github.com/segmentio/superagent-retry/blob/master/lib/index.js
/**
 * HACK: Resets the internal state of a request.
 */

function reset (request, timeout) {
  var headers = request.req._headers;
  var path = request.req.path;

  request.req.abort();
  request.called = false;
  request._aborted = false; // code is here
  request.timeout(timeout);
  delete request.req;
  delete request._timer;
  ...
}
GiveDaData commented 4 years ago

For anyone wanting to solve this, here is my solution Using superagent own retry

.retry(3, (err, res) => {
    if (err && err.message.includes("ECONNABORTED")) {
       //  test for this error catch
        return true;
    }
    return false;
})

You could check for err to catch ECONNABORTED return true will retry request (you can even retry status 200) return false will not retry