cmayomsft / promptly-dotnet

.NET conversation modelling library for building bots with Bot Builder SDK V4.
MIT License
8 stars 6 forks source link

SubTopic can't maintain `HasActiveTopic` flag. #4

Closed maiko-okawa closed 6 years ago

maiko-okawa commented 6 years ago

Hi, when I add async to SubTopic's OnReceiveActivity() and call await method, SubTopic can't maintain HasActiveTopic flag.

        public async override Task OnReceiveActivity(IBotContext context)
        {
            // HasActiveTopic is always false.
            if (HasActiveTopic)
            {
                await ActiveTopic.OnReceiveActivity(context);
            }

            // in this method, coll some async method.
            await this.SetActiveTopic(ADD_ALARM_TOPIC)
                    .OnReceiveActivity(context);
        }
cmayomsft commented 6 years ago

I'm not able to reproduce this issue when trying it on M4.2 of the BB SDK V4.

OnTurn() (used to be called OnReceiveActivity()) returns a Task. Most of my samples return a Task that the Bot Builder SDK V4 can await/manage, using the guard style of coding:

        public override Task OnTurn(ITurnContext turnContext)
        {
            if (HasActiveTopic)
            {
                return ActiveTopic.OnTurn(turnContext);
            }

            if (State.Alarm.Title == null)
            {
                return SetActiveTopic(TITLE_PROMPT)
                    .OnTurn(turnContext);
            }

            if (State.Alarm.Time == null)
            {
                return SetActiveTopic(TIME_PROMPT)
                    .OnTurn(turnContext);
            }

            OnSuccess(turnContext, State.Alarm);
            return Task.CompletedTask;
        }
    }

But, once you add async (in order to call other async libraries), you need to let async/await manage the Tasks for you (you can't return Tasks like the above).

By doing that in the example below, I was able to call an async method and Topic.HasActiveTopic worked as expected (HasActiveTopic always returned the correct value):

        public override async Task OnTurn(ITurnContext turnContext)
        {
            if (HasActiveTopic)
            {
                await ActiveTopic.OnTurn(turnContext);
            }
            else if (State.Alarm.Title == null)
            {
                var s = await SimulateAsyncMethod();

                await SetActiveTopic(TITLE_PROMPT)
                    .OnTurn(turnContext);
            }
            else if (State.Alarm.Time == null)
            {
                await SetActiveTopic(TIME_PROMPT)
                    .OnTurn(turnContext);
            }
            else
            {
                OnSuccess(turnContext, State.Alarm);
                await Task.CompletedTask;
            }
        }

        private async Task<string> SimulateAsyncMethod()
        {
            await Task.Delay(1000);
            return "Foo!";
        }
    }

Could this be the issue?

If not, please let me know and I’ll keep looking into the issue.

cmayomsft commented 6 years ago

maiko-okawa and I discussed via email, so closing. Will reopen if this doesn't solve the issue.