{"action":"created","issue":{"url":"https://api.github.com/repos/Microsoft/BotBuilder/issues/4169","repository_url":"https://api.github.com/repos/Microsoft/BotBuilder","labels_url":"https://api.github.com/repos/Microsoft/BotBuilder/issues/4169/labels{/name}","comments_url":"https://api.github.com/repos/Microsoft/BotBuilder/issues/4169/comments","events_url":"https://api.github.com/repos/Microsoft/BotBuilder/issues/4169/events","html_url":"https://github.com/Microsoft/BotBuilder/issues/4169","id":295244739,"number":4169,"title":"endDialog() incorrectly resumes a 'parent' dialog","user":{"login":"SynerG","id":7237097,"avatar_url":"https://avatars1.githubusercontent.com/u/7237097?v=4","gravatar_id":"","url":"https://api.github.com/users/SynerG","html_url":"https://github.com/SynerG","followers_url":"https://api.github.com/users/SynerG/followers","following_url":"https://api.github.com/users/SynerG/following{/other_user}","gists_url":"https://api.github.com/users/SynerG/gists{/gist_id}","starred_url":"https://api.github.com/users/SynerG/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/SynerG/subscriptions","organizations_url":"https://api.github.com/users/SynerG/orgs","repos_url":"https://api.github.com/users/SynerG/repos","events_url":"https://api.github.com/users/SynerG/events{/privacy}","received_events_url":"https://api.github.com/users/SynerG/received_events","type":"User","site_admin":false},"labels":[{"id":429202607,"url":"https://api.github.com/repos/Microsoft/BotBuilder/labels/Node.js%20SDK","name":"Node.js SDK","color":"6eff54","default":false},{"id":622541392,"url":"https://api.github.com/repos/Microsoft/BotBuilder/labels/dialogs","name":"dialogs","color":"fbca04","default":false}],"state":"open","locked":false,"assignee":{"login":"FranciscoPonceGomez","id":16775093,"avatar_url":"https://avatars0.githubusercontent.com/u/16775093?v=4","gravatar_id":"","url":"https://api.github.com/users/FranciscoPonceGomez","html_url":"https://github.com/FranciscoPonceGomez","followers_url":"https://api.github.com/users/FranciscoPonceGomez/followers","following_url":"https://api.github.com/users/FranciscoPonceGomez/following{/other_user}","gists_url":"https://api.github.com/users/FranciscoPonceGomez/gists{/gist_id}","starred_url":"https://api.github.com/users/FranciscoPonceGomez/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/FranciscoPonceGomez/subscriptions","organizations_url":"https://api.github.com/users/FranciscoPonceGomez/orgs","repos_url":"https://api.github.com/users/FranciscoPonceGomez/repos","events_url":"https://api.github.com/users/FranciscoPonceGomez/events{/privacy}","received_events_url":"https://api.github.com/users/FranciscoPonceGomez/received_events","type":"User","site_admin":false},"assignees":[{"login":"FranciscoPonceGomez","id":16775093,"avatar_url":"https://avatars0.githubusercontent.com/u/16775093?v=4","gravatar_id":"","url":"https://api.github.com/users/FranciscoPonceGomez","html_url":"https://github.com/FranciscoPonceGomez","followers_url":"https://api.github.com/users/FranciscoPonceGomez/followers","following_url":"https://api.github.com/users/FranciscoPonceGomez/following{/other_user}","gists_url":"https://api.github.com/users/FranciscoPonceGomez/gists{/gist_id}","starred_url":"https://api.github.com/users/FranciscoPonceGomez/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/FranciscoPonceGomez/subscriptions","organizations_url":"https://api.github.com/users/FranciscoPonceGomez/orgs","repos_url":"https://api.github.com/users/FranciscoPonceGomez/repos","events_url":"https://api.github.com/users/FranciscoPonceGomez/events{/privacy}","received_events_url":"https://api.github.com/users/FranciscoPonceGomez/received_events","type":"User","site_admin":false}],"milestone":null,"comments":1,"created_at":"2018-02-07T18:29:24Z","updated_at":"2018-02-10T09:16:08Z","closed_at":null,"author_association":"CONTRIBUTOR","body":"## Bot Info\r\n* SDK Platform: Node.js\r\n* SDK Version: 3.12.0\r\n\r\n## Issue Description\r\n\r\nEnding a dialog sometimes has unexpected side effects that corrupts the dialog stack and leaves it in an unexpected state.\r\n\r\nThe issue is caused by the assumptions made by the `session.endDialog()/.endDialogWithResult()` methods. When called, these methods assume that the next dialog in the stack is its parent and should move forward to the next step or be removed from the stack (if there are no more steps). But sometimes this assumption is not correct.\r\n\r\nFor example, let's say we a dialog stack that contains a dialog `A`:\r\n```\r\ncallstack = [A]\r\n```\r\nThen, an event happens that adds dialog `B` to the stack:\r\n```\r\ncallstack = [A,B]\r\n```\r\nThe event (e.g. a timer, a external action, or even a command in the conversation) is unrelated to A, so **A is NOT the parent of B**.\r\n\r\n## Expected Behavior\r\nWhen `B` executes `B.endDialog()`, `B` should be removed from the stack, but `A` should be left untouched (as it is unrelated to `B`).\r\n\r\n## Actual Results\r\nWhen `B` executes `B.endDialog()`, the current implementation removes `B` , and CAUSES `A` to move forward to its next step. \r\n\r\nMoreover, if `A` does not have more steps, `A` is also removed from the stack and the *'move forward or delete'* effect cascades to the next dialog in the call stack.\r\n\r\nFor example, in a callstack where `A` launches a single-step `A_child` that proposes some buttons to the user and will communicate the button pressed to its parent `A`. If an event causes to launch a `B` dialog for a notification, we get this dialog stack:\r\n```\r\ncallstack = [A, A_child, B]\r\n```\r\nWhen `B` executes `B.endDialog()`, the single-step `A_child` dialog is directly removed (although the user has no pressed any button yet), and `A` is resumed at its next step without the result expected from `A_child` button choice.\r\n\r\n## Desired Solution\r\n`BotBuilder SDK` should keep track of why the dialog was added to the stack. If the dialog is not a child of the next dialog in the stack, ending the dialog should not have any side effects in the next dialog (no moving forward or removing it).\r\n\r\n## Current Workaround\r\nI have seen in the implementation of `WaterfallDialog` that when a dialog is resumed with `ResumeReason.reprompt` the dialog does not move forward, neither executes a step.\r\n\r\nSo, as a hacky solution, in dialogs that are only added to the stack due to external events, I am ending them with `session.endDialogWithResult({resumed: ResumeReason.reprompt})` to prevent dialog stack corruption.\r\nHowever, it is not really a `reprompt` and this is an internal implementation detail that could change in the future with unexpected consequences.\r\n\r\n`Botbuilder SDK` should directly keep track of why the dialog was added to the stack, and not resume the 'parent' if it is not really its parent.\r\n\r\n\r\n\r\n\r\n\r\n\r\n"},"comment":{"url":"https://api.github.com/repos/Microsoft/BotBuilder/issues/comments/364638073","html_url":"https://github.com/Microsoft/BotBuilder/issues/4169#issuecomment-364638073","issue_url":"https://api.github.com/repos/Microsoft/BotBuilder/issues/4169","id":364638073,"user":{"login":"SynerG","id":7237097,"avatar_url":"https://avatars1.githubusercontent.com/u/7237097?v=4","gravatar_id":"","url":"https://api.github.com/users/SynerG","html_url":"https://github.com/SynerG","followers_url":"https://api.github.com/users/SynerG/followers","following_url":"https://api.github.com/users/SynerG/following{/other_user}","gists_url":"https://api.github.com/users/SynerG/gists{/gist_id}","starred_url":"https://api.github.com/users/SynerG/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/SynerG/subscriptions","organizations_url":"https://api.github.com/users/SynerG/orgs","repos_url":"https://api.github.com/users/SynerG/repos","events_url":"https://api.github.com/users/SynerG/events{/privacy}","received_events_url":"https://api.github.com/users/SynerG/received_events","type":"User","site_admin":false},"created_at":"2018-02-10T09:16:08Z","updated_at":"2018-02-10T09:16:08Z","author_association":"CONTRIBUTOR","body":"Ok, I have found an \"official\" approach to begin a dialog without a parent.\r\n\r\n`BotBuilder SDK` internally implements an `Interruption`dialog (`consts.DialogId.Interruption`).\r\nThis special dialog is added to the callstack to act as a wrapper of the real dialog that should begin. When the dialog ends, `Interruption` uses exactly the `ResumeReason.reprompt` tip that I discussed previously to prevent activating the next dialog in the stack (as it is not a parent).\r\n\r\nHowever, you cannot manually wrap the dialog in an `Interruption` directly because the internal `consts.DialogId` is not re-exported. So you cannot use it in your own `customAction()` for example.\r\n\r\nAFAIK, the `Interruption` wrapper dialog is only used by the method `beginDialogAction()`.\r\n\r\nSo, to add a dialog to the stack that is not a child of a previous dialog, it should be started using  `beginDialogAction()`. \r\n\r\nIf you start the dialog with `beginDialog()`, there is no parameter you can specify to indicate that it is an interruption and `BotBuilder` does not control where/how the dialog was added, so it is always considered a child of the previous dialog in the stack.\r\n\r\nAs the `beginDialogAction()` method exists, this issue can be closed. Although it would be great if  `BotBuilder` automatically managed the dialog hierarchy (beyond a stack) or there was a parameter for `beginDialog()` to indicate that the dialog added to the stack is not a child.\r\n\r\n"}}
{"actor":{"display_login":"SynerG"}}