tim-kos / node-retry Goto Github PK
View Code? Open in Web Editor NEWAbstraction for exponential and custom retry strategies for failed operations.
License: Other
Abstraction for exponential and custom retry strategies for failed operations.
License: Other
I am working on improved logging in pnpm. When a request fails, I want to print something like:
WARN GET <URL> error (500). Will retry in 16 seconds. 2 retries left.
I can calculate how many retries are left but I can't know how long the retry will wait. So it would be great if the retry function would return timeout
instead of true
here:
node-retry/lib/retry_operation.js
Line 92 in c402d77
Sometimes, you want to retry an operation indefinitely. It would be nice if "Infinity" (or some other special value) could be specified for "retries". Currently, that causes Node to blow up with out of memory.
Isn't the current attempt missing when executing this callback right here?
node-retry/lib/retry_operation.js
Line 110 in b8e26ea
I also found this line which confuses me. The readme says nothing about an option called forever
in the timeouts
function and I'm unsure if it's a miss in the readme or in the code.
Line 37 in b8e26ea
The timeout variable is used as the timeout in a setTimeout but this line right here sets timeout to an array of size 1 but setTimeout doesn't accept an array as a second argument.
node-retry/lib/retry_operation.js
Line 65 in b8e26ea
node-retry/lib/retry_operation.js
Line 86 in b8e26ea
Couldn't this cause unwanted behaviour?
Suppor using Promise instead of CallBack will be very convenient.
If you agree, I'd like to make a Pull Request for it.
I've run into a use case where I'd like to reuse a previously created retry.operation
. Upon some errors, I'd like to restart the whole operation from scratch, exactly as if a new operation would have been created. The reason why creating an actual new retry.operation()
is inconvenient is because the function being retried contains a reference to the operation and uses it to call op.retry
and, basically, retry itself. So while I could potentially refactor my module to avoid this, it would be much simpler to just do something along the lines of:
function onError(err, op) {
log('some error occurred', err);
// Reset operation internal state -> start anew
op.reset();
return op.retry(err);
}
let op = retry.operation();
op.attempt(function() {
getSomeResource().then((res) => {
res.on('error', (err) => onError(err, op));
// Do something with resource
// ...
}).catch((err) => {
// Retry operation normally
if (!op.retry(err)) return;
});
});
Any thoughts on this? Can this be done, currently?
if (this._options.unref) {
^
TypeError: Cannot read property 'unref' of undefined
retry_operation.js:63:24
sometimes unref is unset. This generally happens when we have many different instances of operation are obtained from retry.operation() calls and one of them is retried. This is in v0.10.1
basically something like:
var retry = require('retry');
function myRetry(simpleFunction) {
// declare options here
var operation = retry.operation(options);
operation.attempt(simpleFunction, timeOutOptions);
}
setInterval(myRetry(function(attempt) {
// ....
}, 1000);
I think you should fix the example in the readme.
function faultTolerantResolve(address, cb) {
var operation = retry.operation();
operation.attempt(function(currentAttempt) {
dns.resolve(address, function(err, addresses) {
if (operation.retry(err)) {
return;
}
cb(operation.mainError(), addresses);
});
});
}
The issue is here:
cb(operation.mainError(), addresses);
Once an error was passed, mainError will always return an error, however an error needs only to be passed to the callback if retries did not help.
cb(err ? operation.mainError() : null, addresses);
Best,
Oleg
It looks like each attempt calls the callback when the connection finally succeeds. The desired behavior is for the callback to be called once.
// Open the DDP connection
function connectDDPWithRetry(ddpConnection, cb) {
const operation = retry.operation();
operation.attempt((currentAttempt) => {
console.log(`executing. attempt #${currentAttempt}`); /* eslint no-console:0 */
ddpConnection.connect(err => {
if (err) {
operation.retry(err);
} else {
console.log('calling CB');
cb();
}
});
});
}
connectDDPWithRetry(ddp, (err) => {
console.log('inside callback');
if (err) throw err;
const { JOB_USER, JOB_PASS } = process.env;
DDPlogin.loginWithUsername(ddp, JOB_USER, JOB_PASS, err2 => {
if (err2) throw err2;
console.log('CONNECTED');
Job.processJobs('jobs', 'fooJob', (job, cb) => {
// This will only be called if a fooJob job is obtained
console.log(job.data); /* eslint no-console: 0 */
job.done();
cb();
});
});
});
executing. attempt #1
executing. attempt #2
executing. attempt #3
executing. attempt #4
executing. attempt #5
executing. attempt #6
calling CB
inside callback
calling CB
inside callback
calling CB
inside callback
calling CB
inside callback
calling CB
inside callback
calling CB
inside callback
CONNECTED
CONNECTED
CONNECTED
CONNECTED
Login failed: Error, too many requests. Please slow down. You must wait 9 seconds before trying again. [too-many-requests]
./fooWorker.js:43
if (err2) throw err2;
^
I have a module that depends on this. I just discovered that on forever
retries, there is no delay in between the retries. I don't know how to write an appropriate test for this project, but I'm fairly sure it is on this end.
When using a factor < 1, the longest timeout will come first:
> r = require('retry')
{ operation: [Function], timeouts: [Function], _createTimeout: [Function] }
> r.timeouts({factor: 0.5, retries: 3})
[ 1000, 500, 250 ]
To fix this, the resulting timeout array should be sorted in ascending order.
--fg
What am I doing wrong, if the number of attempts is retries+1?
I specified the retries
options parameter to 5 for example, but then it attempts 6 times to execute the operation.
You can found the details at #58 too.
When we give retries
Infinity value, it will produce this error
#
# Fatal error in , line 0
# Fatal JavaScript invalid size error 169220804
#
#
#
#FailureMessage Object: 0000007FAFAFD6C0
1: 00007FF6B5CC30AF v8::internal::CodeObjectRegistry::~CodeObjectRegistry+112511
2: 00007FF6B5BE023F v8::CFunctionInfo::HasOptions+7055
3: 00007FF6B68B6392 V8_Fatal+162
4: 00007FF6B6440153 v8::internal::FactoryBase<v8::internal::Factory>::NewFixedDoubleArray+259
5: 00007FF6B62E8B73 v8::internal::FeedbackNexus::ic_state+60339
6: 00007FF6B62FFBD0 v8::Message::GetIsolate+14688
7: 00007FF6B6174711 v8::internal::CompilationCache::IsEnabledScriptAndEval+26849
8: 00007FF6B6612541 v8::internal::SetupIsolateDelegate::SetupHeap+494417
9: 000001FE08B399D9
Work around:
await pRetry(run, {
retries: Infinity,
onFailedAttempt: () => {
console.log("Retrying..");
},
});
"p-retry": "^5.0.0"
We are using the async-retry library, which seems to make a new retryOperation object, call attempt
on it, and then call retry
on it if needed. If it passes in an error to retry
, it seems to rely on node-retry to add the error to the list of errors that's being kept.
However, when there is a timeout, the error passed in is dropped. In practice, this means that when something throws one error and then times out, the one error is never added to retryOperation._errors
, so our code (which eventually receives operation.mainError()
) gets the error message "RetryOperation timeout occurred".
It would be nice if the error passed in to retry
always made its way into the _errors
array, so this wouldn't happen.
We can copy line 53 and paste just above line 49. I would have made a PR but I couldn't figure out how to do it on this repo.
The stop method present in the module does not clear the current running timer. Hence it waits for the current running timer to complete, then executes the callback, in which the retry method causes the exit from the retry loop.
For ex, if the timeouts are set to 1, 2, 4, 8 and 16, and currently the retry loop is waiting for 16 seconds, then even if the stop method is called in between, the timer still continues for the 16 seconds, and executes the callback before exiting as retry method will cause the exit.
However, suggested behaviour should be that retry loop exits instantly. This behaviour is needed when the program needs to exit and the retry is stuck waiting in the retry loop.
This is not an bug report.
I would like some assistance on the settings/values i need to use to make my operation retry 5 times with the 5th retry being somewhere around 1 hour after the first attempt.
This is lib is awesome by the way!!!
Thank you so much for this project. I've used it many times and it is my go-to backoff library.
I was curious, is there anything preventing a 1.0 release at this point? The library seems pretty stable and does its job really well. Bumping it into "stable" semver territory seems like it would be a good thing.
Hi @tim-kos,
Thanks indeed for a superb work to you and all contributors of the project.
Can you release a new version and update a version in npm?
There are 14 new valuable commits since 0.10.1.
Cheers
The message here is that RetryOperation.try()
has been deprecated, but there's no indication of what should be used instead.
The message should include something like "use attempt() instead".
https://github.com/tim-kos/node-retry/blob/master/lib/retry_operation.js#L78
I am often writing the same stuff over and over, this function would reduce a lot, but it is only possible to implement it based on 2 conventions:
- callback is always the last argument
- err is always the first argument
So basically I want to say: wrap all functions of object obj
into retry logic, use opts
for retry options, optionally props
is a list of props I need to wrap, otherwise wrap all functions.
retry.wrap(opts, obj, props)
Would you accept a pull request implementing this?
This is doc about typescript 4.4 breaking change:
https://devblogs.microsoft.com/typescript/announcing-typescript-4-4/#more-compliant-indirect-calls-for-imported-functions
for this code
import { timeouts} from 'retry'
console.log(timeouts({
retries: 5,
}))
typescript will compile it to
"use strict";
exports.__esModule = true;
var retry_1 = require("retry");
console.log((0, retry_1.timeouts)({
retries: 5
}));
note it is (0, retry_1.timeouts)(...)
and it will have different "this" context
and it will throw error:
**\node_modules\retry\lib\retry.js:34
timeouts.push(this.createTimeout(i, opts));
^
TypeError: this.createTimeout is not a function
at exports.timeouts (**\node_modules\retry\lib\retry.js:34:24)
at Object.<anonymous> (**\test-fail.js:7:34)
at Module._compile (node:internal/modules/cjs/loader:1101:14)
at Object.Module._extensions..js (node:internal/modules/cjs/loader:1153:10)
at Module.load (node:internal/modules/cjs/loader:981:32)
at Function.Module._load (node:internal/modules/cjs/loader:822:12)
at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12)
at node:internal/main/run_main_module:17:47
workaround is use import * as retry from 'retry' instead
Could you please add to documentation about the order of operation.errors()
. Is it ordered and if so, are the newest errors at index 0 in the array or at the last element in the array?
Use case:
I have a long running process that use retry
in various places. This process intercepts SIGINT
and SIGTERM
to gracefully do some cleaning before exiting, closing sockets and stuff like that keep the event loop from emptying. Though, the process won't exit if there is a retry running.. and I have to wait for the whole timer duration before exiting.
To solve this we could add an unref
(true/false) option so that the setTimeout's don't prevent the process from exiting. Would you be willing to add this functionality?
Hi,
It appears to me that the following splice call
// retry forever, only keep last error
this._errors.splice(this._errors.length - 1, this._errors.length);
is not doing what it says it does. It actually only removes the last element of _errors
.
Since it has been there for 3 years and nobody complaining about it, I prefer checking with you before if:
Thanks for the great work.
Hi,
I found that the .wrap() function acts oddly when being called as follows:
var obj = {
fn1: function () { ... },
fn2: function () { ... },
};
retry.wrap(obj, { nRetries: 3, randomize: true }, ['fn1', 'fn2']);
Both obj.fn1() and obj.fn2() will now call the same original function due to this line https://github.com/tim-kos/node-retry/blob/master/lib/retry.js#L76. The variable original
will be hoisted to the top of the function declaration of wrap
.
I'm going to solve my specific use-case by calling wrap twice, once per function instead. But if you'd like to, I could try to get back with a PR for solving this.
Is it possible to deal with Promise?
If yes, could you provide some example cases?
If not, could you support to refactoring the operation.attempt
to return a Promise?
function faultTolerantResolve(address, cb) { // <== What if the faultTolerantResolve returns a Promise?
var operation = retry.operation();
operation.attempt(function(currentAttempt) {
dns.resolve(address, function(err, addresses) { // <== What if the dns.resolve returns a Promise?
if (operation.retry(err)) {
return;
}
cb(err ? operation.mainError() : null, addresses);
});
});
}
Does this look like a reasonably safe use of retry?. I am trying to implement a 2-3 attempt retry when something goes awry in my redis subscriber. If it fails after max attempts I will just save to a dead letter queue table in my database (or raise some sort of alert if the the db save fails :))
pubsub.subscribe(`TEST`, async(message) => {
console.log("processing...", message )
var operation = retry.operation({
retries: 2,
factor: 2,
minTimeout: 1 * 1000,
maxTimeout: 60 * 1000,
randomize: false,
});
return operation.attempt(async(currentAttempt) => {
console.log("currentAttempt is",currentAttempt)
let err
try{
//my logic that might fail and throw an error goes here
}catch(whoops){
err = whoops
}
if (operation.retry(err)) {
console.log('returning for a retry...')
return;
}
if(operation.mainError()){
console.log("There was a persistent error, we should save message to a DLQ here:" ,operation.mainError().message)
}
})
})
I've tried to make node-retry
work with Promises, so that I can call it for example like await retryCommand('curl http://example.com')
. But it's not working as expected, because I get numerous console.log
output with the same attempt
number before the attempt
number gets incremented. For example:
doing 2: curl http://example.com
doing 2: curl http://example.com
doing 2: curl http://example.com
...
doing 3: curl http://example.com
doing 3: curl http://example.com
doing 3: curl http://example.com
Why would the operation.attempt
callback get executed again with the same attempt
number? I'm passing an async
callback to operation.attempt
but I don't see why that would be the problem.
function retryCommand(command) {
return new Promise((resolve, reject) => {
const operation = retry.operation({})
operation.attempt(async (attempt) => {
if (attempt > 1) console.log(`doing ${attempt}: ${command}`)
try {
const execResult = await execPromise(command)
resolve(execResult)
} catch (e) {
if (errorIsRetriable(e)) {
if (operation.retry(e)) return
reject(operation.mainError())
} else {
reject(e)
}
}
})
})
}
Basically the code checks if the error should be retried (with a custom errorIsRetriable(e)
function), and if not then the Promise rejects right away.
Hi,
I see that, although being a really useful module, keywords section is not present in the package.json. Was this intentional? Having keywords like retry can be really helpful for others who do a quick search on npm for retry related modules. If needed, I can help.
are these operations synchronous or async?
Is there a reason why wrap
only supports an object with functions as properties, or would it also be possible to take a single function as argument and wrap it?
What is the right way of using node-retry for operations that use EventEmitter for 'error' events, rather than callback?
Repro code:
const retry = require('retry');
const testCase = (configAmmend) => {
return new Promise((resolve) => {
const op = retry.operation({
retries: 1,
minTimeout: 800,
factor: 1,
...configAmmend
});
const startTime = Date.now();
op.attempt((attempt) => {
console.log(`${attempt}: ${Date.now() - startTime}`);
op.retry(new Error('fail on purpose'));
if (attempt === 2) {
resolve();
}
});
});
};
const test = async () => {
console.log('test 1');
await testCase({});
console.log('test 2');
await testCase({ maxTimeout: 1000 });
console.log('test 3');
await testCase({ maxTimeout: undefined });
}
test();
Expected behavior:
test 1
1: 0
2: 809
test 2
1: 0
2: 810
test 3
1: 0
2: 809
Actual behavior:
test 1
1: 0
2: 809
test 2
1: 0
2: 810
test 3
1: 0
2: 15
How could i use this to retry http requests.
My http requests are promise based and i like to use async and await.
public await getResult(sessionId: any, requestId: any) {
const url = this.config.backendUrl + "/check/report";
const options = {
method: "GET",
uri: url,
headers: {
"X-IDCHECK-SESSION-ID": sessionId,
},
body: {},
json: true,
simple: false,
resolveWithFullResponse: true,
};
return result = await request(options);
}
I use the request-promise-native library as my http client. https://github.com/request/request-promise-native
Can we add badges for travis CI and code coverage reports?
I'd like to use the _createTimeout
function directly to calculate a delay for a job which is to be re-inserted into a databased-backed task queue.
I know I can just use the function, but the _
indicates to me I'm not really supposed to... Would you be happy for this to be made public, e.g. strip the _
?
Hi, I wrote an higher-level function util withRetry
which makes the usage of the library very easy and also supports promises.
this is what I wrote:
const retry = require('retry');
function withRetry(callback) {
return (...args) => {
const operation = retry.operation({ retries: 5 });
return new Promise((resolve, reject) => {
operation.attempt(async function (currentAttempt) {
try {
const result = await callback(...args);
resolve(result);
} catch (err) {
console.log(`attempt #${currentAttempt} failed. error: ${err.message}`);
if (operation.retry(err)) {
return;
}
reject(err);
}
});
});
};
}
module.exports = withRetry;
now any function could be enhanced with retry logic using a single line. e.g:
async functuin myFunc(a, b) {
// .. bla bla async logic ..
return a + b
}
module.exports = withRetry(myFunc);
if it feels useful, I can make the code more generic, and contribute as PR
Quick fix to ensure attempts got increased at the time when retry function got invoked.
Otherwise, the _attempts is not accurate as the retry actually got executed after timeout.
Too lazy for a pull request. Just paste the patch here.
diff --git a/lib/retry_operation.js b/lib/retry_operation.js
index f24d2d5..07add06 100644
--- a/lib/retry_operation.js
+++ b/lib/retry_operation.js
@@ -25,10 +25,9 @@ RetryOperation.prototype.retry = function(err) {
return false;
}
- this._attempts++;
-
var self = this;
setTimeout(function() {
+ self._attempts++;
self._fn(self._attempts);
if (self._operationTimeoutCb) {
@@ -106,4 +105,4 @@ RetryOperation.prototype.mainError = function() {
}
return mainError;
-};
\ No newline at end of file
+};
Hi. Thanks for this module, it's come in handy.
I just started trying out operation.reset()
and I find it's usage a little weird. It requires that you wrap the entire operation.attempt()
in another closure function, and then recursively call that closure function after operation.reset()
in order to start the operation from scratch again, ie:
var operation = retry.operation();
var fn = function() {
operation.attempt(function(currentAttempt) {
...
if (needToStartAgain()){
operation.reset();
fn();
return;
}
if (operation.retry(error)) return;
...
});
}
This just seems a little clunky because you could easily just declare a whole new operation in fn() instead of reusing the old one, at which point reset
isn't even necessary.
My idea is that calling reset
and then retry
(with or without an error) would begin the operation all over again from scratch, ie:
operation.attempt(function(currentAttempt) {
...
if (needToStartAgain()){
operation.reset();
operation.retry();
return;
}
if (operation.retry(error)) return;
...
});
Basically it's a way to force retrying from scratch.
Anyways, just an idea. I'll submit a PR, feel free to reject.
Oh one other thing. As it is currently, more than one reset
call probably won't work. The reset
function assigns _originalTimeouts
to _timeouts
as an object reference, which will modify _originalTimeouts
in any subsequent retry
calls, might be better to clone it instead:
lib/retry_operation.js
RetryOperation.prototype.reset = function() {
this._attempts = 1;
this._timeouts = this._originalTimeouts; // should clone this instead, via JSON.parse(JSON.stringify(this._originalTimeouts));
}
According to the formula, a minTimeout
of 1000ms
will always execute the first attempt after 1000ms
(assuming randomize: false
).
Is there a way to make retry
attempt immediately (perhaps on process.nextTick
)?
Note that putting the minTimeout
value to 1ms
will do it but then we don't get any benefit of the backoff.
attempt() and retry() both set the _operationTimeout timeout after invoking _fn(). Therefore, the time it takes _fn() to perform any synchronous processing (which could be significant) is ignored.
The timeout should be set immediately before invoking _fn().
Naïve reading of the docs would suggest retries: 1
means 'do it once, and then retry it once if it failed'.
Is that what it means? Or is it 'do it once and then stop'?
this comes in handy when working with sockets and streams in general, you want to reattempt and recover as much as possible but fail fast in some cases.
The first example provided in the documentation contains too much information for me wrap my mind around what's going on, and how I properly write a retry.
My questions:
given:
retry = require 'retry'
operation = retry.operation
operation.attempt (curAttempt) ->
# do some work
# what goes here to indicate that I failed and need to retry?
Hi the latest release (0.10.0) does not appear in the tags: https://github.com/tim-kos/node-retry/tags
Please tag this release and the future ones so that automatic upstream release monitoring works.
Hey, hope you're doing well!
I have a question about this chapter in the docs. You declare factor
as an exponential factor but in the formula below:
Math.min(random * minTimeout * Math.pow(factor, attempt), maxTimeout)
it's used in terms of the power's base, not the factor itself, isn't it? From my perspective, it's not the same.
Can you elaborate this subtle moment, please?
Sometimes it is necessary to stop retrying. For instance, when the retry causes the environment to enter some severe state (remote server returns 500), we really don't want to retry. Would be nice to have a function to accomplish this.
retry_operation.js -> Line 63
if (this._options.unref) {
self._timeout.unref();
}
this
is used out of scope. Change to self
Hi I have the following code
retryer.js
const retry = require('retry');
const retryer = async (call, options) =>
new Promise((resolve, reject) => {
const { retries: maxRetries } = options
const operation = retry.operation(options)
operation.attempt(async currentAttempt => {
try {
const response = await call()
resolve(response)
} catch (err) {
if (currentAttempt === maxRetries) {
console.error('max amount of calls reached, aborting')
return reject(err)
}
console.error('failed call number: ', currentAttempt);
operation.retry(err)
}
})
})
module.exports = retryer;
then I run this code which send a 404 on purpose
main.js
const axios = require('axios');
const retryer = require('./retryer')
const url = 'https://pokeapi.co/api/v2'
const makeCall = (resource) => {
return axios({
method: 'get',
url: url + resource
})
}
const main = async () => {
const retryerOptions = {
retries: 5,
minTimeout: 1 * 1000,
maxTimeout: 5 * 1000
}
console.log(`making call to ${url} ...`);
const response = await retryer(() => makeCall('/pokemons'), retryerOptions)
console.log('my data', response.data);
console.log('finishing..');
}
main()
the retryer behaves as expected
but in my tests when I pass a mocked function that always returns a rejected promise, the retryer only do one call and then exits, why is this?
retryer.test.js
const retry = require('../retryer.js')
const APICallRetryerOptions = {
retries: 5,
minTimeout: 1000,
maxTimeout: 5000
}
describe('node-retry test issue', () => {
it('should throw and error when the max amount of attempts is reached', async () => {
const mockAxiosError = {
isAxiosError: true,
config: {
url: 'bad-site.com',
method: 'get'
},
response: {
status: 500,
statusText: 'some weird error',
data: {
Error: 'Mock error!'
}
}
}
const mockApiCall = jest
.fn()
.mockRejectedValueOnce(mockAxiosError)
.mockRejectedValueOnce(mockAxiosError)
.mockRejectedValueOnce(mockAxiosError)
.mockRejectedValueOnce(mockAxiosError)
.mockRejectedValueOnce(mockAxiosError)
retry(() => mockApiCall(), APICallRetryerOptions)
expect(mockApiCall).toHaveBeenCalledTimes(5)
})
})
the above code, as I said before, calls mockAPICall once and then exits, I've tried to debug the test to see why that happens but I could figure it out, also I've done some research but I couldn't find a similar issue
I also tried to change mockAPICall
declaration to const mockApiCall = jest.fn(() => Promise.reject(mockAxiosError))
but still getting just one call
can you point out what I'm doing wrong?
Thanks!
Use Case:
$ retry --num 3 npm install
See: travis_try at https://docs.travis-ci.com/user/common-build-problems/#Timeouts-installing-dependencies
If you agree, I'd like to make a Pull Request for it.
A declarative, efficient, and flexible JavaScript library for building user interfaces.
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google ❤️ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.