[Streaming, Preview] New connections cause existing connections to break

[henryjenkins]

Versions

using botframework-4.6.0-preview2
on nodejs 10.15.3

Describe the bug

Given a directline-speech client is connected to a bot via a Bot Channel Registration in Azure,
when a new directline-speech client connects,
then the existing client will no longer receive any activities

To Reproduce

Steps to reproduce the behaviour:

• Setup a bot with support for directline-speech. I used one of the sample bots: https://github.com/microsoft/BotBuilder-Samples/tree/master/samples/javascript_nodejs/05.multi-turn-prompt
  You will need to update this sample:
  -- update package.json to use the correct version of botframework
  -- update index.js
  --- add enableWebSockets: true to the config passed to the BotFrameworkAdapter
  --- add an argument to the restify constructor: { handleUpgrades: true }
  --- add a GET handler for /api/messages (just copy the POST handler)

• Configure a bot channel registration in Azure to connect to your bot. I used Ngrok to create a public HTTPS endpoint for my bot running locally.
  Ensure you configure the MSApp ID and password correctly in your .env file.

• Configure a directline-speech client to connect to your Azure Bot Channel registration.
  I used the sample client found here: https://docs.microsoft.com/en-us/samples/azure-samples/cognitive-services-direct-line-speech-client/cognitive-services-direct-line-speech-client/
  I also tried modified versions of the Java client SDK (https://github.com/Azure-Samples/cognitive-services-speech-sdk/tree/master/samples/java/jre/console) and the Javascript client sdk (https://github.com/Azure-Samples/cognitive-services-speech-sdk/tree/master/samples/js/node)
  You will need to configure the client with the subscription key and region that you used when setting up the bot channel registration.

• Start one instance of the client. You should get a response from your bot, and you should be able to continue the conversation across multiple turns.

• Start a second instance of the client. This second instance should work fine, but the original instance will no longer receive responses from the bot.

Expected behavior

The JS Bot adapter should support multiple concurrent connections

Additional context

I tried the same test with the DotNet C# version of the same sample bot (05), and it does not suffer from the same problem.
[bug]

[axelsrz]

@carlosscastro could you take a look at this please?

[stevengum]

Closing as a fix was merged into the 4.6-preview-staging branch via #1412

per: @stevengum the bug wasn't actually fixed in this PR. Investigation ongoing.

[stevengum]

Currently, the singleton Adapter pattern used in the samples doesn't work for supporting multiple WebSocket connections.

botbuilder-samples/experimental/directline-speech/javascript_nodejs/02.echo-bot - 651f405

const adapter = new BotFrameworkAdapter({
    appId: process.env.MicrosoftAppId,
    appPassword: process.env.MicrosoftAppPassword,
    enableWebSockets: true
});

const myBot = new EchoBot();

server.post('/api/messages', (req, res) => {
    adapter.processActivity(req, res, async (context) => {
        // Route to main dialog.
        await myBot.run(context);
    });
});

// Listen for GET requests to the same route to accept Upgrade requests for Streaming.
server.get('/api/messages', (req, res) => {
    adapter.processActivity(req, res, async (context) => {
        // After connecting via WebSocket, run this logic for every request sent over
        // the WebSocket connection.
        await myBot.run(context);
    });
});

In Node.js the streaming implementation relies on creating a new Adapter for every request.

The correct pattern for supporting multiple websocket connections is to create a new BotFrameworkAdapter for every Upgrade request. The code below works in 4.6.0-preview2.

// Listen for GET requests to the same route to accept Upgrade requests for Streaming.
server.get('/api/messages', (req, res) => {
    // Create an adapter scoped to this WebSocket connection to allow storing session
    // data.
    const streamingAdapter = new BotFrameworkAdapter({
        appId: process.env.MicrosoftAppId,
        appPassword: process.env.MicrosoftAppPassword,
        enableWebSockets: true
    });
    streamingAdapter.processActivity(req, res, async (context) => {
        // After connecting via WebSocket, run this logic for every request sent over
        // the WebSocket connection.
        await myBot.run(context);
    });
});

For Named Pipes, a singleton adapter is usable because only one Named Pipe connection is supported from the App Service Extension.

[stevengum]

The combined code snippets above could look like the following:

const settings = {
    appId: process.env.MicrosoftAppId,
    appPassword: process.env.MicrosoftAppPassword,
    enableWebSockets: true
};

const adapter = new BotFrameworkAdapter(settings);
const myBot = new EchoBot();

server.post('/api/messages', (req, res) => {
    adapter.processActivity(req, res, async (context) => {
        await myBot.run(context);
    });
});

// Listen for GET requests to the same route to accept Upgrade requests for Streaming.
server.get('/api/messages', (req, res) => {
    // Create an adapter scoped to this WebSocket connection to allow storing session
    // data.
    const streamingAdapter = new BotFrameworkAdapter(settings);
    streamingAdapter.processActivity(req, res, async (context) => {
        // After connecting via WebSocket, run this logic for every request sent over
        // the WebSocket connection.
        await myBot.run(context);
    });
});