Don't remember we changed something related to timeout. I saw it once in codesandbox and it can't be reproducible any more, no matter in local test or https://codesandbox.io/s/axios2710-bgphy. Can you confirm it is always the same result with different timeout values?

Yes it is consistent for different values of timeout. However, the break in behavior seems to occur between 0.19.0 and 0.19.1 (i.e. the timeout value is respected for 0.19.0)

@fresidue Can you provide the required information in the issue template? My codesandbox confirms your conclusion, which uses Node.js 10.17.0. But the local environment(OSX 10.14.5, Node.js 10.17.0) always works well.

The timeout functionality breaks with https://github.com/axios/axios/pull/1752/files in axios v0.19.1

This is a pretty problematic bug. If URL is an IP address which is not reachable, the timeout will not be respected.

@chinesedfan I'm running into this as well. I tried to provide the requested information below. Let me know if there is anything I can do to help get this fixed.

• Axios Version: 0.19.2 works as expected in 0.18.1
• OS: macOS 10.15.4 (19E264b)
• Browser: Node.js
• Browser Version: [e.g. 22] v10.16.0
• Additional Library Versions: NA

The timeout functionality breaks with #1752 (files) in axios v0.19.1

@fishcharlie According to @tblasche's discovery, it seems to be due to req.setTimeout. So this is an upstream problem. Can you or someone to check issues of Node.js?

@chinesedfan I'm not finding anything on the Node.js issue section regarding this. I found a few similar things, but they didn't look like the same thing.

I'd be happy to submit an issue and help try to move this forward. My only concern is that I don't have as much knowledge as some of the others here in terms of the story behind this issue an associated PR.

I'm also not sure that we have a concrete failing example that is simple and easy to reproduce. Which makes it harder since anyone looking at the issue will have to dig through the axios codebase.

Any thoughts on how to proceed? Would love to help get this fixed ASAP, but just want to make sure we are all on the same page, and I know I'm slightly more limited in my knowledge level about this.

I'm also not sure that we have a concrete failing example that is simple and easy to reproduce.

@fishcharlie That's really what we need. Once confirmed, we can open an issue to Node.js.

The sample can be extracted from lib/adapters/http.js, which is a simple request based on Node.js http module. You can write two cases. One is using setTimeout like axios v0.18, and the other is like #1752.

@chinesedfan I was only able to reproduce this error when using the NPM package nock, under very specific conditions. Took me a while to get a failing example. Below you can find the most minimal code I was able to use to recreate the problem.

I attempted to create this also with an Express server that just doesn't send any response, and was unable to reproduce that. So I'm wondering if there is something else going on behind the scenes with like the connection or something.

If you think this all looks good I will open an issue on the nodejs/node repository. But I'm still unsure if there is something else about how nock handles things that I need to investigate further first. But that might be past my expertise level at this point.

Let me know what your thoughts are.

Working Code

const http = require("http");

const nock = require("nock");
nock("http://test.com")
	.persist()
	.get("/test")
	.reply(200, (a, b, c) => {});

const req = http.request("http://test.com/test", () => {
	console.log("Response received");
});
setTimeout(() => {
	req.abort();
	console.error("Response timed out");
}, 5000);

Failing Code

const http = require("http");

const nock = require("nock");
nock("http://test.com")
	.persist()
	.get("/test")
	.reply(200, (a, b, c) => {});

const req = http.request("http://test.com/test", () => {
	console.log("Response received");
});
req.setTimeout(5000, () => {
	req.abort();
	console.error("Response timed out");
});

This issue is called as "Call Timeout".

From request's README:

timeout - integer containing number of milliseconds, controls two timeouts.

Read timeout: Time to wait for a server to send response headers (and start the response body) before aborting the request.
Connection timeout: Sets the socket to timeout after timeout milliseconds of inactivity. Note that increasing the timeout beyond the OS-wide TCP connection timeout will not have any effect (the default in Linux can be anywhere from 20-120 seconds)

HTTPRequest#setTimeout calls Socket#setTimeout internally, which only tracks "socket inactivity". socket timer will be reset if any socket activity were detected.

according to documentation:

Sets the socket to timeout after timeout milliseconds of inactivity on the socket. By default net.Socket do not have a timeout.

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.

so, if socket timeout is 1000ms and server sends packets per each 800ms, socket won't trigger timeout event.

Here's quick reproducible codesandbox project:
https://codesandbox.io/s/keen-surf-07ibg

Please take a look into request's source - Read Timeouts has been handled by creating internal timers.

Quick & Dirty workaround at this moment:

// Quick-and-dirty solution for call-timeout issue
// @see https://github.com/axios/axios/issues/2710
async function patchedAxios<T>(options: AxiosRequestConfig): Promise<AxiosResponse<T>> {
  if (options.timeout) {
    const cancelSource = axios.CancelToken.source();
    const readTimeoutTimer = setTimeout(() => {
      cancelSource.cancel(`timeout of ${options.timeout}ms exceeded`);
    }, options.timeout);

    try {
      return await axios({
        ...options,
        cancelToken: cancelSource.token,
      });
    } catch (e) {
      if (axios.isCancel(e)) {
        throw new Error(e.message);
      }

      throw e;
    } finally {
      clearTimeout(readTimeoutTimer);
    }
  } else {
    return axios(options);
  }
}

@chinesedfan Wasn’t the reason for this change (and not using setTimeout/internal timers) due to a security vulnerability? Wouldn’t the code @mooyoul just suggested have that same security vulnerability?

Another reproducible project: https://github.com/mooyoul/broken-axios-timeout

@mooyoul Again, I'm pretty sure your workaround has a security vulnerability in it. I'd suggest you read the original comment in #1752.

@fishcharlie My workaround does not replace existing socket timeout handlers.
It just add additional timer to handle Call Timeout, Connection timeout will be handled by axios's side as-is (v0.19.1 and v0.19.2 in this case) - because these axios releases use HTTPRequest#setTimeoutinternally.

To clarify, I'm not meaning that we should handle just single case.
We MUST handle both Connection Timeout and Call Timeout (as @RikkiGibson pointed that in PR #1752) cases like request did.

Connection Timeout can be handled by calling HTTPRequest#setTimeout.
but as i earlier mentioned, HTTPRequest#setTimeout just tracks "socket inactivity". It does not handle "Call Timeout". "Call Timeout" should be handled by user-land. because current Node.js built-in http client does not support Call Timeout handling.

About "security vulnerability" that you've mentioned, I'm assuming that was caused by slow sockets. Slow sockets exhausted resources and blocked event loop. This issue was resolved by limiting connection timeouts as PR #1752 proposed. We just need to add internal timers to detect read timeout.

@mooyoul Ahhh, got it. Is there a reason you think your workaround can't be turned into a PR on this main repo?

@mooyoul If you think this is the correct path forward, I'd be happy to work on a PR and submit it to this repo. Let me know. Would love to get this fixed in a release so I don't have to use any workarounds or anything.

@mooyoul @fishcharlie Really appreciated for your investigations. In summary, we should keep both of timers, right? Glad to see a PR with some tests.

Yeah. I’m working on this issue and PR will be available in next few hours :)

@mooyoul Awesome! Let me know if I can help at all!! Thanks for all the work you’ve put into this so far.

@mooyoul Just wanted to check in and see if there was any updates on this. Is there any support I can provide in getting this across the finish line?

Hello All, I've spent few hours to know how timeouts are actually handled in both XHR and Node.js built-in HTTPClient.

Before starting a talk, I want to clarify variety types of timeouts.

In general, There are a lot of timeout types:

DNS Lookup Timeout

"DNS Lookup Timeout" indicates that client submitted the query to the DNS server,
but didn't get a response in specified time.

It usually occurs when the DNS server you queried was having a problem and couldn't reply.
Network errors could be another reason as well.

Connect Timeout

"Connect Timeout" indicates that client created socket and sent "TCP SYN" packet to server,
but server dropped SYN packet and never replied SYN-ACK packet in specified time.

You can quickly reproduce "Connect Timeout" by connecting to non-routable destination like 10.255.255.1.

It usually occurs when server firewall drops SYN packets. Bad network connectivity could be another reason as well.

See also: http://willbryant.net/overriding_the_default_linux_kernel_20_second_tcp_socket_connect_timeout

Read Timeout

"Read Timeout" indicates that time of inactivity between two data packets
when waiting for the server's response exceeded specified time.

It usually occurs when server was super busy. Bad network condition could be another reason as well.

Write Timeout

"Write Timeout" indicates that time of inactivity between two data packets
when sending the request to the server exceeded specified time.

It usually occurs when server was super busy. Bad network condition could be another reason as well.

Call Timeout

"Call Timeout" indicates that "complete" HTTP call didn't completed in specified time.
It includes entire timings from DNS Lookup to Response body download.

So many timeout types, right?

I wanted to know how platforms actually handle these timeouts. and here are what i've found during experiment:
(you can see full experiment results from https://github.com/mooyoul/timeout-experiments)

• timeout option in XHR defines "Call Timeout" which tracks entire HTTP call (from DNS Lookup to Response body download)
• timeout option in http#request in Node.js defines "Idle timeout" which tracks "socket inactivity"
• Currently, HTTPRequest#setTimeoutonly tracks "idle timeout". which means "DNS Lookup timeout", "Connect Timeout", and "Call Timeout" cannot be limited or detected.

Luckily, In Node.js environment, Unsupported timeout types can be implemented from user-land (except connect timeout - it has a limitation of OS-wide setting). but In Browser environment, it cannot be implemented.

So the problem is what we should choose - Keep idiomatic timeout behavior, or Provide more timeout flexibility only in Node.js.
I chose keeping idiomatic behavior. I am one of person who wants more timeout flexibility/options. but it makes axios difficult to maintain.

I'm not sure which way is the best - Please leave your thoughts!

So at this moment, I'm opening a PR that just reverts previous changes, with additional tests.

I just proposed PR #2874

@RikkiGibson @fishcharlie @chinesedfan Would you leave a feedback to upper comment? I want to hear your thoughts. Thanks!

Hey, I just made my way here from the linked Nock issue.

One thing I've noticed as missing from this thread is that when the change was made in 0.19.1, req.setTimeout is not calling Node's native ClientRequest.setTimeout (as the comment implies). Because as of 1.7.0, RedirectableRequest#.setTimeout uses a custom timeout logic that does not match Node's (PR).

I don't know the internals of Axios that well, so I'm not sure if this helps at all, but it does explain why Nock's delayBody doesn't work.

default headers are also not working

in 0.18.1 following worked just fine

axios.defaults.headers.common['X-Locale'] = 'en-gb';

after updating to latest this no longer works