Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fetch will always hang up about 4s #2348

Closed
himself65 opened this issue Oct 15, 2023 · 4 comments
Closed

fetch will always hang up about 4s #2348

himself65 opened this issue Oct 15, 2023 · 4 comments
Labels
bug Something isn't working

Comments

@himself65
Copy link
Member

himself65 commented Oct 15, 2023

Bug Description

Upstream: nodejs/node#50188

Related: nodejs/node#43522, nodejs/node#48383

undici.fetch will still be hanging up even socket.end has been called, this will cause the node.js 18 server not to close at the right time.

node.js 19/20 won't have this issue because the server will always terminate all socket connections when calling server.close

Reproducible By

I did some small test code on node.js 20. Comparing undici.fetch with node-fetch

You will see that

const http = require("node:http")
const {promisify} = require("node:util")
const net = require("node:net");

async function test1() {
    const server = http.createServer(async (req, res) => {
        res.writeHead(200, {"Content-Type": "text/plain"});
        res.write("Hello world");
        res.end();
    });

    const listenPromisied = promisify(server.listen.bind(server));
    const closePromisied = promisify(net.Server.prototype.close.bind(server));

    await listenPromisied(0, "127.0.0.1");
    const address = server.address();
    const nodeFetch = (await import("node-fetch")).default
    await nodeFetch(`http://${address.address}:${address.port}`);
    console.time("server close");
    await closePromisied();
    console.timeEnd("server close");
}

async function test2() {
    const server = http.createServer(async (req, res) => {
        res.writeHead(200, {"Content-Type": "text/plain"});
        res.write("Hello world");
        res.end();
    });

    const listenPromisied = promisify(server.listen.bind(server));
    const closePromisied = promisify(net.Server.prototype.close.bind(server));

    await listenPromisied(0, "127.0.0.1");
    const address = server.address();
    const nodeFetch = (await import("undici")).fetch
    await nodeFetch(`http://${address.address}:${address.port}`);
    console.time("server close");
    await closePromisied();
    console.timeEnd("server close");
}

test1().then(() => test2());

Expected Behavior

closePromisied should finish in within 100ms

Logs & Screenshots

> git:(main) ✗ node index.cjs        
server close: 0.193ms
server close: 4.001s

Environment

undici >= 4.4.1

Node.js 18.x, 19.x, 20.x

Additional context

@himself65 himself65 added the bug Something isn't working label Oct 15, 2023
@himself65 himself65 changed the title fetch will always pending about 4s fetch will always hang up about 4s Oct 15, 2023
@himself65
Copy link
Member Author

Useful comment: nodejs/node#50188 (comment)

@KhafraDev
Copy link
Member

KhafraDev commented Oct 15, 2023

replace the closes with

const closePromisied = promisify(server.close.bind(server));

edit: just saw this was about v18. It seems tangentially related to other v18 issues with idle connections but I can't definitely tell if it's an issue with undici.

@metcoder95
Copy link
Member

Maybe we can test with the branch? The default keep-alive connections made by undici might be the cause for the long close-up, as none of the peers has ended the connection (the timeout for keep alive is 4s, which more or less matches with the numbers shown); the fix should solve the hanging idle connections if the server ends it properly.

@himself65
Copy link
Member Author

I think the reason is Keep-Alive is by default on node.js, and the server in nodejs 18 won't close all idle connections by default.

Closing as this is intended behavior

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants