Non-extensible rejections result in unhandled rejection

[openreply-dleinhaeuser]

If the input function somehow rejects with a non-object, decorateErrorWithCounts will throw an error which leads to an unhandled rejection.

Example:

const pRetry = require('./');

pRetry(() => Promise.reject(Object.preventExtensions(new Error())))
    .catch(console.error);

Expected result:
The retry handler treats the non-extendable like any other (minus the decorating of course).

Actual result:
An UnhandledPromiseRejectionWarning is generated, the code continues on neither the resolution nor the rejection path.

Generally the code seems to not be well defended against errors from unexpected sources. For example. throwing from the onFailedAttempt function would also result in an unhandled rejection.

[schmiegelt]

As I had the same issue, I solved it by wrapping the actual function call in an own function. In there, the non-extendable error can be cought and replaced by an extendable one.

So, instead of

const response = await doSomething()

I created a new function

async function trySomething() {
    try {
        await doSomething()
        return "Success"
    }
    catch(e) {
        //console.log(e)
        throw(new Error("Could not do something"))
    }
}

and used this in the p-reply code

const run = async () => {
	const response = await trySomething()

	return response
};

[marchuffnagle]

This seems like a tricky one to fix in p-retry without making a breaking change. p-retry itself doesn't appear to read the two properties set on the error (attemptNumber and retriesLeft), but it does specify that they'll be available in the error passed to onFailedAttempt and shouldRetry.

Short of making a breaking change, should the documentation be updated to say, "if your function to be retried throws something that can't be extended, this is going to get weird"? I can't imagine this happens that often, but it's certainly more than zero.