From a50dd51ca04bc9a57747301a400e6adf447b1fca Mon Sep 17 00:00:00 2001 From: Ethan Chiu <17chiue@gmail.com> Date: Wed, 22 Jul 2020 18:01:50 -0400 Subject: [PATCH] Fix Timeout-minutes for Whole Composite Action Step (#599) * Exploring child Linked Cancellation Tokens * Preliminary Timeout-minutes fix * Final Solution for resolving cancellation token's timeout vs. cancellation * Clean up + Fix error handling * Use linked tokens instead * Clean up * one liner * Remove JobExecutionContext => Replace with public Root accessor * Move CreateLinkedTokenSource in the CreateCompositeStep Function --- src/Runner.Worker/ExecutionContext.cs | 12 +++--- .../Handlers/CompositeActionHandler.cs | 42 +++---------------- 2 files changed, 13 insertions(+), 41 deletions(-) diff --git a/src/Runner.Worker/ExecutionContext.cs b/src/Runner.Worker/ExecutionContext.cs index 6f6279c6b..5385490d7 100644 --- a/src/Runner.Worker/ExecutionContext.cs +++ b/src/Runner.Worker/ExecutionContext.cs @@ -60,10 +60,12 @@ namespace GitHub.Runner.Worker bool EchoOnActionCommand { get; set; } + ExecutionContext Root { get; } + // Initialize void InitializeJob(Pipelines.AgentJobRequestMessage message, CancellationToken token); void CancelToken(); - IExecutionContext CreateChild(Guid recordId, string displayName, string refName, string scopeName, string contextName, Dictionary intraActionState = null, int? recordOrder = null, IPagingLogger logger = null); + IExecutionContext CreateChild(Guid recordId, string displayName, string refName, string scopeName, string contextName, Dictionary intraActionState = null, int? recordOrder = null, IPagingLogger logger = null, CancellationTokenSource cancellationTokenSource = null); // logging long Write(string tag, string message); @@ -180,7 +182,7 @@ namespace GitHub.Runner.Worker } } - private ExecutionContext Root + public ExecutionContext Root { get { @@ -254,7 +256,7 @@ namespace GitHub.Runner.Worker DictionaryContextData inputsData, Dictionary envData) { - step.ExecutionContext = Root.CreateChild(_record.Id, step.DisplayName, _record.Id.ToString("N"), scopeName, step.Action.ContextName, logger: _logger); + step.ExecutionContext = Root.CreateChild(_record.Id, step.DisplayName, _record.Id.ToString("N"), scopeName, step.Action.ContextName, logger: _logger, cancellationTokenSource: CancellationTokenSource.CreateLinkedTokenSource(_cancellationTokenSource.Token)); step.ExecutionContext.ExpressionValues["inputs"] = inputsData; step.ExecutionContext.ExpressionValues["steps"] = Global.StepsContext.GetScope(step.ExecutionContext.GetFullyQualifiedContextName()); @@ -273,7 +275,7 @@ namespace GitHub.Runner.Worker return step; } - public IExecutionContext CreateChild(Guid recordId, string displayName, string refName, string scopeName, string contextName, Dictionary intraActionState = null, int? recordOrder = null, IPagingLogger logger = null) + public IExecutionContext CreateChild(Guid recordId, string displayName, string refName, string scopeName, string contextName, Dictionary intraActionState = null, int? recordOrder = null, IPagingLogger logger = null, CancellationTokenSource cancellationTokenSource = null) { Trace.Entering(); @@ -298,7 +300,7 @@ namespace GitHub.Runner.Worker { child.ExpressionFunctions.Add(item); } - child._cancellationTokenSource = new CancellationTokenSource(); + child._cancellationTokenSource = cancellationTokenSource ?? new CancellationTokenSource(); child._parentExecutionContext = this; child.EchoOnActionCommand = EchoOnActionCommand; diff --git a/src/Runner.Worker/Handlers/CompositeActionHandler.cs b/src/Runner.Worker/Handlers/CompositeActionHandler.cs index 80e4d13a5..3ab7957cf 100644 --- a/src/Runner.Worker/Handlers/CompositeActionHandler.cs +++ b/src/Runner.Worker/Handlers/CompositeActionHandler.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.IO; using System.Linq; using System.Text; +using System.Threading; using System.Threading.Tasks; using GitHub.DistributedTask.ObjectTemplating.Tokens; using GitHub.DistributedTask.Pipelines.ContextData; @@ -197,19 +198,11 @@ namespace GitHub.Runner.Worker.Handlers step.ExecutionContext.Complete(TaskResult.Failed); } - // Handle Cancellation - // We will break out of loop immediately and display the result - if (ExecutionContext.CancellationToken.IsCancellationRequested) - { - ExecutionContext.Result = TaskResult.Canceled; - break; - } - await RunStepAsync(step); - // Handle Failed Step - // We will break out of loop immediately and display the result - if (step.ExecutionContext.Result == TaskResult.Failed) + // Directly after the step, check if the step has failed or cancelled + // If so, return that to the output + if (step.ExecutionContext.Result == TaskResult.Failed || step.ExecutionContext.Result == TaskResult.Canceled) { ExecutionContext.Result = step.ExecutionContext.Result; break; @@ -245,7 +238,8 @@ namespace GitHub.Runner.Worker.Handlers } catch (OperationCanceledException ex) { - if (step.ExecutionContext.CancellationToken.IsCancellationRequested) + if (step.ExecutionContext.CancellationToken.IsCancellationRequested && + !ExecutionContext.Root.CancellationToken.IsCancellationRequested) { Trace.Error($"Caught timeout exception from step: {ex.Message}"); step.ExecutionContext.Error("The action has timed out."); @@ -253,7 +247,6 @@ namespace GitHub.Runner.Worker.Handlers } else { - // Log the exception and cancel the step. Trace.Error($"Caught cancellation exception from step: {ex}"); step.ExecutionContext.Error(ex); step.ExecutionContext.Result = TaskResult.Canceled; @@ -273,29 +266,6 @@ namespace GitHub.Runner.Worker.Handlers step.ExecutionContext.Result = Common.Util.TaskResultUtil.MergeTaskResults(step.ExecutionContext.Result, step.ExecutionContext.CommandResult.Value); } - // Fixup the step result if ContinueOnError. - if (step.ExecutionContext.Result == TaskResult.Failed) - { - var continueOnError = false; - try - { - continueOnError = templateEvaluator.EvaluateStepContinueOnError(step.ContinueOnError, step.ExecutionContext.ExpressionValues, step.ExecutionContext.ExpressionFunctions); - } - catch (Exception ex) - { - Trace.Info("The step failed and an error occurred when attempting to determine whether to continue on error."); - Trace.Error(ex); - step.ExecutionContext.Error("The step failed and an error occurred when attempting to determine whether to continue on error."); - step.ExecutionContext.Error(ex); - } - - if (continueOnError) - { - step.ExecutionContext.Outcome = step.ExecutionContext.Result; - step.ExecutionContext.Result = TaskResult.Succeeded; - Trace.Info($"Updated step result (continue on error)"); - } - } Trace.Info($"Step result: {step.ExecutionContext.Result}"); // Complete the step context.