Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

BeginDialog failed with dialog id not found #3913

Closed
DingmaomaoBJTU opened this issue May 13, 2020 · 6 comments
Closed

BeginDialog failed with dialog id not found #3913

DingmaomaoBJTU opened this issue May 13, 2020 · 6 comments
Assignees
Labels
bug Indicates an unexpected problem or an unintended behavior. investigate Needs more information in order to proceed P1 Painful if we don't fix, won't block releasing R9 Release 9 - May 15th, 2020
Milestone

Comments

@DingmaomaoBJTU
Copy link
Contributor

DingmaomaoBJTU commented May 13, 2020

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

Version

What package version of the SDK are you using.
4.9.0-rc4

Describe the bug

BeginDialog failed with dialog id not found for special flow structure (call one dialog multi time). This issue is very easy to repro when build big project like calendar/todo/who skills.
image

When loading dialog set, after key value pair (A2, A2) has been added, a new pair (A22, A22) is processed. But A22 and A2 are somehow pointed to the same ref so the dialog id will update from A2 to A22. So the key value pair(A2, A2) turned out to be (A2, A22) for this special parent dialog. It is not valid anymore. When begin A22, the error A22 not found will show because the dialog set only have value for A2.

After investigation, this bug is caused by this commit (Adaptive: Detect and load graph cycles during type loading (#3857) )9dd230f. Things work well after revert this change.

To Reproduce

Steps to reproduce the behavior:

  1. Put the source code under Microsoft.Bot.Builder.TestBot.Json
  2. input "c"
  3. See error : sub0222 not found.
    Bug.zip

Expected behavior

Dialog should be invoke successfully.

Screenshots

If applicable, add screenshots to help explain your problem.

Additional context

Add any other context about the problem here.

[bug]

@DingmaomaoBJTU
Copy link
Contributor Author

This is quite blocking for calendar/todo/who skills. Could you please help and investigate?
And thanks @xieofxie and @luhan2017 's great help for investigation~

@boydc2014
Copy link
Contributor

@tomlm @vishwacsena looks like a "P0" to me...

@carlosscastro
Copy link
Member

Investigating!

@sgellock sgellock added bug Indicates an unexpected problem or an unintended behavior. investigate Needs more information in order to proceed P1 Painful if we don't fix, won't block releasing R9 Release 9 - May 15th, 2020 labels May 13, 2020
@carlosscastro
Copy link
Member

carlosscastro commented May 13, 2020

Thanks @boydc2014 @xieofxie @DingmaomaoBJTU and @luhan2017 for the investigation and the example, that really accelerated the fix, you guys ROCK!

Can we work together to add tests to the adaptive project that cover all of composer templates? That would allow us to verify changes to the SDK early and not even allow things that break composer to be merged to the master branch of the SDK. Thoughts? + @cwhitten

@carlosscastro
Copy link
Member

Pending sign off from composer team but fix got merged to 4.9 branch. We'll generate a 4.9.1 release candidate build and publish to myget, in order to be verified tonight, before releasing the actual 4.9.1 build to nuget tomorrow.

@DingmaomaoBJTU
Copy link
Contributor Author

Thank you for your quick response! @carlosscastro

@munozemilio munozemilio added this to the R9 milestone Jul 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or an unintended behavior. investigate Needs more information in order to proceed P1 Painful if we don't fix, won't block releasing R9 Release 9 - May 15th, 2020
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants