demmer/bluebird-retry

utility for retrying a bluebird promise until it succeeds

bluebird-retry's Issues

Using with request-promise

I have this very simple test:
server which always returns 503 (and log the request). Therefore request-promise will reject.
client which should continuously request the server.

I expect the client to hit the server 5 times and then reject. In reality the client hits the serve 1 time, then reject.

Server:

var http = require('http');
var server = http.createServer(function (req, res) {
    console.log(new Date());
    res.statusCode = 503;
    res.end();
});
server.listen(8000);

Client:

var rp = require('request-promise');
var retry = require('bluebird-retry');
retry(rp('http://localhost:8000').promise(), { max_tries: 5 })
    .then(function (res) {
        console.log('success');
    })
    .catch(function (err) {
        console.log('fail');
    });

Am I doing something wrong?

Add way to run code between each retry attempt

For various use cases, you may want to run code after a failed attempt and before the next try. For example:

• Want to log errors/warnings whenever a retry is needed or keep a count of how many retries something took on average
• Want to do some cleanup after a failed attempt, e.g. clearing caches or closing a socket

You could argue that it could be better to do cleanup in a catch+rethrow at the end of the function being retried, but that's a bit more verbose (compared to encapsulating that boilerplate in the retry API itself). And for the first use case, where you want to log a warning on the second attempt (first retry) but not the first attempt, you have to resort to hacks like keeping a counter value somewhere.

Suggestion: add an option for a callback like beforeRetry which is run immediately before every retry attempt (but not before the very first attempt).

Allow chaining

It would be more intuitive if done(...) callback could be chained like thenables:

retry(myfunc).done(function(result) {
    return someOtherPromiseReturning();
})
.then(function() {
    // next step
});

standard Javascript-Errors lead to timeouts

If you produce a RangeError, ReferenceError, SyntaxError, TypeError or URIError in the retry-func without the "predicate"-option, it will timeout, because of the default "catch-all-implementation".
Perhaps you should document this feature and encourage the people to use predicate-filters.
Otherwise it will be confusing.

Compatibility issues in older browsers and reserved keywords

I am using bluebird-retry in a old browser that doesn't seem to support the Promise.try in your code, it simply chokes on it because "try" is a reserved keyword.
I do notice bluebird has a fallback option of Promise.attempt, and I request that you support this :)

I'm using Opera 11.0 (http://www.opera.com/download/guide/?custom=yes) so you can see how the browser behaves using bluebird-retry.
I can't upgrade the browser because of various reasons.

I've not checked if you're conflicting with other reserved keywords.

Retry on condition

Hi and thanks for your work

That would be great if a custom filter could be implemeted to perform the retry or not

A gentle way to replace something like this :

db.query("SELECT *...").then(() => {
	console.log("OK");
}).catch(error => {
	if (error == "ECONNREFUSED")
		retry(() => db.query("SELECT *..."), {timeout: 5000}).then(() => {
			console.log("OK");
		});
});

By something like this :

retry(() => db.query("SELECT *..."), {filter: (error) => { return (error == "ECONNREFUSED"); }, timeout: 5000})
	.then(() => {
		console.log("OK");
	})

Change args on Retry?

We are calling a service that requires a slightly different Authorization token on each retry request ( HMAC token ). I'd like to use the args option to call a function to regenerate the token on each retry ( the function attachAuthHeader(requestConfig, log) below ), but when I look at all the retry requests going down the wire, bb retry still seems to be holding onto the original token, so doesn't look like my function is getting invoked on each retry. Is there some other way to do this?

Code looks like :

return bbRetry(request-promise, {
interval: 1000,
backoff: 2,
max_tries: 2,
predicate: err => {
const predResult = defaultPredicate(err, log) && (_.isFunction(condition) ? condition(err) : true);
return predResult;
},
throw_original: true,
context,
args: [attachAuthHeader(requestConfig, log)]
});
}

Feature Request: Get rid off of bluebird dependency

It would be great if you can get rid of the dependency on bluebird to avoid the bluebird package is getting loaded multiple times. This is very easy to do - see promise-retryer, for example

bluebird as peer dependency

Can bluebird be moved in a peerDependency?

On the frontend otherwise it can not pay nice with other promises in the code that use a different version.

Would you consider using fn.apply() instead of fn()?

Using something in the lines of fn.apply(stateObj, options.args) would make it possible possible to call fn using arguments in ways cleaner than retry(() => { return fn(a, b, c); }); and also run fn in a specific scope, which would allow it to keep track of what has happened in previous runs. Would this make any sense?

Cancel function feature undocumented

It also can create some unexpected consequences if somebody passes the function that returns the promise but also calls the callback if it's supplied.

Not sure how the cancel feature can be used at all, I guess to prevent retrying on certain types of errors. The better approach may have been to pass an optional cancelWhen (or retryWhen) function via options that would be checking the promise rejection and returning true (or false) if the retrying should be cancelled and the opposite value otherwise.

In any case, documenting this feature could be helpful... I can do it if you like, just let me know which approach you prefer.

Feature request: Retry forever

Would it be possible to support a "retry forever" mode, e.g. by setting "max_retries: -1".

Looking at the code this seems to be easy to implement, and it should not pile up the call stack due to the asynchronous execution of the retries.

What do you think?

If you have no time to work on this, I am happy to make a proposal and provide this to via pull request

`.timeout`

Hey, I'm just wondering why you implemented timing logic given there's .timeout

webpack compliance

While trying to use this otherwise great module with webpack, I get a Cannot find module 'bluebird' error.
Looks as if the browserified version basically loads well in webpack, but it avoids webpack's static require substitution. :(

Promise all is not working

Update project to use webpack

Hey @demmer, do you mind if I update this project to use webpack? I'll produce the build with the UMD output so this can be used on both the client and server side. Have you looked into this as of yet?

Providing interval: 0 uses the default (1000)

It's certainly an edge case, but I don't necessarily want to wait for retries to occur. Providing an interval of 0 is treated as falsey and thus is ignored.

https://github.com/jut-io/bluebird-retry/blob/master/lib/bluebird-retry.js#L17

bluebird as s peer dependency

Can bluebird be move in a peerDependency?

Feature request: retry returns Promise

Not sure if this has been implemented yet but it seems I can't get it working. I want to get the value returned by the retry so that it can be used by next chained promise. For example,

const retry = require('bluebird-retry');
... ...
const result =  retry(func, { interval: 2000 }).done((res) => Promise.resolve(res));

I'd expect the variable result to be res promise, but it' undefined instead.
Anyway I can get the promise back after retry call?

Error "Fatal TypeError: func is not a function"

I'm starting with a function just like in the readme example. But i get the error:
"Fatal TypeError: func is not a function"

One difference, I'd like to use a function which requires parameters.

The `basic usage` example dose not output the correct result

I think it should be:

var Promise = require('bluebird');
var retry = require('bluebird-retry');

function promiseSuccess(args) {
  return Promise.resolve(args);
};

var count = 0;
function myfunc() {
  console.log('myfunc called ' + (++count) + ' times');
  if (count < 3) {
    throw new Error('i fail the first two times');
  } else {
    return promiseSuccess('i succeed the third time');
  }
}

retry(myfunc)
  .done(function(result) { console.log(result); } );

Does it working with Bluebird 3?

Does it working with Bluebird 3?

Exceeding "max_tries" throws "Fatal undefined"

I was using Request to scrap some pages and started getting "Fatal undefined" errors. After A WHILE I figured what's happening and was able to reproduce the problem by minimally altering the code in your example:

var Promise = require('bluebird');
var retry = require('bluebird-retry');

var count = 0;
function myfunc() {
    console.log('myfunc called ' + (++count) + ' times');
    if (count < 10) {
        return Promise.reject(10);
    } else {
        return Promise.resolve('succeed the third time');
    }
}

retry(myfunc).done(function(result) {
    console.log(result);
});

view diff

console output:

C:\my-project-folder>node script.js
myfunc called 1 times
myfunc called 2 times
myfunc called 3 times
myfunc called 4 times
myfunc called 5 times
Fatal undefined

C:\my-project-folder>

You should consider displaying error message which is easier to understand like "Fatal: You have exceeded maximum number of retries which is 5. You can alter 'max_tries' option if you want to retry more times".

external interface is missing

It doesn't seem right that a retry loop can only be interrupted (or interacted with) form inside the callback.

There are many scenarios when it needs to happen externally.

Function retry should return an object/interface for stopping, and even monitoring the current status:

var r = retry(cb);

if(/*something happenned*/ && r.attempts > 3) {
   r.stop();
   // or even with a reason:
   // r.stop(new Error('Had to stop externally'));
}

also, how about pausing the retries?

r.pause(); // pause unconditionally
// or
r.pause(100); // pause for 100 millisecond
// and resume:
r.resume(); // to resume the retries, if currently paused

plus access to the current status:

r.getStatus() // = running | finished | paused

README typo in an example

I was trying some examples on the README, and noticed this line throw new Error('bail');} needs to have the } removed at the end in the logFail example.

function logFail() {
    console.log(new Date().toISOString());
    throw new Error('bail');}
}

Thanks for your work on this utility, it looks pretty helpful.

Filter errors

Would it be possible to, analog to bluebird .catch, make it possible to only retry on certain instances of errors. For instance with request-promise I'm only interested in retrying on RequestError not on StatusCodeError.

Maybe this can be done by accepting a constructor as an option or a predicate-based filter?

Seems doesn't work with bluebird Promise.join

static registerToConsul(consulAgent, serviceTag, pid, port, checks) {
    const _registerToConsul = () => {
      return Promise.join(this.getLocalIp(), this.generateServiceId(serviceTag, pid, port), (ipInfo, serviceId) => {
        return consulAgent.agent.service.register({
          name: serviceId,
          tags: [serviceTag, require('os').hostname()],
          address: ipInfo[0],
          port: port,
          checks: checks
        }).then((res) => {
          throw new Error(123);
          Promise.reject(123);
        });
      }).catch((err) => {
        console.log(111);
        Promise.reject(err);
      });
    };

    setTimeout(() => {
      retry(_registerToConsul, {
        interval: 100,
        max_tries: 65535
      });
    }, Math.random() * 1000);
  }

Tried either throw error or Promise.reject in then or catch, will not retry, but if don't use Promise.join, then will work, am I misunderstanding something?