From 92756f2c3f6bd405ede11ac04acb8055e104c2f8 Mon Sep 17 00:00:00 2001 From: Joel Mut Date: Wed, 11 Nov 2020 18:06:59 -0300 Subject: [PATCH 1/4] [PORT] Emit better error messages for all dialogs --- .../botbuilder/dialogs/dialog_context.py | 214 +++++++++++------- .../tests/test_activity_prompt.py | 82 +++++++ 2 files changed, 216 insertions(+), 80 deletions(-) diff --git a/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_context.py b/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_context.py index b081cdea5..f79ef8e3c 100644 --- a/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_context.py +++ b/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_context.py @@ -69,26 +69,30 @@ async def begin_dialog(self, dialog_id: str, options: object = None): :param dialog_id: ID of the dialog to start :param options: (Optional) additional argument(s) to pass to the dialog being started. """ - if not dialog_id: - raise TypeError("Dialog(): dialogId cannot be None.") - # Look up dialog - dialog = await self.find_dialog(dialog_id) - if dialog is None: - raise Exception( - "'DialogContext.begin_dialog(): A dialog with an id of '%s' wasn't found." - " The dialog must be included in the current or parent DialogSet." - " For example, if subclassing a ComponentDialog you can call add_dialog() within your constructor." - % dialog_id - ) - # Push new instance onto stack - instance = DialogInstance() - instance.id = dialog_id - instance.state = {} - - self._stack.insert(0, (instance)) - - # Call dialog's begin_dialog() method - return await dialog.begin_dialog(self, options) + try: + if not dialog_id: + raise TypeError("Dialog(): dialogId cannot be None.") + # Look up dialog + dialog = await self.find_dialog(dialog_id) + if dialog is None: + raise Exception( + "'DialogContext.begin_dialog(): A dialog with an id of '%s' wasn't found." + " The dialog must be included in the current or parent DialogSet." + " For example, if subclassing a ComponentDialog you can call add_dialog() within your constructor." + % dialog_id + ) + # Push new instance onto stack + instance = DialogInstance() + instance.id = dialog_id + instance.state = {} + + self._stack.insert(0, (instance)) + + # Call dialog's begin_dialog() method + return await dialog.begin_dialog(self, options) + except Exception as err: + self.__set_exception_context_data(err) + raise # TODO: Fix options: PromptOptions instead of object async def prompt(self, dialog_id: str, options) -> DialogTurnResult: @@ -99,13 +103,17 @@ async def prompt(self, dialog_id: str, options) -> DialogTurnResult: :param options: Contains a Prompt, potentially a RetryPrompt and if using ChoicePrompt, Choices. :return: """ - if not dialog_id: - raise TypeError("DialogContext.prompt(): dialogId cannot be None.") + try: + if not dialog_id: + raise TypeError("DialogContext.prompt(): dialogId cannot be None.") - if not options: - raise TypeError("DialogContext.prompt(): options cannot be None.") + if not options: + raise TypeError("DialogContext.prompt(): options cannot be None.") - return await self.begin_dialog(dialog_id, options) + return await self.begin_dialog(dialog_id, options) + except Exception as err: + self.__set_exception_context_data(err) + raise async def continue_dialog(self): """ @@ -114,20 +122,25 @@ async def continue_dialog(self): to determine if a dialog was run and a reply was sent to the user. :return: """ - # Check for a dialog on the stack - if self.active_dialog is not None: - # Look up dialog - dialog = await self.find_dialog(self.active_dialog.id) - if not dialog: - raise Exception( - "DialogContext.continue_dialog(): Can't continue dialog. A dialog with an id of '%s' wasn't found." - % self.active_dialog.id - ) - - # Continue execution of dialog - return await dialog.continue_dialog(self) - - return DialogTurnResult(DialogTurnStatus.Empty) + try: + # Check for a dialog on the stack + if self.active_dialog is not None: + # Look up dialog + dialog = await self.find_dialog(self.active_dialog.id) + if not dialog: + raise Exception( + "DialogContext.continue_dialog(): Can't continue dialog. " + "A dialog with an id of '%s' wasn't found." + % self.active_dialog.id + ) + + # Continue execution of dialog + return await dialog.continue_dialog(self) + + return DialogTurnResult(DialogTurnStatus.Empty) + except Exception as err: + self.__set_exception_context_data(err) + raise # TODO: instance is DialogInstance async def end_dialog(self, result: object = None): @@ -142,22 +155,27 @@ async def end_dialog(self, result: object = None): :param result: (Optional) result to pass to the parent dialogs. :return: """ - await self.end_active_dialog(DialogReason.EndCalled) - - # Resume previous dialog - if self.active_dialog is not None: - # Look up dialog - dialog = await self.find_dialog(self.active_dialog.id) - if not dialog: - raise Exception( - "DialogContext.EndDialogAsync(): Can't resume previous dialog." - " A dialog with an id of '%s' wasn't found." % self.active_dialog.id - ) - - # Return result to previous dialog - return await dialog.resume_dialog(self, DialogReason.EndCalled, result) - - return DialogTurnResult(DialogTurnStatus.Complete, result) + try: + await self.end_active_dialog(DialogReason.EndCalled) + + # Resume previous dialog + if self.active_dialog is not None: + # Look up dialog + dialog = await self.find_dialog(self.active_dialog.id) + if not dialog: + raise Exception( + "DialogContext.EndDialogAsync(): Can't resume previous dialog." + " A dialog with an id of '%s' wasn't found." + % self.active_dialog.id + ) + + # Return result to previous dialog + return await dialog.resume_dialog(self, DialogReason.EndCalled, result) + + return DialogTurnResult(DialogTurnStatus.Complete, result) + except Exception as err: + self.__set_exception_context_data(err) + raise async def cancel_all_dialogs(self): """ @@ -165,12 +183,16 @@ async def cancel_all_dialogs(self): :param result: (Optional) result to pass to the parent dialogs. :return: """ - if self.stack: - while self.stack: - await self.end_active_dialog(DialogReason.CancelCalled) - return DialogTurnResult(DialogTurnStatus.Cancelled) + try: + if self.stack: + while self.stack: + await self.end_active_dialog(DialogReason.CancelCalled) + return DialogTurnResult(DialogTurnStatus.Cancelled) - return DialogTurnResult(DialogTurnStatus.Empty) + return DialogTurnResult(DialogTurnStatus.Empty) + except Exception as err: + self.__set_exception_context_data(err) + raise async def find_dialog(self, dialog_id: str) -> Dialog: """ @@ -179,11 +201,15 @@ async def find_dialog(self, dialog_id: str) -> Dialog: :param dialog_id: ID of the dialog to search for. :return: """ - dialog = await self.dialogs.find(dialog_id) + try: + dialog = await self.dialogs.find(dialog_id) - if dialog is None and self.parent is not None: - dialog = await self.parent.find_dialog(dialog_id) - return dialog + if dialog is None and self.parent is not None: + dialog = await self.parent.find_dialog(dialog_id) + return dialog + except Exception as err: + self.__set_exception_context_data(err) + raise async def replace_dialog( self, dialog_id: str, options: object = None @@ -195,29 +221,37 @@ async def replace_dialog( :param options: (Optional) additional argument(s) to pass to the new dialog. :return: """ - # End the current dialog and giving the reason. - await self.end_active_dialog(DialogReason.ReplaceCalled) + try: + # End the current dialog and giving the reason. + await self.end_active_dialog(DialogReason.ReplaceCalled) - # Start replacement dialog - return await self.begin_dialog(dialog_id, options) + # Start replacement dialog + return await self.begin_dialog(dialog_id, options) + except Exception as err: + self.__set_exception_context_data(err) + raise async def reprompt_dialog(self): """ Calls reprompt on the currently active dialog, if there is one. Used with Prompts that have a reprompt behavior. :return: """ - # Check for a dialog on the stack - if self.active_dialog is not None: - # Look up dialog - dialog = await self.find_dialog(self.active_dialog.id) - if not dialog: - raise Exception( - "DialogSet.reprompt_dialog(): Can't find A dialog with an id of '%s'." - % self.active_dialog.id - ) - - # Ask dialog to re-prompt if supported - await dialog.reprompt_dialog(self.context, self.active_dialog) + try: + # Check for a dialog on the stack + if self.active_dialog is not None: + # Look up dialog + dialog = await self.find_dialog(self.active_dialog.id) + if not dialog: + raise Exception( + "DialogSet.reprompt_dialog(): Can't find A dialog with an id of '%s'." + % self.active_dialog.id + ) + + # Ask dialog to re-prompt if supported + await dialog.reprompt_dialog(self.context, self.active_dialog) + except Exception as err: + self.__set_exception_context_data(err) + raise async def end_active_dialog(self, reason: DialogReason): instance = self.active_dialog @@ -230,3 +264,23 @@ async def end_active_dialog(self, reason: DialogReason): # Pop dialog off stack self._stack.pop(0) + + def __set_exception_context_data(self, exception: Exception): + if not hasattr(exception, "data"): + exception.data = {} + + if not type(self).__name__ in exception.data: + stack = [] + current_dc = self + + while current_dc is not None: + stack = stack + [x.id for x in current_dc.stack] + current_dc = current_dc.parent + + exception.data[type(self).__name__] = { + "active_dialog": None + if self.active_dialog is None + else self.active_dialog.id, + "parent": None if self.parent is None else self.parent.active_dialog.id, + "stack": self.stack, + } diff --git a/libraries/botbuilder-dialogs/tests/test_activity_prompt.py b/libraries/botbuilder-dialogs/tests/test_activity_prompt.py index ab8fa4971..378561ab9 100644 --- a/libraries/botbuilder-dialogs/tests/test_activity_prompt.py +++ b/libraries/botbuilder-dialogs/tests/test_activity_prompt.py @@ -215,3 +215,85 @@ async def aux_validator(prompt_context: PromptValidatorContext): step1 = await adapter.send("hello") step2 = await step1.assert_reply("please send an event.") await step2.assert_reply("please send an event.") + + async def test_activity_prompt_onerror_should_return_dialogcontext(self): + # Create ConversationState with MemoryStorage and register the state as middleware. + convo_state = ConversationState(MemoryStorage()) + + # Create a DialogState property, DialogSet and AttachmentPrompt. + dialog_state = convo_state.create_property("dialog_state") + dialogs = DialogSet(dialog_state) + dialogs.add(SimpleActivityPrompt("EventActivityPrompt", validator)) + + async def exec_test(turn_context: TurnContext): + dialog_context = await dialogs.create_context(turn_context) + + results = await dialog_context.continue_dialog() + + if results.status == DialogTurnStatus.Empty: + options = PromptOptions( + prompt=Activity( + type=ActivityTypes.message, text="please send an event." + ) + ) + + try: + await dialog_context.prompt("EventActivityPrompt", options) + await dialog_context.prompt("Non existent id", options) + except Exception as err: + self.assertIsNotNone(err.data["DialogContext"]) + self.assertEqual( + err.data["DialogContext"]["active_dialog"], + "EventActivityPrompt", + ) + else: + raise Exception("Should have thrown an error.") + + elif results.status == DialogTurnStatus.Complete: + await turn_context.send_activity(results.result) + + await convo_state.save_changes(turn_context) + + # Initialize TestAdapter. + adapter = TestAdapter(exec_test) + + await adapter.send("hello") + + async def test_activity_replace_dialog_onerror_should_return_dialogcontext(self): + # Create ConversationState with MemoryStorage and register the state as middleware. + convo_state = ConversationState(MemoryStorage()) + + # Create a DialogState property, DialogSet and AttachmentPrompt. + dialog_state = convo_state.create_property("dialog_state") + dialogs = DialogSet(dialog_state) + dialogs.add(SimpleActivityPrompt("EventActivityPrompt", validator)) + + async def exec_test(turn_context: TurnContext): + dialog_context = await dialogs.create_context(turn_context) + + results = await dialog_context.continue_dialog() + + if results.status == DialogTurnStatus.Empty: + options = PromptOptions( + prompt=Activity( + type=ActivityTypes.message, text="please send an event." + ) + ) + + try: + await dialog_context.prompt("EventActivityPrompt", options) + await dialog_context.replace_dialog("Non existent id", options) + except Exception as err: + self.assertIsNotNone(err.data["DialogContext"]) + else: + raise Exception("Should have thrown an error.") + + elif results.status == DialogTurnStatus.Complete: + await turn_context.send_activity(results.result) + + await convo_state.save_changes(turn_context) + + # Initialize TestAdapter. + adapter = TestAdapter(exec_test) + + await adapter.send("hello") From b81e161c74d6598b938a67aeeb61ae04d79f3069 Mon Sep 17 00:00:00 2001 From: Joel Mut Date: Thu, 12 Nov 2020 09:55:08 -0300 Subject: [PATCH 2/4] Add pylint: disable=no-member for Exception.data property --- .../botbuilder-dialogs/tests/test_activity_prompt.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libraries/botbuilder-dialogs/tests/test_activity_prompt.py b/libraries/botbuilder-dialogs/tests/test_activity_prompt.py index 378561ab9..6a60b9662 100644 --- a/libraries/botbuilder-dialogs/tests/test_activity_prompt.py +++ b/libraries/botbuilder-dialogs/tests/test_activity_prompt.py @@ -241,11 +241,11 @@ async def exec_test(turn_context: TurnContext): await dialog_context.prompt("EventActivityPrompt", options) await dialog_context.prompt("Non existent id", options) except Exception as err: - self.assertIsNotNone(err.data["DialogContext"]) + self.assertIsNotNone(err.data["DialogContext"]) # pylint: disable=no-member self.assertEqual( - err.data["DialogContext"]["active_dialog"], + err.data["DialogContext"]["active_dialog"], # pylint: disable=no-member "EventActivityPrompt", - ) + ) else: raise Exception("Should have thrown an error.") @@ -284,7 +284,7 @@ async def exec_test(turn_context: TurnContext): await dialog_context.prompt("EventActivityPrompt", options) await dialog_context.replace_dialog("Non existent id", options) except Exception as err: - self.assertIsNotNone(err.data["DialogContext"]) + self.assertIsNotNone(err.data["DialogContext"]) # pylint: disable=no-member else: raise Exception("Should have thrown an error.") From 7ae86d182d01fa3ab340610411e2bd21bea65d55 Mon Sep 17 00:00:00 2001 From: Joel Mut Date: Thu, 12 Nov 2020 09:56:41 -0300 Subject: [PATCH 3/4] Remove unused trailing-whitespace --- libraries/botbuilder-dialogs/tests/test_activity_prompt.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/botbuilder-dialogs/tests/test_activity_prompt.py b/libraries/botbuilder-dialogs/tests/test_activity_prompt.py index 6a60b9662..cf58d8210 100644 --- a/libraries/botbuilder-dialogs/tests/test_activity_prompt.py +++ b/libraries/botbuilder-dialogs/tests/test_activity_prompt.py @@ -245,7 +245,7 @@ async def exec_test(turn_context: TurnContext): self.assertEqual( err.data["DialogContext"]["active_dialog"], # pylint: disable=no-member "EventActivityPrompt", - ) + ) else: raise Exception("Should have thrown an error.") From 1ee3377a0a5360f60b8722d05f495de441d86ad7 Mon Sep 17 00:00:00 2001 From: Joel Mut Date: Thu, 12 Nov 2020 10:10:17 -0300 Subject: [PATCH 4/4] Fix black formating --- .../botbuilder-dialogs/tests/test_activity_prompt.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/libraries/botbuilder-dialogs/tests/test_activity_prompt.py b/libraries/botbuilder-dialogs/tests/test_activity_prompt.py index cf58d8210..2f2019c91 100644 --- a/libraries/botbuilder-dialogs/tests/test_activity_prompt.py +++ b/libraries/botbuilder-dialogs/tests/test_activity_prompt.py @@ -241,9 +241,13 @@ async def exec_test(turn_context: TurnContext): await dialog_context.prompt("EventActivityPrompt", options) await dialog_context.prompt("Non existent id", options) except Exception as err: - self.assertIsNotNone(err.data["DialogContext"]) # pylint: disable=no-member + self.assertIsNotNone( + err.data["DialogContext"] # pylint: disable=no-member + ) self.assertEqual( - err.data["DialogContext"]["active_dialog"], # pylint: disable=no-member + err.data["DialogContext"][ # pylint: disable=no-member + "active_dialog" + ], "EventActivityPrompt", ) else: @@ -284,7 +288,9 @@ async def exec_test(turn_context: TurnContext): await dialog_context.prompt("EventActivityPrompt", options) await dialog_context.replace_dialog("Non existent id", options) except Exception as err: - self.assertIsNotNone(err.data["DialogContext"]) # pylint: disable=no-member + self.assertIsNotNone( + err.data["DialogContext"] # pylint: disable=no-member + ) else: raise Exception("Should have thrown an error.")