Add error detection when EmitEventAsync fails (#6708)

libraries/Microsoft.Bot.Builder.Dialogs/DialogExtensions.cs

@@ -2,6 +2,7 @@
 // Licensed under the MIT License.
 
 using System;
+using System.Collections.Generic;
 using System.Linq;
 using System.Security.Claims;
 using System.Security.Principal;
@@ -74,8 +75,25 @@ internal static async Task<DialogTurnResult> InternalRunAsync(ITurnContext turnC
                 }
                 catch (Exception err)
                 {
-                    // fire error event, bubbling from the leaf.
-                    var handled = await dialogContext.EmitEventAsync(DialogEvents.Error, err, bubble: true, fromLeaf: true, cancellationToken: cancellationToken).ConfigureAwait(false);
+                    var handled = false;
+                    var innerExceptions = new List<Exception>();
+                    try
+                    {
+                        // fire error event, bubbling from the leaf.
+                        handled = await dialogContext.EmitEventAsync(DialogEvents.Error, err, bubble: true, fromLeaf: true, cancellationToken: cancellationToken).ConfigureAwait(false);
+                    }
+#pragma warning disable CA1031 // Do not catch general exception types (capture the error in case it's not handled properly)
+                    catch (Exception emitErr)
+#pragma warning restore CA1031 // Do not catch general exception types
+                    {
+                        innerExceptions.Add(emitErr);
+                    }
+
+                    if (innerExceptions.Any())
+                    {
+                        innerExceptions.Add(err);
+                        throw new AggregateException("Unable to emit the error as a DialogEvent.", innerExceptions);
+                    }
 
                     if (!handled)
                     {

tests/Microsoft.Bot.Builder.AI.QnA.Tests/LanguageServiceTests.cs

@@ -1119,17 +1119,26 @@ public void LanguageService_Test_Endpoint_EmptyKbId()
         [Trait("TestCategory", "LanguageService")]
         public async void LanguageService_Test_NotSpecifiedQnAServiceType()
         {
+            AggregateException result = null;
             var mockHttp = new MockHttpMessageHandler();
             mockHttp.When(HttpMethod.Post, GetRequestUrl())
                 .Respond("application/json", GetResponse("LanguageService_ReturnsAnswer.json"));
 
             var rootDialog = CreateLanguageServiceActionDialog(mockHttp, true, ServiceType.QnAMaker);
 
-            await Assert.ThrowsAsync<HttpRequestException>(() => CreateFlow(rootDialog, nameof(LanguageServiceAction_MultiTurnDialogBase_WithNoAnswer))
-        .Send("What happens if I pass empty qnaServiceType with host of Language Service")
-        .StartTestAsync());
+            try
+            {
+                await CreateFlow(rootDialog, nameof(LanguageServiceAction_MultiTurnDialogBase_WithNoAnswer))
+                    .Send("What happens if I pass empty qnaServiceType with host of Language Service")
+                    .StartTestAsync();
+            }
+            catch (AggregateException ex)
+            {
+                result = ex;
+            }
 
-            //Assert.Throws<ArgumentOutOfRangeException>(() => new CustomQuestionAnswering(endpoint, qnaMakerOptions));
+            Assert.Equal(2, result.InnerExceptions.Count);
+            Assert.All(result.InnerExceptions, (e) => Assert.IsType<HttpRequestException>(e));
         }
 
         /// <summary>

tests/Microsoft.Bot.Builder.Dialogs.Tests/DialogExtensionsTests.cs

@@ -2,11 +2,14 @@
 // Licensed under the MIT License.
 
 using System;
+using System.Collections.Generic;
 using System.Linq;
 using System.Security.Claims;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.Bot.Builder.Adapters;
+using Microsoft.Bot.Builder.Dialogs.Adaptive;
+using Microsoft.Bot.Builder.Dialogs.Adaptive.Conditions;
 using Microsoft.Bot.Builder.Skills;
 using Microsoft.Bot.Connector.Authentication;
 using Microsoft.Bot.Schema;
@@ -143,6 +146,40 @@ public async Task RunAsyncShouldSetTelemetryClient()
             Assert.Equal(telemetryClientMock.Object, dialog.TelemetryClient);
         }
 
+        [Fact]
+        public async Task RunAsyncWithCircularReferenceExceptionShouldFail()
+        {
+            var conversationReference = TestAdapter.CreateConversation(nameof(RunAsyncWithCircularReferenceExceptionShouldFail));
+
+            var adapter = new TestAdapter(conversationReference)
+                   .UseBotState(new ConversationState(new MemoryStorage()), new UserState(new MemoryStorage()));
+
+            try
+            {
+                var dm = new DialogManager(new AdaptiveDialog("adaptiveDialog")
+                {
+                    Triggers = new List<OnCondition>()
+                {
+                    new OnBeginDialog()
+                    {
+                        Actions = new List<Dialog>()
+                        {
+                            new CustomExceptionDialog()
+                        }
+                    }
+                }
+                });
+
+                await new TestFlow((TestAdapter)adapter, dm.OnTurnAsync)
+                    .Send("hi")
+                    .StartTestAsync();
+            }
+            catch (AggregateException ex)
+            {
+                Assert.Equal(2, ex.InnerExceptions.Count);
+            }
+        }
+
         /// <summary>
         /// Creates a TestFlow instance with state data to recreate and assert the different test case.
         /// </summary>
@@ -253,5 +290,35 @@ private static async Task<DialogTurnResult> FinalStepAsync(WaterfallStepContext
                 return await stepContext.EndDialogAsync(stepContext.Result, cancellationToken);
             }
         }
+
+        private class CustomExceptionDialog : Dialog
+        {
+            public CustomExceptionDialog()
+                : base("custom-exception")
+            {
+            }
+
+            public override Task<DialogTurnResult> BeginDialogAsync(DialogContext dc, object options = null, CancellationToken cancellationToken = default)
+            {
+                var e1 = new CustomException("Parent: Self referencing Exception");
+                var e2 = new CustomException("Child: Self referencing Exception", e1);
+                e1.Children.Add(e2);
+                throw e1;
+            }
+
+            private class CustomException : Exception
+            {
+#pragma warning disable SA1401 // Fields should be private
+                public List<CustomException> Children = new List<CustomException>();
+                public CustomException Parent;
+#pragma warning restore SA1401 // Fields should be private
+
+                public CustomException(string message, CustomException parent = null)
+                    : base("Error: " + message)
+                {
+                    Parent = parent;
+                }
+            }
+        }
     }
 }