TooTallNate / proxy-agents

Node.js HTTP Proxy Agents Monorepo
https://proxy-agents.n8.io
872 stars 229 forks source link

Timeout option doesn't work as expected #256

Open archvlad opened 8 months ago

archvlad commented 8 months ago

🐞 Bug Report

Describe the bug

When I send request using node-fetch without proxy and it take a long time for server to respond I use timeout to limit the time of awaiting response. After timeout passed error is thrown. But when I make request with proxy using HttpsProxyAgent to the same server I receive error not after timeout but after about 30 seconds.


To Reproduce

  1. Run this code:
    
    const fetch = require("node-fetch");
    const { HttpsProxyAgent } = require("https-proxy-agent");

async function main() { let agent = new HttpsProxyAgent("http://localhost:8888", { timeout: 5000, });

await fetch("https://1.1.1.1:1111", {
    agent,
    timeout: 5000,
});

}

main();


---

### **Expected behaviour**
<!-- A clear and concise description of what you expected to happen. -->

```javascript
FetchError: network timeout at: https://1.1.1.1:1111/

Your environment

archvlad commented 8 months ago

I think the problem is around these lines of code: https://github.com/TooTallNate/proxy-agents/blob/b133295fd16f6475578b6b15bd9b4e33ecb0d0b7/packages/https-proxy-agent/src/index.ts#L134-L141

In socks-proxy-agent, by the way, timeout works fine. This is the snippet where timeout implemented. https://github.com/TooTallNate/proxy-agents/blob/b133295fd16f6475578b6b15bd9b4e33ecb0d0b7/packages/socks-proxy-agent/src/index.ts#L152-L159 When timeout is over socks_1.SocksClient.createConnection(socksOpts)) throws this error:

SocksClientError: Proxy connection timed out

And cleanup() function is called.

However, in https-proxy-agent I don't see such mechanism.

aalgogiver commented 8 months ago

Agree, socket hangs indefinitely in my case too. Timeout option is not taken into consideration by the implementation.

@archvlad , the fix seems to be pretty straightforward. Would you like to suggest a PR for this?