vercel / async-retry

Retrying made simple, easy and async
https://npmjs.com/async-retry
MIT License
1.85k stars 53 forks source link

throw Error inside onRetry #79

Open yokomotod opened 3 years ago

yokomotod commented 3 years ago

Throwing an error in onRetry seems like a weird behavior to me.

const retry = require("async-retry");

(async () => {
  try {
    await retry(
      (bail, attempt) => {
        throw new Error(`inside error at ${attempt}`);
      },
      {
        onRetry: (e, attempt) => {
          console.error(e.message);
          throw new Error(`onRetry error at ${attempt}`);
        },
      },
    );
  } catch (e) {
    console.error(e.message);
  }
})();

Result:

inside error at 1
onRetry error at 1
inside error at 2
/home/xxxxx/playground.js:12
          throw new Error(`onRetry error at ${attempt}`);
          ^

Error: onRetry error at 2
    at Object.onRetry (/home/xxxxx/playground.js:12:17)
    at onError (/home/xxxxx/node_modules/async-retry/lib/index.js:33:17)
    at RetryOperation.runAttempt [as _fn] (/home/xxxxx/node_modules/async-retry/lib/index.js:43:9)
    at Timeout._onTimeout (/home/xxxxx/node_modules/retry/lib/retry_operation.js:81:10)
    at listOnTimeout (node:internal/timers:557:17)
    at processTimers (node:internal/timers:500:7)

I expected that it caught once without retry like:

inside error at 1
onRetry error at 1
(finish)

otherwise, with retry like:

inside error at 1
inside error at 2
inside error at 3
inside error at 4
inside error at 5
inside error at 6
onRetry error at 6
(finish)

Is this a bug?

jonniesweb commented 3 weeks ago

Too bad this library isn't being maintained anymore. Ended up fixing this by wrapping the function to be retried with my own try/catch that calls what I had in onRetry, eg.

  async withRetry(fn) {
    let attempt = 0;
    const wrapper = async (...params) => {
      try {
        attempt++;
        return await fn(...params);
      } catch (e) {
        console.warn(`Retrying attempt ${attempt}`, e);

        // what I would have had in my onRetry
        if (e.code == 401 || e.code == 400) {
          await maybeThrowsError();
        }

        // always throw original error
        throw e;
      }
    }

    return await retry(wrapper, { retries: 1 });
  }