Add ConnectorClientOptions to BotFrameworkAdapterSettings

[EricDahlvang]

Describe the solution you'd like The ability to set ServiceClientOptions when ConnectorClients are created

Describe alternatives you've considered Hacking the createConnectorClientInternal method prototype on BotFrameworkAdapter, and accessing the ConnectorClient's properties directly:

BotFrameworkAdapter.prototype.createConnectorClientInternal = function (serviceUrl, credentials) {
    // Set noRetryPolicy: true so ServiceClient code does not add one.
    // This code adds the default retry policies, with a modified exponentialRetryPolicy
    const client = new ConnectorClient(credentials, { baseUri: serviceUrl, noRetryPolicy: true });

    //retryCount, retryInterval, minRetryInterval, maxRetryInterval, requestTimeout
    client._requestPolicyFactories.push(retryWithTimeoutPolicy(3, 1000 * 2, 1000 * 1, 1000 * 4, 1000 * 5));
    client._requestPolicyFactories.push(systemErrorRetryPolicy());
    client._requestPolicyFactories.push(throttlingRetryPolicy());

    return client;
};

[enhancement]

[stevengum]

Edit: If we moved to 1.8.13 or 1.8.14 as proposed in #2018 we would get built-in proxy support. By allowing for developers to provide their own HttpClient, we would also get KeepAlive (#183) and custom proxy support. The ability to provide their own HttpClient is enough to make this issue a priority.

---

This would also get us proxy support and httpClient injection, so I'm all for this.

I believe passing it in via BotFrameworkAdapterSettings should suffice, what do you think @EricDahlvang?

private createConnectorClientInternal(serviceUrl: string, credentials: AppCredentials): ConnectorClient {
    // Helper method for deep clone and setting of serviceUrl and combining/setting USER_AGENT
    // Clones/creates ClientOptions from BotFrameworkAdapter.settings.clientOptions,
    // a new property with a type of ServiceClientOptions
    const clientOptions = this.cloneOrCreateClientOptions(serviceUrl);

    if (BotFrameworkAdapter.isStreamingServiceUrl(serviceUrl)) {
        // Check if we have a streaming server. Otherwise, requesting a connector client
        // for a non-existent streaming connection results in an error
        if (!this.streamingServer) {
            throw new Error(`Cannot create streaming connector client for serviceUrl ${ serviceUrl } without a streaming connection. Call 'useWebSocket' or 'useNamedPipe' to start a streaming connection.`);
        }

        clientOptions.httpClient = clientOptions.httpClient || new StreamingHttpClient(this.streamingServer);

        return new ConnectorClient(credentials, clientOptions);
    }

    return new ConnectorClient(credentials, clientOptions);
}

[stevengum]

Using diffchecker, I compared the released 1.2.6 ServiceClientOptions with what's in master (2.x) of @azure/ms-rest-js. The latest from 1.x is the same as master for this interface.

~While there are only additions and union types, we should still guard ourselves against relying on this type.~

Guarding against ServiceClientOptions isn't necessary as we have a direct dependency via the subtype ConnectorClientOptions.

[EricDahlvang]

I don't think we can go to 2.0, due to the axios-based to node-fetch change. This would be breaking, since developers are potentially customizing axios in their bot code right now.

[stevengum]

I'm not advocating for going to 2.0 and I'm aware of that change. The screenshots are a snippet indicating what the ServiceClientOptions delta is.