grantila / fetch-h2

HTTP/1+2 Fetch API client for Node.js
MIT License
336 stars 16 forks source link

connection timeout not supported #99

Open stefan-guggisberg opened 4 years ago

stefan-guggisberg commented 4 years ago

The timeout fetch option doesn't account for the time it takes to establish a connection.

An example:

  await fetch('http://example.com:81', { timeout: 1000 });

The above statement hangs forever (i.e. ~75s in my case on macos). Aborting the request using AbortController doesn't work either. The timeout option seems only to apply for already established connections.

The problem is here: https://github.com/grantila/fetch-h2/blob/master/lib/context-http1.ts#L375, where no timeout is set. Something like the following should work (sorry, not Typescript):

     async makeNewConnection(url, timeout) {
        let handled = false;
        return new Promise((resolve, reject) => {
            const { hostname, port } = utils_1.parseInput(url);
            const socket = net_1.createConnection({ port: parseInt(port, 10), host: hostname, timeout }, () => {
                // reset connect timeout
                socket.setTimeout(0);
                resolve(socket);
            });
            socket.once("error", (err) => {
                if (!handled) {
                    handled = true;
                    reject(err);
                }
            });
            socket.once("timeout", () => {
                if (!handled) {
                    handled = true;
                    reject(new core_1.TimeoutError());
                }
                socket.destroy();
            });
        });
    }

SImilar problem exists with https:// where a timeout can be set in the context options, e.g. new Context( { session: { timeout: 1000 } }), but the timeout event is not handled:

https://github.com/grantila/fetch-h2/blob/master/lib/context-https.ts#L53