Skip to content
This repository has been archived by the owner on May 5, 2023. It is now read-only.

Pass timeout to SocksClient to prevent open sockets #26

Closed
berstend opened this issue Aug 23, 2018 · 2 comments
Closed

Pass timeout to SocksClient to prevent open sockets #26

berstend opened this issue Aug 23, 2018 · 2 comments

Comments

@berstend
Copy link

berstend commented Aug 23, 2018

Hey there,
thanks a lot for the agents - they're really useful. 馃憤

I ran into an issue with socks proxy sockets not being closed properly,
which expressed itself in the node process not shutting down in time.

The following is an example of a "faulty" socks proxy (it's actually a http proxy):

const agent = new ProxyAgent('socks5://163.172.173.187:3000')
agent.timeout = 1 * 1000 // socket timeout, relevant for socks

The above will correctly timeout the connection after 1 second (agent-base functionality),
but the underlying socket used by socks remains open for the default timeout of 30s.

The fix is very simple (I confirmed it's working), we just have to pass the timeout property to the socks client (Socks docs).

I'd love to present you a finished PR for this small fix, but unfortunately I'm somehow unable to find a way to use the existing timeout property from the agent-base Class. 馃槄

Would be great if you could have a quick look, if you could tell me how to re-use the existing timeout property I'm happy to submit a PR.

Thanks!

@berstend
Copy link
Author

As this was blocking I did a quick fork and abused the proxy options object. 馃槃

yarn add berstend/node-socks-proxy-agent-with-timeout
import * as ProxyAgent from 'proxy-agent'
import * as SocksProxyAgentWithTimeout from 'socks-proxy-agent-with-timeout'

const agent = new ProxyAgent({
  protocol: 'socks5:',
  hostname: '163.172.173.187',
  port: '3000',
  timeout: 3 * 1000, // socks timeout
  proxies: {
    socks5: SocksProxyAgentWithTimeout
  }
})
agent.timeout = 1 * 1000 // general proxy agent timeout

@Kikobeats
Copy link
Collaborator

Closing because a timeout is already associated with the connect command:
https://github.com/TooTallNate/node-socks-proxy-agent/blob/master/src/agent.ts#L166

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

Successfully merging a pull request may close this issue.

2 participants