segmentio / superagent-retry

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

Custom Retry Conditions #6

Open skeggse opened 10 years ago

skeggse commented 10 years ago

Retry is great for some use-cases, but it seems like it could be extended to support custom retry scenarios.

A good example might be OAuth: if I receive a 401 Unauthorized response, I want to update my access token. Naturally, that operation is asynchronous, so it'd need a callback.

var token;

getToken();

function getToken(callback) {
  oauth.getOAuthAccessToken(function(err, accessToken) {
    if (err)
      return callback && callback(err);
    token = accessToken
    callback && callback(null);
  });
}

superagent
  .get('https://segment.io/dosomethingsecret')
  // superagent-oauth
  .sign(oauth, token)
  // retry once
  .retry(1)
  // this API isn't entirely feasible, it's just an example
  // synchronously check whether to retry
  .when(function(err, res) {
    return res && res.unauthorized;
  })
  // only retry after this task finishes
  .after(1, function(next) {
    getToken(next);
  })
  .end(onresponse);

function onresponse(err, res) {
  // ...
}
leviwheatcroft commented 8 years ago

I'm 2.5 years late to this party but... it's looks like you can add custom conditions something like this:

const superagent = require('superagent')
const superagentRetry = require('superagent-retry')
superagentRetry(superagent)
superagentRetry.retries.unauthorized = (err, res) => res && res.unauthorized

retries property is exported here

skeggse commented 8 years ago

Thanks for the note. That would certainly take care of the .when part of the example, but it doesn't solve the rest of the problem. Note that some details of my example are not feasible (the bound token wouldn't get reapplied to the request), so this specific use-case might be outside the scope of superagent-retry. That said, forcing subsequent retries to wait with setTimeout or wait until an async operation could be in-scope, and we might consider adding that functionality.

Also worth noting: the retries field is not documented, and as such I consider the custom retry condition undocumented behavior. Maybe if it had @api public in the jsdocs I'd use it, though ideally it'd be documented in the readme.

FraBle commented 6 years ago

@skeggse Did you find a solution for the OAuth token refresh scenario?

yocontra commented 6 years ago

It would be great if this library was able to be configured in a way to respect 429 (Rate Limit) and Retry-After headers, just to give an example case.