Botbuilder-js: Integrate AgentKeepAlive fix into botframework-connector

[Stevenic]

High volume azure hosted bots can start getting timeouts due to an SNAT issue. We have a blog post for how to fix this but I think we could build the fix into the connector such that's it easier to turn on and configure. I'm not sure it should be on by default but I'm all for making it easy to enable.

[justinwilaby]

The challenge here is to get both proxy support and keep-alive working together. It seems that now you can have just one or the other.

[damadei]

Guys, any news on this? I'm doing a benchmark with the partner and we are seeing 1/2 of the performance from a Node.js bot compared to a .NET Core one. If somebody can point me where this call is made I could try to implement the change to check if this is the cause of the performance difference I'm seeing and submit a PR.

[EricDahlvang]

Hey @damadei

Thank you for your willingness to help out with this issue. I'd like to work with you it, and I've emailed you offline.

[EricDahlvang]

It seems that the reason BotBuilder-dotnet does not have these issues is due to the caching of ConnectorClients https://github.com/Microsoft/botbuilder-dotnet/blob/master/libraries/Microsoft.Bot.Builder/BotFrameworkAdapter.cs#L59 as well as using a static HttpClient https://github.com/Microsoft/botbuilder-dotnet/blob/master/libraries/Microsoft.Bot.Builder/BotFrameworkAdapter.cs#L886 These two patterns, or something similar, should also be implemented in BotBuilder-js.

[stevengum]

@EricDahlvang https://github.com/Azure/ms-rest-js/issues/308 and https://github.com/axios/axios/pull/1831

[sgellock]

Now that we are off autorest, we can modify the code that could address this. moving out of 4.5

[stevengum]

This is still blocked; the last release of the 1.x line of @azure/ms-rest-js does not contain a way to inject a custom Agent with keepAlive configured to true (AxiosHttpClient.sendRequest()).

[stevengum]

Actually, previous comment only applies for using the built-in Node.js HttpClient (AxiosHttpClient in 1.x of ms-rest-js).

If https://github.com/microsoft/botbuilder-js/issues/2013 is addressed and ServiceClientOptions is surfaced to developers, they would be able to provide their own HttpClient which could use their own HTTP request library with Agent support.

[EricDahlvang]

With https://github.com/microsoft/botbuilder-js/issues/2013 merged, it is now possible to provide a custom @ms-rest-js.HttpClient as a parameter to BotFrameworkAdapter:

const adapter = new BotFrameworkAdapter({
    appId: process.env.MicrosoftAppId,
    appPassword: process.env.MicrosoftAppPassword,
    clientOptions: { httpClient: new CustomAxiosHttpClient() } 
});

However, this is still very inconvenient and cumbersome to implement. The AxiosHttpClient provided by the 1.x version of ms-rest-js contains some custom error handling, abort logic, stream support, etc. Much of this is not needed by the sdk, but in order to use agent keepAlive our recommendation for R9 will be to copy this file into the bot and modify axios.create() to include:

httpAgent: new http.Agent({ keepAlive: true }),
httpsAgent: new https.Agent({ keepAlive: true }),

We could make this easier.

Lastly, we need to consider options for providing keepAlive agents to QnAMaker, LUIS, and storage providers.

[Bradrajkumar]

Hi Team,
i am facing one issue of connection reset an need the same requirement so that i can take a reference to implement the code base directly in my solution.
FYI
I am not using any node module named : - @ms-rest-js.
any input or complete solution will be truly helpful for me.

[EricDahlvang]

Hi @Bradrajkumar

Currently, the sdk provides and httpClient parameter in BotFrameworkAdapter's ConnectorClientOptions:

const adapter = new BotFrameworkAdapter({
    appId: process.env.MicrosoftAppId,
    appPassword: process.env.MicrosoftAppPassword,
    clientOptions: { httpClient: new CustomAxiosHttpClient() } 
});

Here is a CustomAxiosHttpClient implementation:

customAxiosHttpClient_js.txt

Unfortunately, the sdk does not currently expose an agent parameter for QnAMaker or Luis implementations which are using node-fetch

[stevengum]

🚀 https://github.com/Azure/ms-rest-js/pull/404 🚀