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" diff --git a/libraries/Microsoft.Bot.Builder.Dialogs.Adaptive/TriggerConditions/OnError.cs b/libraries/Microsoft.Bot.Builder.Dialogs.Adaptive/TriggerConditions/OnError.cs index 8b49ef95ee..d9f37452aa 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 infinite loops in case of error. + /// + /// The number of executions allowed for this trigger. + [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..8860d9e019 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 infinite loops in case of errors. + /// + public const string ExecutionLimit = "turn.executionLimit"; + /// /// The result from the last dialog that was called. /// 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 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" },