From 37422110ce8226023257af4ef68129bb2cebffec Mon Sep 17 00:00:00 2001 From: CeciliaAvila Date: Wed, 10 Apr 2024 16:19:39 -0300 Subject: [PATCH 1/6] Add ExecutionLimit property to break loops --- .../TriggerConditions/OnError.cs | 22 +++++++++++++++++ .../DialogExtensions.cs | 24 +++++++++++++++++++ .../Memory/TurnPath.cs | 5 ++++ 3 files changed, 51 insertions(+) diff --git a/libraries/Microsoft.Bot.Builder.Dialogs.Adaptive/TriggerConditions/OnError.cs b/libraries/Microsoft.Bot.Builder.Dialogs.Adaptive/TriggerConditions/OnError.cs index 8b49ef95ee..34337188b7 100644 --- a/libraries/Microsoft.Bot.Builder.Dialogs.Adaptive/TriggerConditions/OnError.cs +++ b/libraries/Microsoft.Bot.Builder.Dialogs.Adaptive/TriggerConditions/OnError.cs @@ -3,6 +3,8 @@ using System.Collections.Generic; using System.Runtime.CompilerServices; +using System.Threading.Tasks; +using AdaptiveExpressions.Properties; using Newtonsoft.Json; namespace Microsoft.Bot.Builder.Dialogs.Adaptive.Conditions @@ -31,6 +33,26 @@ public OnError(List actions = null, string condition = null, [CallerFile { } + /// + /// Gets or sets the number of executions allowed. Used to avoid infinit loops in case of error. + /// + /// . + [JsonProperty("executionLimit")] + public NumberExpression ExecutionLimit { get; set; } = new NumberExpression(); + + /// + /// Method called to execute the rule's actions. + /// + /// Context. + /// A with plan change list. + public override Task> ExecuteAsync(ActionContext actionContext) + { + // 10 is the default number of executions we'll allow before breaking the loop. + var limit = ExecutionLimit.Value > 0 ? ExecutionLimit.Value : 10; + actionContext.State.SetValue(TurnPath.ExecutionLimit, limit); + return base.ExecuteAsync(actionContext); + } + /// /// Called when a change list is created. /// diff --git a/libraries/Microsoft.Bot.Builder.Dialogs/DialogExtensions.cs b/libraries/Microsoft.Bot.Builder.Dialogs/DialogExtensions.cs index a76e29e1a7..6bdfbf87c5 100644 --- a/libraries/Microsoft.Bot.Builder.Dialogs/DialogExtensions.cs +++ b/libraries/Microsoft.Bot.Builder.Dialogs/DialogExtensions.cs @@ -12,6 +12,7 @@ using Microsoft.Bot.Builder.Skills; using Microsoft.Bot.Connector.Authentication; using Microsoft.Bot.Schema; +using Newtonsoft.Json.Linq; namespace Microsoft.Bot.Builder.Dialogs { @@ -64,6 +65,8 @@ internal static async Task InternalRunAsync(ITurnContext turnC // or we have had an exception AND there was an OnError action which captured the error. We need to continue the // turn based on the actions the OnError handler introduced. var endOfTurn = false; + var errorHandlerCalled = 0; + while (!endOfTurn) { try @@ -79,8 +82,29 @@ internal static async Task InternalRunAsync(ITurnContext turnC var innerExceptions = new List(); try { + errorHandlerCalled++; + // fire error event, bubbling from the leaf. handled = await dialogContext.EmitEventAsync(DialogEvents.Error, err, bubble: true, fromLeaf: true, cancellationToken: cancellationToken).ConfigureAwait(false); + + var turnObject = (JObject)turnContext.TurnState["turn"]; + + var executionLimit = 0; + + foreach (var childToken in turnObject.Children()) + { + if (childToken.Name == "executionLimit") + { + executionLimit = (int)childToken.Value; + } + } + + if (executionLimit > 0 && errorHandlerCalled > executionLimit) + { + // if the errorHandler has being called multiple times, there's an error inside the onError. + // We should throw the exception and break the loop. + handled = false; + } } #pragma warning disable CA1031 // Do not catch general exception types (capture the error in case it's not handled properly) catch (Exception emitErr) diff --git a/libraries/Microsoft.Bot.Builder.Dialogs/Memory/TurnPath.cs b/libraries/Microsoft.Bot.Builder.Dialogs/Memory/TurnPath.cs index e619c5b8ec..5bdf0feb41 100644 --- a/libraries/Microsoft.Bot.Builder.Dialogs/Memory/TurnPath.cs +++ b/libraries/Microsoft.Bot.Builder.Dialogs/Memory/TurnPath.cs @@ -72,6 +72,11 @@ public class TurnPath /// public const string ActivityProcessed = "turn.activityProcessed"; + /// + /// Used to limit the execution of a trigger avoiding infinit loops in case of errors. + /// + public const string ExecutionLimit = "turn.executionLimit"; + /// /// The result from the last dialog that was called. /// From eb7507dedaef09ad75c74db48111de492509ddf3 Mon Sep 17 00:00:00 2001 From: CeciliaAvila Date: Thu, 11 Apr 2024 14:20:18 -0300 Subject: [PATCH 2/6] Fix typos in comments --- .../TriggerConditions/OnError.cs | 2 +- libraries/Microsoft.Bot.Builder.Dialogs/Memory/TurnPath.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/Microsoft.Bot.Builder.Dialogs.Adaptive/TriggerConditions/OnError.cs b/libraries/Microsoft.Bot.Builder.Dialogs.Adaptive/TriggerConditions/OnError.cs index 34337188b7..01b9607ff5 100644 --- a/libraries/Microsoft.Bot.Builder.Dialogs.Adaptive/TriggerConditions/OnError.cs +++ b/libraries/Microsoft.Bot.Builder.Dialogs.Adaptive/TriggerConditions/OnError.cs @@ -34,7 +34,7 @@ public OnError(List actions = null, string condition = null, [CallerFile } /// - /// Gets or sets the number of executions allowed. Used to avoid infinit loops in case of error. + /// Gets or sets the number of executions allowed. Used to avoid infinite loops in case of error. /// /// . [JsonProperty("executionLimit")] diff --git a/libraries/Microsoft.Bot.Builder.Dialogs/Memory/TurnPath.cs b/libraries/Microsoft.Bot.Builder.Dialogs/Memory/TurnPath.cs index 5bdf0feb41..8860d9e019 100644 --- a/libraries/Microsoft.Bot.Builder.Dialogs/Memory/TurnPath.cs +++ b/libraries/Microsoft.Bot.Builder.Dialogs/Memory/TurnPath.cs @@ -73,7 +73,7 @@ public class TurnPath public const string ActivityProcessed = "turn.activityProcessed"; /// - /// Used to limit the execution of a trigger avoiding infinit loops in case of errors. + /// Used to limit the execution of a trigger avoiding infinite loops in case of errors. /// public const string ExecutionLimit = "turn.executionLimit"; From 6b0f424bbda798d8223852db8dcd1ec7e3dd09a9 Mon Sep 17 00:00:00 2001 From: CeciliaAvila Date: Thu, 11 Apr 2024 14:32:31 -0300 Subject: [PATCH 3/6] Add missing comment. --- .../TriggerConditions/OnError.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/Microsoft.Bot.Builder.Dialogs.Adaptive/TriggerConditions/OnError.cs b/libraries/Microsoft.Bot.Builder.Dialogs.Adaptive/TriggerConditions/OnError.cs index 01b9607ff5..d9f37452aa 100644 --- a/libraries/Microsoft.Bot.Builder.Dialogs.Adaptive/TriggerConditions/OnError.cs +++ b/libraries/Microsoft.Bot.Builder.Dialogs.Adaptive/TriggerConditions/OnError.cs @@ -36,7 +36,7 @@ public OnError(List actions = null, string condition = null, [CallerFile /// /// Gets or sets the number of executions allowed. Used to avoid infinite loops in case of error. /// - /// . + /// The number of executions allowed for this trigger. [JsonProperty("executionLimit")] public NumberExpression ExecutionLimit { get; set; } = new NumberExpression(); From a080c90c5f570d976e6be01fd42af998d9e01034 Mon Sep 17 00:00:00 2001 From: CeciliaAvila Date: Wed, 17 Apr 2024 14:42:55 -0300 Subject: [PATCH 4/6] Add new property to schema files --- .../TriggerConditions/Microsoft.OnError.schema | 13 ++++++++++++- .../TriggerConditions/Microsoft.OnError.uischema | 3 ++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/libraries/Microsoft.Bot.Builder.Dialogs.Adaptive/Schemas/TriggerConditions/Microsoft.OnError.schema b/libraries/Microsoft.Bot.Builder.Dialogs.Adaptive/Schemas/TriggerConditions/Microsoft.OnError.schema index ddce686825..950722d963 100644 --- a/libraries/Microsoft.Bot.Builder.Dialogs.Adaptive/Schemas/TriggerConditions/Microsoft.OnError.schema +++ b/libraries/Microsoft.Bot.Builder.Dialogs.Adaptive/Schemas/TriggerConditions/Microsoft.OnError.schema @@ -3,5 +3,16 @@ "$role": [ "implements(Microsoft.ITrigger)", "extends(Microsoft.OnCondition)" ], "title": "On error", "description": "Action to perform when an 'Error' dialog event occurs.", - "type": "object" + "type": "object", + "properties": { + "executionLimit": { + "$ref": "schema:#/definitions/numberExpression", + "title": "Execution limit", + "description": "Number of executions allowed for this trigger. Used to break loops in case of errors inside the trigger.", + "examples": [ + 3, + "=f(x)" + ] + } + } } diff --git a/libraries/Microsoft.Bot.Builder.Dialogs.Adaptive/Schemas/TriggerConditions/Microsoft.OnError.uischema b/libraries/Microsoft.Bot.Builder.Dialogs.Adaptive/Schemas/TriggerConditions/Microsoft.OnError.uischema index 3c4081c0ba..43ab4bc23d 100644 --- a/libraries/Microsoft.Bot.Builder.Dialogs.Adaptive/Schemas/TriggerConditions/Microsoft.OnError.uischema +++ b/libraries/Microsoft.Bot.Builder.Dialogs.Adaptive/Schemas/TriggerConditions/Microsoft.OnError.uischema @@ -3,7 +3,8 @@ "form": { "order": [ "condition", - "*" + "*", + "executionLimit" ], "hidden": [ "actions" From 552bfae4bdb0375480c8adff9699410c10eae1da Mon Sep 17 00:00:00 2001 From: CeciliaAvila Date: Thu, 18 Apr 2024 10:29:13 -0300 Subject: [PATCH 5/6] Add unit tests --- .../ConditionalsTests.cs | 12 +++ .../ConditionalsTests_OnErrorLoop.test.dialog | 65 +++++++++++++ ...sTests_OnErrorLoopDefaultLimit.test.dialog | 92 +++++++++++++++++++ 3 files changed, 169 insertions(+) create mode 100644 tests/Microsoft.Bot.Builder.Dialogs.Adaptive.Tests/Tests/ConditionalsTests/ConditionalsTests_OnErrorLoop.test.dialog create mode 100644 tests/Microsoft.Bot.Builder.Dialogs.Adaptive.Tests/Tests/ConditionalsTests/ConditionalsTests_OnErrorLoopDefaultLimit.test.dialog diff --git a/tests/Microsoft.Bot.Builder.Dialogs.Adaptive.Tests/ConditionalsTests.cs b/tests/Microsoft.Bot.Builder.Dialogs.Adaptive.Tests/ConditionalsTests.cs index 2cacc2bc17..f15771b5a2 100644 --- a/tests/Microsoft.Bot.Builder.Dialogs.Adaptive.Tests/ConditionalsTests.cs +++ b/tests/Microsoft.Bot.Builder.Dialogs.Adaptive.Tests/ConditionalsTests.cs @@ -89,6 +89,18 @@ public async Task ConditionalsTests_OnChooseIntent() await TestUtils.RunTestScript(_resourceExplorerFixture.ResourceExplorer); } + [Fact] + public async Task ConditionalsTests_OnErrorLoop() + { + await TestUtils.RunTestScript(_resourceExplorerFixture.ResourceExplorer); + } + + [Fact] + public async Task ConditionalsTests_OnErrorLoopDefaultLimit() + { + await TestUtils.RunTestScript(_resourceExplorerFixture.ResourceExplorer); + } + [Fact] public void OnConditionWithCondition() { diff --git a/tests/Microsoft.Bot.Builder.Dialogs.Adaptive.Tests/Tests/ConditionalsTests/ConditionalsTests_OnErrorLoop.test.dialog b/tests/Microsoft.Bot.Builder.Dialogs.Adaptive.Tests/Tests/ConditionalsTests/ConditionalsTests_OnErrorLoop.test.dialog new file mode 100644 index 0000000000..59293c4517 --- /dev/null +++ b/tests/Microsoft.Bot.Builder.Dialogs.Adaptive.Tests/Tests/ConditionalsTests/ConditionalsTests_OnErrorLoop.test.dialog @@ -0,0 +1,65 @@ +{ + "$schema": "../../../tests.schema", + "$kind": "Microsoft.Test.Script", + "dialog": { + "$kind": "Microsoft.AdaptiveDialog", + "id": "ErrorLoop", + "autoEndDialog": true, + "defaultResultProperty": "dialog.result", + "triggers": [ + { + "$kind": "Microsoft.OnBeginDialog", + "actions": [ + { + "$kind": "Microsoft.SendActivity", + "activity": "Throw Exception in BeginDialog." + }, + { + "$kind": "Microsoft.ThrowException", + "errorValue": "Exception in BeginDialog." + } + ] + }, + { + "$kind": "Microsoft.OnError", + "actions": [ + { + "$kind": "Microsoft.SendActivity", + "activity": "Throw Exception in OnError." + }, + { + "$kind": "Microsoft.ThrowException", + "errorValue": "Exception in OnError." + } + ], + "executionLimit": 3 + } + ] + }, + "script": [ + { + "$kind": "Microsoft.Test.UserSays", + "text": "hi" + }, + { + "$kind": "Microsoft.Test.AssertReply", + "text": "Throw Exception in BeginDialog." + }, + { + "$kind": "Microsoft.Test.AssertReply", + "text": "Throw Exception in OnError." + }, + { + "$kind": "Microsoft.Test.AssertReply", + "text": "Throw Exception in OnError." + }, + { + "$kind": "Microsoft.Test.AssertReply", + "text": "Throw Exception in OnError." + }, + { + "$kind": "Microsoft.Test.AssertReply", + "text": "Exception in OnError." + } + ] +} \ No newline at end of file diff --git a/tests/Microsoft.Bot.Builder.Dialogs.Adaptive.Tests/Tests/ConditionalsTests/ConditionalsTests_OnErrorLoopDefaultLimit.test.dialog b/tests/Microsoft.Bot.Builder.Dialogs.Adaptive.Tests/Tests/ConditionalsTests/ConditionalsTests_OnErrorLoopDefaultLimit.test.dialog new file mode 100644 index 0000000000..cad53c9b68 --- /dev/null +++ b/tests/Microsoft.Bot.Builder.Dialogs.Adaptive.Tests/Tests/ConditionalsTests/ConditionalsTests_OnErrorLoopDefaultLimit.test.dialog @@ -0,0 +1,92 @@ +{ + "$schema": "../../../tests.schema", + "$kind": "Microsoft.Test.Script", + "dialog": { + "$kind": "Microsoft.AdaptiveDialog", + "id": "ErrorLoop", + "autoEndDialog": true, + "defaultResultProperty": "dialog.result", + "triggers": [ + { + "$kind": "Microsoft.OnBeginDialog", + "actions": [ + { + "$kind": "Microsoft.SendActivity", + "activity": "Throw Exception in BeginDialog." + }, + { + "$kind": "Microsoft.ThrowException", + "errorValue": "Exception in BeginDialog." + } + ] + }, + { + "$kind": "Microsoft.OnError", + "actions": [ + { + "$kind": "Microsoft.SendActivity", + "activity": "Throw Exception in OnError." + }, + { + "$kind": "Microsoft.ThrowException", + "errorValue": "Exception in OnError." + } + ] + } + ] + }, + "script": [ + { + "$kind": "Microsoft.Test.UserSays", + "text": "hi" + }, + { + "$kind": "Microsoft.Test.AssertReply", + "text": "Throw Exception in BeginDialog." + }, + { + "$kind": "Microsoft.Test.AssertReply", + "text": "Throw Exception in OnError." + }, + { + "$kind": "Microsoft.Test.AssertReply", + "text": "Throw Exception in OnError." + }, + { + "$kind": "Microsoft.Test.AssertReply", + "text": "Throw Exception in OnError." + }, + { + "$kind": "Microsoft.Test.AssertReply", + "text": "Throw Exception in OnError." + }, + { + "$kind": "Microsoft.Test.AssertReply", + "text": "Throw Exception in OnError." + }, + { + "$kind": "Microsoft.Test.AssertReply", + "text": "Throw Exception in OnError." + }, + { + "$kind": "Microsoft.Test.AssertReply", + "text": "Throw Exception in OnError." + }, + { + "$kind": "Microsoft.Test.AssertReply", + "text": "Throw Exception in OnError." + }, + { + "$kind": "Microsoft.Test.AssertReply", + "text": "Throw Exception in OnError." + }, + { + "$kind": "Microsoft.Test.AssertReply", + "text": "Throw Exception in OnError." + }, + { + "$kind": "Microsoft.Test.AssertReply", + "text": "Exception in OnError." + } + ] +} \ No newline at end of file From e3a39ab5eedb2cd9108fa5bd29e7e6a02a3cdc28 Mon Sep 17 00:00:00 2001 From: CeciliaAvila Date: Thu, 18 Apr 2024 11:30:06 -0300 Subject: [PATCH 6/6] Update tests.schema and tests.uischema files --- tests/tests.schema | 9 +++++++++ tests/tests.uischema | 3 ++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/tests/tests.schema b/tests/tests.schema index ad41a31b42..8d89e66d53 100644 --- a/tests/tests.schema +++ b/tests/tests.schema @@ -6498,6 +6498,15 @@ } }, "properties": { + "executionLimit": { + "$ref": "#/definitions/numberExpression", + "title": "Execution limit", + "description": "Number of executions allowed for this trigger. Used to break loops in case of errors inside the trigger.", + "examples": [ + 3, + "=f(x)" + ] + }, "condition": { "$ref": "#/definitions/condition", "title": "Condition", diff --git a/tests/tests.uischema b/tests/tests.uischema index 3f601049c5..70d334af9d 100644 --- a/tests/tests.uischema +++ b/tests/tests.uischema @@ -1166,7 +1166,8 @@ "label": "Error occurred", "order": [ "condition", - "*" + "*", + "executionLimit" ], "subtitle": "Error event" },