Skip to content

Conversation flow breaks when using more than 1 skill in a dialog #6248

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
14 tasks
silverbulletgt opened this issue Mar 30, 2021 · 16 comments
Open
14 tasks
Assignees
Labels
Bot Services Required for internal Azure reporting. Do not remove. Do not change color. bug Indicates an unexpected problem or an unintended behavior. customer-replied-to Indicates that the team has replied to the issue reported by the customer. Do not delete. customer-reported Issue is created by anyone that is not a collaborator in the repository. P1 Painful if we don't fix, won't block releasing
Milestone

Comments

@silverbulletgt
Copy link

silverbulletgt commented Mar 30, 2021

Github issues should be used for bugs and feature requests. Use Stack Overflow for general "how-to" questions.

Version

Bot Framework SDK - 4.12.2
Composer - 1.4.0

Describe the bug

I created a git repo to demonstrate the issue MultiSkillComposerBot

The project contains 2 components

  • Bot.Skills - a Web API which exposes 2 skills
  • ComposerMultiSkillDialog - a project created in the Bot Framework Composer which has 4 intents
    • TestSkill1: called using the text "Call Skill 1"
    • TestSkill2: called using the text "Call Skill 2"
    • Basic Intent: called using the text "Basic Intent"
    • CallBoth: called using the text "Call Both"
      • This calls both skills - This is where the issue exists

When using the intent "CallBoth" with the text "Call Both" the expected conversation output is:

  • "In Call Both"
  • "Response From Test Skill 1."
  • "Response From Test Skill 2."
  • "Finished Call Both"

Though the actual output is:

  • "In Call Both"
  • "Response From Test Skill 1."

For some reason the dialog/intent "CallBoth" is ending when the 1st skill is called.

After that if "Basic Intent" is sent then the response is:

  • "In Basic Intent" (which is expected)
  • "Response From Test Skill 2"
  • "Finished Call Both"

The 2nd skill & the ending "Finished Call Both" are then presented after "In Basic Intent".

It appears that the call to the 2nd skill is somehow getting stuck & only appearing after another action occurs.

To Reproduce

Using the repo MultiSkillComposerBot

Setup the projects:

Bot.Skills

This project needs to be running on local IIS.
There is a publish profile in the project which publishes the project to: C:\inetpub\wwwroot\Bot.MultiSkill.Skills

Open MultiSkillComposerBot.sln in Visual Studio

Right click on Bot.Skills & choose "Publish"

Publish using "FolderProfile.pubxml"

Open IIS

Create a new application pool called "Bot.MultiSkill.Skills" which is set to "No Managed Code"

Open Sites / Default Site & right click on "Bot.MultiSkill.Skills" & "Convert To Application" selecting the "Bot.MultiSkill.Skills" application pool.

This can be tested as working by going to be below 2 URLs in a browser:
http://localhost/bot.multiskill.skills/api/manifest/Test.Skill1
http://localhost/bot.multiskill.skills/api/manifest/Test.Skill2

You should see JSON text as the response similar to the below:
{"$schema":"https://schemas.botframework.com/schemas/skills/skill-manifest-2.1.preview-0.json","$id":"Test.Skill2","name":"Test.Skill2","description":"Responds with a message indicating Test Skill 2 was successfully called.","publisherName":"Test","version":"1.0","iconUrl":null,"tags":[],"endpoints":[{"name":"production","protocol":"BotFrameworkV3","description":"Production endpoint","endpointUrl":"http://localhost/Bot.MultiSkill.Skills/api/skill/Test.Skill2","msAppId":"00000000-0000-0000-0000-000000000000"}]}

ComposerMultiSkillDialog

This project was built using Bot Framework Composer

Open the project in Bot Framework Composer.
Click "Start Bot"

When that action is completed select your preference of "Open Web Chat" or "Test in Emulator"

Enter the text "Call Both"
See the response:

  • "In Call Both"
  • "Response From Test Skill 1."
    Enter Text "Basic Intent"
    See the response
  • "In Basic Intent" (which is expected)
  • "Response From Test Skill 2"
  • "Finished Call Both"

Expected behavior

Conversation flow when entering the text "Call Both" should be:

  • "In Call Both"
  • "Response From Test Skill 1."
  • "Response From Test Skill 2."
  • "Finished Call Both"

Screenshots

ComposerScreenshotShowingCallBothIntent
EmulatorScreenshotShowingCallBothIntentConversationFlow

Additional context

I am routing all of the skills through a single endpoint in SkillConsumerController.cs. I wonder if this is causing some kind of sharing of resources / context which is contributing to the issue. I am doing this because in the actual system there will be dozens of skills. Setting the code up in this way will require only 1 file to be added each time a new skill is created.

Tracking Status

Dotnet SDK TODO

  • PR
  • Merged

Javascript SDK TODO

  • PR
  • Merged

Python SDK TODO

  • PR
  • Merged

Java SDK TODO

  • PR
  • Merged

Samples TODO

  • PR
  • Merged

Docs TODO

  • PR
  • Merged

Tools TODO

  • PR
  • Merged
@silverbulletgt silverbulletgt added bug Indicates an unexpected problem or an unintended behavior. needs-triage The issue has just been created and it has not been reviewed by the team. labels Mar 30, 2021
@msomanathan msomanathan added customer-reported Issue is created by anyone that is not a collaborator in the repository. Bot Services Required for internal Azure reporting. Do not remove. Do not change color. labels Mar 31, 2021
@dmvtech
Copy link

dmvtech commented Apr 1, 2021

Hi @silverbulletgt I took a look at your repo/repro and am attempting to set it up. I'll let you know if I have any questions.

I did, quickly, setup something similar (approach wise) in Composer entirely (test skills running locally), and am getting somewhat similar results.

  • "In Call Both"
  • "Response From Test Skill 1."
  • "Finished Call Both

Instead of:

  • "In Call Both"
  • "Response From Test Skill 1."

But it never does respond from skill 2.

I'll update when I have more findings or questions.

@dmvtech
Copy link

dmvtech commented Apr 6, 2021

Hi @silverbulletgt Thank you for pointing this one out. As you can see there is a fix that has been merged. If you are able to build with that and then test, that would be great. I can do the same, but it may take me a while (you might be able to get to it sooner than I).

@silverbulletgt
Copy link
Author

Hi @dmvtech Thank you so much for addressing this so quickly. To be honest I'm not really sure how I would go about integrating a change from this repo onto my local machine for testing.

I know how to download the repo & build, then adjust my C# projects to point to the dlls which are created during that build. I don't know how I would setup the Bot Composer to use those libraries, then build it on my local machine to be able to start the bot & do testing. I'm not very familiar with npm & yarn. Would you mind redoing the test you performed with the composer & the 2 skills when you get a chance? Sorry I can't be of more assistance from this perspective.

Also, I identified this issue while working on a project which is planned for a production release in about 2 months (June 2021). Do you think there is any chance that this change would be deployed to the downloadable version of the Bot Composer within that time frame?

@pavolum
Copy link

pavolum commented Apr 20, 2021

@cleemullins do we have an ETA for the release that includes this bug?

@cleemullins
Copy link
Contributor

cleemullins commented Apr 20, 2021

This change was merged into our 4.13 branch, which is now live on Nuget & NPM. If you update to the latest packages, you'll have this fix.

(Edit): Given that you're using Composer, you should be able to test this using the Nightly Composer feed. The next version of Composer will be releasing in the next month or so, which will pickup these fixes.

@dmvtech
Copy link

dmvtech commented Apr 20, 2021

@silverbulletgt I just tested with Composer and it's not quite working for me. But that might be that I don't have everything set up correctly. I'm still working on it and will let you know.

But as said above; the fix is in 4.13. If you can, please test it in your repro and let us know if how it behaves for you.

@silverbulletgt
Copy link
Author

Hi @dmvtech, thanks for taking a look. I still see the same issue when using the composer. I tried this using the nightly build v1.4.0-nightly.237101.f01cb52 version as well.

In the Bot Composer GitHub project there is a runtime folder which has several C# projects Composer dotnet runtime. These projects currently reference version 4.12.2 of the BotFramework SDK. These are the projects which are used to create the Web App which is started when you use "Start Bot" in the Bot Composer.

You can add a "Custom Runtime" by going to the "Project Settings" in the Bot Composer & turning on the "Custom Runtime" option. This outputs the C# projects to the runtime folder. I did this & then upgraded the various Bot Framework SDK versions to 4.13.0.

After doing this there were a few ambiguous references. 1 specifically was on "SkillConversationIdFactory". I tried running the project by resolving to both the Core.SkillConversationIdFactory version & the Bot.Builder.Skills.SkillConversationIdFactory version.

From there I started the bot from the Bot Composer. I checked to ensure that the runtime did not change after I performed this action. I also tried debugging the Microsoft.BotFramework.Composer.WebApp in Visual Studio directly to ensure that the correct code was in use & then connecting to the emulator manually.

None of these attempts had any impact on the issue. The 2nd skill still did not trigger correctly & would trigger after other actions.

I have commited those changes to my repo here: MultiSkillComposerBot repo

These are my latest commits which use the "Custom Runtime" project & then modify to use the SDK & resolve the ambiguous references in the 2 different ways.
MultiSkillComposerBot-Commits-Upgrade-To-4 13 0 png

I also tried upgrading the Bot.Skills project to 4.13.0 & that also did not have an impact.

At this point I would say that the issue is not yet resolved.

@silverbulletgt
Copy link
Author

Hi there, just wanted to follow up about this issue. Is this something which is still planned to be addressed? Thanks!

@dmvtech
Copy link

dmvtech commented May 7, 2021

Hi @silverbulletgt

So, one of my issues, it was that I was not passing an activity when calling the skills from the root bot (I was calling the skills individually with an recognizer that would hit that trigger, but from the root bot's "call both", that wasn't being done).

I'm not able to get your skills running (not able to run local IIS), but poking around, it looks like your bot just sends the message activity "Response From Test Skill 1." (and similar for skill 2). You do not have any recognizers, etc in your skills, correct?

Do you have steps to get your solution running with iisexpress or Kestrel/.NETCore so that I can get it running for myself and further troubleshoot?

The other thing, is you might want to try with version 4.13.1, which is just a tad newer. Just in case.

@silverbulletgt
Copy link
Author

silverbulletgt commented May 7, 2021

Hi @dmvtech, thank you for taking look at this issue.

No, I don't have any recognizers in my skills. They just send a message response then call stepContext.EndDialogAsync(...).

I have updated the MultiSkillComposerBot repo to be able to run through IISExpress. I have put instructions in the readme.md file on how to use this.

Open the solution file: MultiSkillComposerBot.sln
Ensure that Bot.Skills is set as the startup project
Push F5
Check that the manifest files are running correctly using the below 2 links:
Test Skill 1
Test Skill 2

With that still running open the Bot Composer to the folder "ComposerMultiSkillDialog".
When that opens click "Start Bot"
Run using either "Open Web Chat" or "Test in Emulator"
Then when you utter "Call Both" you will see that only "Test Skill 1" is called.
Utter something else & you will see that "Test Skill 2" is then called.

While making this change I used the "Custom Runtime" option within the Bot Composer & have upgraded the 2 projects which are output to version 4.13.1 of the Microsoft.Bot SDK:

  • ComposerMultiSkillDialog\runtime\azurewebapp\Microsoft.BotFramework.Composer.WebApp.csproj
  • ComposerMultiSkillDialog\runtime\core\Microsoft.BotFramework.Composer.Core.csproj

I also upgraded the Bot.Skills project to version 4.13.1 of the Microsoft.Bot SDK

Unfortunately, none of these changes had any impact on the behavior.

@silverbulletgt
Copy link
Author

@dmvtech Hi, any update on this issue?

@silverbulletgt
Copy link
Author

I have been doing some debugging on this issue & this is what I seem to have found so far.

After the first skill is called the skill returns an "End Of Conversation" reply.
This happens in DialogExtensions.cs - InnerRunAsync on line 141.

            await SendStateSnapshotTraceAsync(dialogContext, cancellationToken).ConfigureAwait(false);

            // Skills should send EoC when the dialog completes.
            if (result.Status == DialogTurnStatus.Complete || result.Status == DialogTurnStatus.Cancelled)
            {
                if (SendEoCToParent(turnContext))
                {
                    // Send End of conversation at the end.
                    var code = result.Status == DialogTurnStatus.Complete ? EndOfConversationCodes.CompletedSuccessfully : EndOfConversationCodes.UserCancelled;
                    var activity = new Activity(ActivityTypes.EndOfConversation) { Value = result.Result, Locale = turnContext.Activity.Locale, Code = code };
                    await turnContext.SendActivityAsync(activity, cancellationToken).ConfigureAwait(false);
                }
            }

            return result;

Then when the 2nd skill is called it seems like the response from the 1st skill is passed in as the activity. In this case that is "EndOfConversation". This triggers logic on the skill side within the same method DialogExtensions.cs - InnerRunAsync on line 101.

            // Handle EoC and Reprompt event from a parent bot (can be root bot to skill or skill to skill)
            if (IsFromParentToSkill(turnContext))
            {
                // Handle remote cancellation request from parent.
                if (turnContext.Activity.Type == ActivityTypes.EndOfConversation)
                {
                    if (!dialogContext.Stack.Any())
                    {
                        // No dialogs to cancel, just return.
                        return new DialogTurnResult(DialogTurnStatus.Empty);
                    }

                    var activeDialogContext = GetActiveDialogContext(dialogContext);

                    // Send cancellation message to the top dialog in the stack to ensure all the parents are canceled in the right order. 
                    return await activeDialogContext.CancelAllDialogsAsync(true, cancellationToken: cancellationToken).ConfigureAwait(false);
                }

This stops the skill from continuing & seems to cancel all remaining dialogs on the composer side of things. Though the dialogs remain on the stack so when something else is uttered by the user the skill continues & passes to actually run since the activity is no longer "EndOfConversation".

@silverbulletgt
Copy link
Author

I have been able to prevent this behavior by adding something into the "Activity" field within the Bot Composer for the skill. This is not ideal since I will need to do this for every skill which does not have some other requirement to pass in a specific activity value. I'm investigating how I can possibly use middleware to modify the activity from "EndOfConversation" to something else. I worry this could impact skills which have a waterfall structure (multiple steps) though.

Preventing Issue By Adding Activity To Skill In Composer

silverbulletgt pushed a commit to silverbulletgt/MultiSkillComposerBot that referenced this issue May 19, 2021
@silverbulletgt
Copy link
Author

silverbulletgt commented May 19, 2021

I was able to build a structure which addresses this issue. Though this solution is not ideal & may not be possible for everyone. It requires that the Bot Composer be setup to use a "Custom Runtime", then the "Custom Runtime" must be modified manually. In the event that the "Custom Runtime" is regenerated these steps would need to be done again.

There could be consequences to preventing a skill from accepting an "EndOfConversation" activity but I'll need to deal with that separately if I find it becomes a problem.

Workaround

It is a combination of Commit to Provide Custom DeclarativeType implementation
And Commit to detect when "EndOfConversation" is passed as the activity of a skill & prevent that from stopping the skill call

Instructions to get the basic structure setup are in the answer to this stackoverflow post

Then to address this issue I had to:
Create a new class "BeginSkillCustom" which is a copy of BeginSkill.cs
Modify the BeginDialogAsync method to have the below body which allows for a custom activity to be passed in.

public override async Task<DialogTurnResult> BeginDialogAsync(DialogContext dc, object options = null, CancellationToken cancellationToken = default)
        {
            var dialogArgs = ValidateBeginDialogArgs(options);

            // Create deep clone of the original activity to avoid altering it before forwarding it.
            var skillActivity = ObjectPath.Clone(dialogArgs.Activity);

            if (Disabled != null && Disabled.GetValue(dc.State))
            {
                return await dc.EndDialogAsync(cancellationToken: cancellationToken).ConfigureAwait(false);
            }

            // Update the dialog options with the runtime settings.
            DialogOptions.BotId = BotId.GetValue(dc.State);
            DialogOptions.SkillHostEndpoint = new Uri(SkillHostEndpoint.GetValue(dc.State));
            DialogOptions.ConversationIdFactory = dc.Context.TurnState.Get<SkillConversationIdFactoryBase>() ?? throw new NullReferenceException("Unable to locate SkillConversationIdFactoryBase in HostContext");
            DialogOptions.SkillClient = dc.Context.TurnState.Get<BotFrameworkClient>() ?? throw new NullReferenceException("Unable to locate BotFrameworkClient in HostContext");
            DialogOptions.ConversationState = dc.Context.TurnState.Get<ConversationState>() ?? throw new NullReferenceException($"Unable to get an instance of {nameof(ConversationState)} from TurnState.");
            DialogOptions.ConnectionName = ConnectionName.GetValue(dc.State);

            // Set the skill to call
            DialogOptions.Skill.Id = DialogOptions.Skill.AppId = SkillAppId.GetValue(dc.State);
            DialogOptions.Skill.SkillEndpoint = new Uri(SkillEndpoint.GetValue(dc.State));

            // Store the initialized DialogOptions in state so we can restore these values when the dialog is resumed.
            dc.ActiveDialog.State[_dialogOptionsStateKey] = DialogOptions;

            // Get the activity to send to the skill.
            Activity activity = null;
            if (Activity != null && ActivityProcessed.GetValue(dc.State))
            {
                // The parent consumed the activity in context, use the Activity property to start the skill.
                activity = await Activity.BindAsync(dc, cancellationToken: cancellationToken).ConfigureAwait(false);
            }
            else if (skillActivity != null)
            {
                activity = skillActivity;
            }

            // Call the base to invoke the skill
            // (If the activity has not been processed, send the turn context activity to the skill (pass through)). 
            return await base.BeginDialogAsync(dc, new BeginSkillDialogOptions { Activity = activity ?? dc.Context.Activity }, cancellationToken).ConfigureAwait(false);
        }

Then add an override to this method in BeginSkillNonRePrompting.cs which detects if "EndOfConversation" is passed in & replaces it with a blank "Event" activity to prevent the skill from being cancelled internally.

/// <summary>
        /// This method is overridden to deal with an issue where if 2 skills are called back to back then the
        /// "EndOfConversation" activity which gets returned from the 1st skill is passed to the 2nd skill.
        /// This prevents the 2nd skill from running successfully because there is logic on the skill side which will
        /// CancelAllDialogs when an EndOfConversation activitiy is passed
        /// 
        /// In this overridden version we will detect if an "EndOfConversation" is passed in & then pass in a blank activity.
        /// I'm not sure if this will cause any other issues. I will need to watch out for this as we continue to build more complex dialogs.
        /// </summary>
        /// <param name="dc">The <see cref="DialogContext"/> for the current turn of conversation.</param>
        /// <param name="options">Optional, initial information to pass to the dialog.</param>
        /// <param name="cancellationToken">A cancellation token that can be used by other objects
        /// or threads to receive notice of cancellation.</param>
        /// <returns>A <see cref="Task"/> representing the asynchronous operation.</returns>
        public override async Task<DialogTurnResult> BeginDialogAsync(DialogContext dc, object options = null, CancellationToken cancellationToken = default)
        {
            if (Disabled != null && Disabled.GetValue(dc.State))
            {
                return await dc.EndDialogAsync(cancellationToken: cancellationToken).ConfigureAwait(false);
            }

            // Update the dialog options with the runtime settings.
            DialogOptions.BotId = BotId.GetValue(dc.State);
            DialogOptions.SkillHostEndpoint = new Uri(SkillHostEndpoint.GetValue(dc.State));
            DialogOptions.ConversationIdFactory = dc.Context.TurnState.Get<SkillConversationIdFactoryBase>() ?? throw new NullReferenceException("Unable to locate SkillConversationIdFactoryBase in HostContext");
            DialogOptions.SkillClient = dc.Context.TurnState.Get<BotFrameworkClient>() ?? throw new NullReferenceException("Unable to locate BotFrameworkClient in HostContext");
            DialogOptions.ConversationState = dc.Context.TurnState.Get<ConversationState>() ?? throw new NullReferenceException($"Unable to get an instance of {nameof(ConversationState)} from TurnState.");
            DialogOptions.ConnectionName = ConnectionName.GetValue(dc.State);

            // Set the skill to call
            DialogOptions.Skill.Id = DialogOptions.Skill.AppId = SkillAppId.GetValue(dc.State);
            DialogOptions.Skill.SkillEndpoint = new Uri(SkillEndpoint.GetValue(dc.State));

            // Store the initialized DialogOptions in state so we can restore these values when the dialog is resumed.
            dc.ActiveDialog.State[_dialogOptionsStateKey] = DialogOptions;

            // Get the activity to send to the skill.
            Activity activity = null;
            if (Activity != null && ActivityProcessed.GetValue(dc.State))
            {
                // The parent consumed the activity in context, use the Activity property to start the skill.
                activity = await Activity.BindAsync(dc, cancellationToken: cancellationToken).ConfigureAwait(false);
            }
            else if (dc.Context.Activity.Type.Equals(ActivityTypes.EndOfConversation))
            {
                //if an EndOfConversation activity is passed then we are assuming that this was becasue another skill
                //was called immediately before this one & the EndOfConversation from that has been passed to this skill
                //we need to prevent this issue otherwise this skill will not be called
                activity = new Activity() { Type = ActivityTypes.Event };
            }

            // Call the base to invoke the skill
            // (If the activity has not been processed, send the turn context activity to the skill (pass through)). 
            return await base.BeginDialogAsync(dc, new BeginSkillDialogOptions { Activity = activity ?? dc.Context.Activity }, cancellationToken).ConfigureAwait(false);
        }

To Turn The Issue Back On

To see the issue again within the original repo see the comment in Startup.cs within the Microsoft.BotFramework.Composer.WebApp project.

//to reproduce the below 2 issues uncomment this line & comment the line with AdaptiveComponentRegistrationCustom
            //The 2 issues are:
            //https://github.com/microsoft/botframework-sdk/issues/6248
            //When 2 skills are called back to back the "EndOfConversation" activity returned after the 1st skill completes gets passed to the 2nd skill & prevents the 2nd skill from completing
            //https://stackoverflow.com/questions/67541109/bot-framework-how-do-i-stop-a-waterfall-dialog-which-is-waiting-for-a-prompt-re/67576904#67576904
            //There is no way to prevent a skill which has a waterfall prompt from reprompting if it is interrupted
            //ComponentRegistration.Add(new AdaptiveComponentRegistration());
            ComponentRegistration.Add(new AdaptiveComponentRegistrationCustom());

@silverbulletgt
Copy link
Author

Hi @dmvtech, would you mind reviewing my last few comments which outline why the issue seems to be occurring & the steps which I have taken to address it within my project. I don't think that the way I have addressed it will be ideal / possible for anyone else. I'm hoping that you & your team can think of a more elegant way to address this & build it into the framework so that I can remove my workaround in a coming release.

Thanks for your help!

@dmvtech
Copy link

dmvtech commented May 19, 2021

Hi @silverbulletgt Sorry for the delay.

I just cloned repo of your repository, fired up the skills in VS/iisexpress and the bot in composer 1.4.1 and found it was working for me:

image

That was done before seeing your last large comment above and noticing the changes you had made 😅. Regarding the need for an activity being sent, that what I was mentioning here.

Thank you for all the comments and research you have done. I will be involving more eyes on this and will update you as soon as possible. Thank you.

@cleemullins cleemullins added this to the R14 milestone Jun 8, 2021
@mrivera-ms mrivera-ms added the P1 Painful if we don't fix, won't block releasing label Jun 9, 2021
@dmvtech dmvtech removed their assignment Jun 15, 2021
@msomanathan msomanathan modified the milestones: R14, R15 Jun 17, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Bot Services Required for internal Azure reporting. Do not remove. Do not change color. bug Indicates an unexpected problem or an unintended behavior. customer-replied-to Indicates that the team has replied to the issue reported by the customer. Do not delete. customer-reported Issue is created by anyone that is not a collaborator in the repository. P1 Painful if we don't fix, won't block releasing
Projects
None yet
Development

No branches or pull requests

7 participants