Skip to content
This repository has been archived by the owner on Aug 22, 2023. It is now read-only.

Suggest adding a ECONNABORTED retry #25

Open
garygchai opened this issue Aug 24, 2017 · 1 comment
Open

Suggest adding a ECONNABORTED retry #25

garygchai opened this issue Aug 24, 2017 · 1 comment

Comments

@garygchai
Copy link

garygchai commented Aug 24, 2017

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
Copy link

GiveDaData commented Jan 30, 2020

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants