fix: 🐛 fix stuck in http request

[binsee]

Fixes: #80

[hcfw007]

Also I have done a little test

    stream
            .on('abort', console.log)
            .on('connect', console.log)
            .on('continue', console.log)
            .on('information', console.log)
            .on('response', console.log)
            .on('socket', console.log)
            .on('timeout', console.log)
            .on('upgrade', console.log)
            .on('close', console.log)
            .on('drain', console.log)
            .on('error', console.log)
            .on('finish', console.log)
            .on('pipe', console.log)
            .on('unpipe', console.log)

none of this were fired when network disconnected

[binsee]

See #80 for updated results

[huan]

I'm thinking about we need an automation unit test for guarding this problem automatically.

[binsee]

@huan
I updated the test. And made some modifications with reference to the nodejs code, it should work fine now.
However, I still think 60 seconds is too long and 5~10 seconds is perfectly sufficient. Too long a timeout and it will just wait in vain and throw an error.
This can be learned from should not timeout and should timeout in the test.

[huan]

https://betterstack.com/community/guides/scaling-nodejs/nodejs-timeouts/

In browsers, fetch() usually times out after a set period of time which varies amongst browsers. For example, in Firefox this timeout is set to 90 seconds by default, but in Chromium, it is 300 seconds.

[binsee]

Ok, I don't think there is enough testing in various scenarios now, and there is no need to set the default value too small.
In use, I can set the environment variable FILEBOX_HTTP_TIMEOUT as needed to achieve the effect I want, which is enough.
So are there any other questions now? Hope this can be merged sooner, to solve the problem I'm having.

[huan]

The latest problem is that the unit test clock is not mocked successfully: It will take a long time to finish the test, instead of seconds.

We have to figure out how to make the clock mock work before it can be merged.

[binsee]

I've spent a day researching this problem, but didn't find a solution. Therefore, I changed the test to run concurrently to reduce the overall time taken by this test.
Looks like looking at the source code of the nodejs project is required to find a clue, but I don't have enough time to do so right now.
Can we merge this first and then optimize the test time?

[binsee]

I investigated the source code of nodejs and found that the problem should be that socket.setTimeout uses the built-in setUnrefTimeout instead of the usual setTimeout.
Regardless of sion or jest, they only hook the common setTimeout, and there is no hook built-in setUnrefTimeout, so it cannot be mocked.

• This is the socket's setTimeout call chain:

https://github.com/nodejs/node/blob/v16.x/lib/_http_client.js#L926-L952

https://github.com/nodejs/node/blob/v16.x/lib/net.js#L533

https://github.com/nodejs/node/blob/v16.x/lib/net.js#L74-L85

https://github.com/nodejs/node/blob/v16.x/lib/net.js#L237-L265

https://github.com/nodejs/node/blob/v16.x/lib/internal/timers.js#L372-L380

• This is the general setTimeout:

https://github.com/nodejs/node/blob/v16.x/lib/timers.js#L140-L168

[huan]

You do not have to keep convincing me that this is impossible, as I believe everything should be testable.

Let me try to figure it out myself later.

Thanks.

[binsee]

Have you found any good solution?