Botbuilder-dotnet: 'Brackets' middleware not nearly as trivial as node variant

[brandonh-msft]

If one wanted to write a "brackets" middleware that surrounded each bot response in a starting bracket ({) and an ending bracket (}) it looks like this in the node SDK (typescript):

public async receiveActivity(context: BotContext, next: () => Promise<void>): Promise<void> {
context.reply("{");
await next();
context.reply("}");
}

but in the .Net SDK, something this trivial isn't possible. You might think it'd look like this:

public async Task ReceiveActivity(IBotContext context, MiddlewareSet.NextDelegate next)
{
    context.Reply("{");
    await next();
    context.Reply("}");
}

but when you execute that you get:

So then you might start playing with TPL and try something like this:

public async Task ReceiveActivity(IBotContext context, MiddlewareSet.NextDelegate next)
{
    context.Reply("{");
    await next().ContinueWith(t => context.Reply("}"));
}

but to no avail.

The only way I was able to get this to work properly was to make the middleware both a Receive and a Send middleware like so:

public class Brackets : IMiddleware, IReceiveActivity, ISendActivity
{
    public async Task ReceiveActivity(IBotContext context, MiddlewareSet.NextDelegate next)
    {
        context.Reply("{");
        await next();
    }

    public async Task SendActivity(IBotContext context, IList<IActivity> activities, MiddlewareSet.NextDelegate next)
    {
        await next();
        context.Reply("}");
    }
}

which is far more complex than what we see in node.

This leads me to believe our middleware pipeline in dotnet still isn't quite right.

cc @cleemullins @ryanvolum @tomlm

[cleemullins]

Interesting. I'll look into this.

[ryanvolum]

Not only is this an issue of complexity, but the C# workaround that Brandon wrote has different functionality. SendActivity gets called every time responses are flushed, so if a developer flushes responses mid-turn, your bot would send multiple close brackets.

[cleemullins]

I know what the bug here is. All the middleware in that pipeline is being run to completion, THEN the receive handler is called.

The receive handler should be called when the decent is completed, not when the ascent is completed.

Easy fix, fortunately.

I’ll have the fix + tests in by Monday.

---

From: Ryan [e-mail]
Sent: Saturday, February 24, 2018 7:37:48 AM
To: Microsoft/botbuilder-dotnet
Cc: Chris Mullins; Assign
Subject: Re: [Microsoft/botbuilder-dotnet] 'Brackets' middleware not nearly as trivial as node variant (#184)

Not only is this an issue of complexity, but the C# workaround that Brandon wrote has different functionality. SendActivity gets called every time responses are flushed, so if a developer flushes responses mid-turn, you'd have multiple close brackets in this scenario.

—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHubhttps://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FMicrosoft%2Fbotbuilder-dotnet%2Fissues%2F184%23issuecomment-368236640&data=04%7C01%7Ccmullins%40microsoft.com%7C97418822edd3415bc10508d57b9c8f64%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636550834739047585%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwifQ%3D%3D%7C-1&sdata=j9Pw4tTLQcyj%2BHoIoqvaRJAs9BMgQB2gm2O%2BnsL7kIs%3D&reserved=0, or mute the threadhttps://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FABHICY9h0NrSsGfn2G5s4p9UZUMB7k3vks5tYCzMgaJpZM4SRkdJ&data=04%7C01%7Ccmullins%40microsoft.com%7C97418822edd3415bc10508d57b9c8f64%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636550834739047585%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwifQ%3D%3D%7C-1&sdata=Ya4ca1hGIjsEFY0DurhkRJ2Z3KBY3F6s1FTpLVY0Aqc%3D&reserved=0.

[Stevenic]

@cleemullins the single pipe middleware approach makes this a lot clearer as well. You run the leading edge for all of the middleware plugins, then execute the bots logic, and when that completes all of the trailing edges for plugins will run.

In the current multiple pipe model your left guessing a bit as to what the sequence should be. It's not intuitive for instance that you should hold open the ContextCreated() pipe until after OnReceive() completes.

[cleemullins]

@stevenic, you don't have to convince me the Single Pipeline model is cleaner. I'm there! :)

[cleemullins]

This has been fixed.