superagent icon indicating copy to clipboard operation
superagent copied to clipboard

TCP reuse

Open yigaldviri opened this issue 4 years ago • 5 comments

Hi, I'm using superagent and while inspecting the network I noticed that superagent is creating a new TCP connection for each request. I'm using superagent for a sequence of requests which results in a large number of TCP connections (can reach several hundreds).

I tried to follow this idea and use agentkeepalive package however this approach has some downsides:

  1. While superagent works out-of-the-box with http and https, agentkeepalive needs to be defined per protocol.
  2. As a result of the previous section, in case I do an http request that redirects to https request I get an error of Protocol "https:" not supported. Expected "http:" since the protocol has changed.
  3. As a result of section 2 the app crushes since with an uncaughtException (which happens only upon redirects and not when using the wrong protocol i.e. the http agentkeepalive for https request)

Needless to say that using request.set('Connection', 'keep-alive'); didn't solve it.

My questions are:

  1. Is there un updated solution for the keep-alive problem?
  2. Is there a built-in option in superagent to define the reuse of TCP connections? If not, is there a plan to support one?

Thanks

yigaldviri avatar Aug 05 '20 08:08 yigaldviri

PR welcome!

niftylettuce avatar Aug 15 '20 08:08 niftylettuce

Tried to find a solution both in superagent and in agentkeepalive repos but it required such changes in both repos that I don't think someone (including me) would approve.

So, after looking deep into the code with @omrilitov , our workaround was to intercept the request every time and since superagent reuse the same request object when redirecting we reset the agent as well. It looks like this:

 const _oldRequest = request.request;
  request.request = function (...args) {
    // first intercept the request and do our stuff
    const { url } = this;

    if (url.startsWith('https:')) {
      this.agent(sslAgent);
    } else {
      this.agent(agent);
    }

    // then continue with superagent logic
    _oldRequest.apply(this, ...args);
  };

Hope this is helping any other developers who run into this issue.

@omrilitov and I thought about creating some kind of an interceptors for superagent - is that something you guys will consider?

yigaldviri avatar Aug 23 '20 10:08 yigaldviri

Couldn't you just use xhook? See https://github.com/jpillora/xhook and some examples are in @cabinjs at https://cabinjs.com/?id=automatic-request-logging.

niftylettuce avatar Aug 23 '20 10:08 niftylettuce

xhooks is designed for client-side interceptors, we needed the changes for server-side

omrilitov avatar Aug 23 '20 10:08 omrilitov

Ah, yeah, I even filed this issue regarding that here https://github.com/jpillora/xhook/issues/103. I'd be open to interceptors, I already have them in Frisbee at https://github.com/niftylettuce/frisbee (if you want a Fetch implementation or inspiration).

niftylettuce avatar Aug 23 '20 10:08 niftylettuce