sindresorhus / p-retry

Retry a promise-returning or async function
MIT License
789 stars 62 forks source link

Loss of stack trace with `pRetry` #78

Open omril1 opened 2 weeks ago

omril1 commented 2 weeks ago

When using pRetry with retry count >= 1, the stack trace of the original calling function is not preserved, which complicates debugging, especially if the code wasn't written with this behavior in mind.

Example:

// pRetryStack.mjs
import pRetry from 'p-retry';

async function foo1() {
    return await foo2();
}

async function foo2() {
    return await pRetry(
        async () => { throw new Error('foo2 failed'); },
        { retries: 1 },
    );
}
// This doesn't lose the stack trace
// async function foo2() {
//     throw new Error('foo2 failed');
// }

async function main() {
    try {
        await foo1();
    } catch (err) {
        console.error(err);
    }
}

main();

Running node pRetryStack.mjs

Would print:

Error: foo2 failed
    at pRetry.retries (file:///****/pRetryStack.mjs:9:29)
    at RetryOperation._fn (/****/node_modules/p-retry/index.js:50:18)
    at Timeout._onTimeout (/****/node_modules/p-retry/node_modules/retry/lib/retry_operation.js:85:10)
    at listOnTimeout (node:internal/timers:569:17)
    at process.processTimers (node:internal/timers:512:7) {
  attemptNumber: 4,
  retriesLeft: 0
}

While the commented out code would print:

Error: foo2 failed
    at foo2 (file:///****/pRetryStack.mjs:12:11)
    at foo1 (file:///****/pRetryStack.mjs:4:18)
    at main (file:///****/pRetryStack.mjs:17:15)
    at file:///****/pRetryStack.mjs:23:1
    at ModuleJob.run (node:internal/modules/esm/module_job:195:25)
    at async ModuleLoader.import (node:internal/modules/esm/loader:337:24)
    at async loadESM (node:internal/process/esm_loader:34:7)
    at async handleMainPromise (node:internal/modules/run_main:106:12)

Suggestion: One possible solution could be to capture the original stack trace and add it as a property to the error, making it easier to trace the origin of the issue while retaining the retry context.

sindresorhus commented 1 week ago

The only way I found to resolve this properly is to not depend on the retry package: https://github.com/sindresorhus/p-retry/pull/79

omril1 commented 1 week ago

The only way I found to resolve this properly is to not depend on the retry package: #79

That sounds way better than what I suggested đŸ˜…