Got: The onCancel handler was attached after the promise settled

[yovanoc]

Describe the bug

• Node.js version: 14.13
• OS & version: MacOS Catalina

The onCancel handler was attached after the promise settled

Actual behavior

This unhandled exception is not catch by a try/catch, but I see that it happen after a timeout error like this issue #1460

Checklist

• [x] I have read the documentation.
• [x] I have tried my code with the latest version of Node.js and Got.

[szmarczak]

Can you please post some code?

[bkarlson]

Node.js version: 15.1.0
OS & version: Windows 10

Just encountered the same, very hard to repro, looks like a race-condition, I will try to dig deeper, but is there any way to catch such exception and just keep running?

Stack:

error: unhandledRejection: The `onCancel` handler was attached after the promise settled.
Error: The `onCancel` handler was attached after the promise settled.
    at onCancel (scripts\node_modules\p-cancelable\index.js:46:12)
    at makeRequest (scripts\node_modules\got\dist\source\as-promise\index.js:38:13)
    at Request.<anonymous> (scripts\node_modules\got\dist\source\as-promise\index.js:143:17)
    at Object.onceWrapper (node:events:434:26)
    at Request.emit (node:events:327:20)
    at Timeout.retry (scripts\node_modules\got\dist\source\core\index.js:1261:30)
    at listOnTimeout (node:internal/timers:555:17)
    at processTimers (node:internal/timers:498:7) 

[szmarczak]

Thanks for the report @bkarlson. Just to confirm - are you running Got 11.8.0?

[bkarlson]

Yes, 11.8.0, and the issue seems to be related to retry logic, because if I disable it (retry: 0) the error never happens. If I have retry: 3 with other retry options set to default, Got will throw this exception at some point.

[Giotino]

An easy fix would be using if (!thePromise._isPending) before setting the onCancel handlers, but it's undocumented.

I'm still investigating on the root cause.

[Kikobeats]

This is still happening.

Here my error trace: https://sentry.io/share/issue/6f578a2a936943cca5afe367e6732995/

[szmarczak]

@yovanoc @Kikobeats @bkarlson Can you try again? The p-cancelable package has received a neat fix that could potentially fix this error. Make sure to update the npm dependencies.

[Kikobeats]

@szmarczak I'm still experimenting the error

https://sentry.io/share/issue/c607e343ef6345c38ad391a7c3f43149/

node version is: v14.16.0
got version is: 11.8.2
p-cancelable: 2.1.0

[szmarczak]

@Kikobeats Can you show the line(s) where you call got? Do you pass any hooks? Do you attach any event handlers?

[szmarczak]

@Kikobeats can you replace dist/as-promise/index.js with https://gist.github.com/szmarczak/310ee80c1085bdacd042c64fd992ef76 ? I made some changes so it logs what happened.

[szmarczak]

@bkarlson @yovanoc Can you too try the above?

[Kikobeats]

@szmarczak In my case is hard to say since there are a couple of dependencies using got and the stack trace is not revealing the details about which the origin of the problem.

Also, since this is running a production environment, I can't replace it in an easy way.

Maybe can you release a got RC version so I can force to be used?

[szmarczak]

I have an idea how to reproduce this. request.destroy() may be called, the promise may be canceled, but request hasn't been emitted yet and there is no check if it has been destroyed. Will check this.

[szmarczak]

Actually no because it's destroyed here https://github.com/sindresorhus/got/blob/4cce4de19ed577f481ba45a023b395a29cadce86/source/core/index.ts#L1469-L1472

[szmarczak]

I think I finally may be able to reproduce this. Actually I can't figure this out :( This is definitely not via afterResponse hook. This must be related to timeouts.

[szmarczak]

@Kikobeats What is the timeout event? Is it request event? It'd be best if you shared the entire timeout error, including timeoutError.timings.

[szmarczak]

Also can you do npm ls got and show me which versions are being used?

[szmarczak]

@Kikobeats @bkarlson @yovanoc If your project uses Got directly you can try

const isResponseOk = response => {
    const {statusCode} = response;
    const limitStatusCode = response.request.options.followRedirect ? 299 : 399;

    return (statusCode >= 200 && statusCode <= limitStatusCode) || statusCode === 304;
};

const instance = got.extend({
    hooks: {
        afterResponse: [
            response => {
                if (isResponseOk(response)) {
                    response.request.destroy();
                }

                return response;
            }
        ]
    }
});

If so, please let me know if that fixes the issue.

[Kikobeats]

Hey @szmarczak, I'm running your code snippet on my production servers since yesterday and I didn't experiment errors until now (maybe it needs a bit of extra time)