operation.reset() is somewhat unintuitive
timbowhite opened this issue · 1 comments
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));
}
PR #66