Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
billoneil opened this issue Jun 21, 2023 · 2 comments
Open

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

billoneil opened this issue Jun 21, 2023 · 2 comments

Comments

@billoneil
Copy link

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:

  • twilio-node version: 4.11.1
  • node version: v18
@billoneil
Copy link
Author

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

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

if (!headers.Connection && !headers.connection && opts.forever) {
headers.Connection = "keep-alive";
} else if (!headers.Connection && !headers.connection) {
headers.Connection = "close";
}

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
Copy link

homer0 commented Jul 14, 2023

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.

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

No branches or pull requests

2 participants