twilio / twilio-node

Node.js helper library
MIT License
1.37k stars 495 forks source link

How to properly utilize keep alive to reuse connections? #941

Open billoneil opened 1 year ago

billoneil commented 1 year ago

Issue Summary

It's unclear from the documentation how to properly re-use connections. There are only sporadic comments and it seems there are multiple settings keepAlive with keepAliveMSecs and/or forever.

We have created a Twilio instance with the following but our APM is still showing every request opening a new tcp connection and querying DNS so the connections are not being reused.

ClientOpts has no way to pass in keepAlive settings so we have to instead pass in the httpClient.

new Twilio(accountId, accountSecret, {
    logLevel: "info",
    httpClient: new RequestClient({
        autoRetry: true,
        maxRetries: 5,
        keepAlive: true,
    }),
});

We have also found the forever option in RequestOptions.

Does this end up overriding the RequestClients keepAlive settings? It's also not clear where this can be passed in?

Looking for an example of how to properly pool connections.

Technical details:

billoneil commented 12 months ago

After several hours of debugging this here is what we found.

The code related to the forever setting invalidates the settings set by keepAlive.

https://github.com/twilio/twilio-node/blob/7519b2f783501890c803a0f4c3d4b64f9c033da9/src/base/RequestClient.ts#L168-L172

This code executes before axios/http.Agent has a chance to set the Connection header. So it will always set Connection: keep-alive if forever is true or set Connection: close if forever is false.

This means the http.Agent keep alive settings end up being ignored. The correct course of action here is probably to remove the else block and let the underlying client (axios) handle the connection header when it is not being explicitly set by the forever setting.

homer0 commented 11 months ago

A workaround that we found to solve this issue was to extend the RequestClient and proxy the call to request:

const twilio = require("twilio");

class MyReqClient extends twilio.RequestClient {
  request(opts) {
    return super.request({
      ...opts,
      forever: true,
    });
  }
}

// ...

const client = new twilio.Twilio(accountId, accountSecret, {
  logLevel: "info",
  httpClient: new MyReqClient({
    autoRetry: true,
    maxRetries: 5,
  }),
});

Not the best solution, but it solves the issue while we wait for a proper fix.