tim-kos/node-retry

'operation.attempt' callback gets executed more than once with same 'attempt' number

rightaway opened this issue · 7 comments

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.

Not sure why this happens, however I've opted to use the following approach until we can figure this issue out:

const maxAttempts = 3;
let attempt = 1;
while (attempt <= maxAttempts) {
  try {
    console.log(`doing ${attempt}: ${command}`);
    const execResult = await execPromise(command);
    // Do something with execResult
    cosole.log(execResult)
    break;
  } catch (e) {
    this.log.error(e);
    // Sleep using linear backoff strategy (5s after first attempt, 10s after second attempt)
    if (attempt < maxAttempt) {
      await new Promise(resolve => setTimeout(resolve, 5000 * attempt));
    }
    attempt += 1;
  }
}

So you are getting the same issue as me? Do you also get it with node-promise-retry?
See my issue in IndigoUnited/node-promise-retry#14, I'm getting it there too.

@rightaway Yep same issue. I don't use node-promise-retry but it's just a wrapper around node-retry so it makes sense that you encountered this issue there as well.

Hey ladies and gentlemen,

I will look into this soon. I am just really busy with Transloadit right now.

Sorry for the delay. :/

FWIW, the following test program seems to work fine for me.

const retry = require("retry");

const sleep = timeout =>
    new Promise(resolve => setTimeout(resolve, timeout));

function log(msg) {
    console.log(`${new Date().toLocaleString()}: ${msg}`);
}

async function asyncThrow() {
    await sleep(1000);
    throw 'whoops';
}

const operation = retry.operation({
    retries: 3,
    factor: 2,
    maxTimeout: 10000,
    randomize: true
});
operation.attempt(async (attemptNumber) => {
    try {
        log(`attemptNumber = ${attemptNumber} - calling asyncThrow`);
        await asyncThrow();
    }
    catch(err) {
        if (operation.retry(err)) {
            log('retrying.');
        } else {
            log('giving up.');
        }
    }
});

Here's the output I get:

2017-11-20 14:34:00: attemptNumber = 1 - calling asyncThrow
2017-11-20 14:34:01: retrying.
2017-11-20 14:34:02: attemptNumber = 2 - calling asyncThrow
2017-11-20 14:34:03: retrying.
2017-11-20 14:34:07: attemptNumber = 3 - calling asyncThrow
2017-11-20 14:34:08: retrying.
2017-11-20 14:34:13: attemptNumber = 4 - calling asyncThrow
2017-11-20 14:34:14: giving up.

Hm okay, thanks for doing this.

@rightaway Do you mind posting your complete code here?

Closing this then for now, because the error is unlikely in node-retry, but rather in the userland code. @avranju 's example works as expected.