search

tim-kos / node-retry

Abstraction for exponential and custom retry strategies for failed operations.
Other
1.22k stars 80 forks source link

.wrap() calls last specified function for all calls #43

Closed atamon closed 6 years ago

atamon commented 7 years ago

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.

tim-kos commented 7 years ago

Yes, I'd be happy to accept a PR for this.

Nice find!

tim-kos commented 6 years ago

Hey @atamon

Would you still like to send a PR for this?

atamon commented 6 years ago

Hi @tim-kos! Of course, it slipped my mind back a year ago. Please see #52 for what I had in mind back then.

tim-kos commented 6 years ago

Fixed in https://github.com/tim-kos/node-retry/pull/52#pullrequestreview-107541811 Thank you!