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

socket is destroyed on timeout #228

Open
orgads opened this issue Jul 3, 2023 · 3 comments
Open

socket is destroyed on timeout #228

orgads opened this issue Jul 3, 2023 · 3 comments

Comments

@orgads
Copy link

orgads commented Jul 3, 2023

Hi,

Can you please explain why do you destroy the socket on timeout?

According to the docs of socket.setTimeout:

When an idle timeout is triggered the socket will receive a 'timeout' event but the connection will not be severed. The user must manually call socket.end() or socket.destroy() to end the connection.

And since follow-redirects is used as a drop-in replacement for http/https (at least by axios), I'd expect its behavior to follow the same rules.

My use-case is axios of a streaming request. If I activate timeout, even after receiving 200 OK the connection is sometimes aborted during streaming.

Example:

import axios from 'axios';
import http from 'http';
import Throttle from 'throttle';
import fs from 'fs';
import url from 'url';
import { Writable } from 'stream';

const server = http.createServer((req, res) => {
  res.writeHead(200);
  fs.createReadStream(url.fileURLToPath(import.meta.url)).pipe(new Throttle(32)).pipe(res);
});
const port = 4040;
server.listen(port, () => {
    console.log(`Server is running on http://localhost:${port}`);
});

setTimeout(() => server.close(), 2000);

class WritableTest extends Writable {
  _write() {}
}

async function main() {
  const buffer = new WritableTest();
  const url = `http://localhost:${port}/test`;
  console.log('start');
  const response = await axios(url, {
    timeout: 20,
    responseType: 'stream',
  });
  console.log(`${response.status} ${response.statusText}`);
  response.data.pipe(new Throttle(300)).pipe(buffer);
  response.data.on('end', () => console.log('end'));
  response.data.on('error', (err) => console.error('error', err));
}

main().catch((err) => console.error(err.message));

Output:

start
Server is running on http://localhost:4040
200 OK
error Error: aborted
    at connResetException (node:internal/errors:717:14)
    at Socket.socketCloseListener (node:_http_client:462:19)
    at Socket.emit (node:events:525:35)
    at TCP.<anonymous> (node:net:322:12) {
  code: 'ECONNRESET'
}

If I set maxRedirects: 0 then the connection is not aborted.

@RubenVerborgh
Copy link
Collaborator

Thanks @orgads, we indeed net some work on the timeout implementation.

I think this issue would be resolved with #181?

@orgads
Copy link
Author

orgads commented Jul 15, 2023

I'm not sure. This is a different problem. The internal timers are cleared on response, but socket.setTimeout remains active, and should still report a timeout event, and I don't see that the timeout -> destroy callback is removed on clearTimer.

@orgads
Copy link
Author

orgads commented Nov 12, 2023

Any progress with this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants