From 85c289d29627ed40da8829ecfe8fb2543ea10082 Mon Sep 17 00:00:00 2001 From: Chad Retz Date: Mon, 9 Dec 2024 14:11:27 -0600 Subject: [PATCH] Fix child workflow already exists and minor README updates Fixes #371 Fixes #368 Fixes #361 --- README.md | 3 +- .../Client/TemporalClient.Workflow.cs | 5 +- .../WorkflowAlreadyStartedException.cs | 29 ++++++++++-- src/Temporalio/Worker/WorkflowInstance.cs | 6 ++- src/Temporalio/Worker/WorkflowWorker.cs | 4 +- .../Worker/WorkflowWorkerTests.cs | 47 +++++++++++++++++++ 6 files changed, 85 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index dcd26296..692d5efe 100644 --- a/README.md +++ b/README.md @@ -88,7 +88,8 @@ CLI: dotnet add package Temporalio -If you are using .NET Framework or a non-standard target platform, see the +The .NET SDK supports .NET Framework >= 4.6.2, .NET Core >= 3.1 (so includes .NET 5+), and .NET Standard >= 2.0. If you +are using .NET Framework or a non-standard target platform, see the [Built-in Native Shared Library](#built-in-native-shared-library) section later for additional information. **NOTE: This README is for the current branch and not necessarily what's released on NuGet.** diff --git a/src/Temporalio/Client/TemporalClient.Workflow.cs b/src/Temporalio/Client/TemporalClient.Workflow.cs index 507503e4..cd19d625 100644 --- a/src/Temporalio/Client/TemporalClient.Workflow.cs +++ b/src/Temporalio/Client/TemporalClient.Workflow.cs @@ -106,8 +106,9 @@ public override async Task> StartWorkflowAsyn { throw new WorkflowAlreadyStartedException( e.Message, - input.Options.Id!, - failure.RunId); + workflowId: input.Options.Id!, + workflowType: input.Workflow, + runId: failure.RunId); } } throw; diff --git a/src/Temporalio/Exceptions/WorkflowAlreadyStartedException.cs b/src/Temporalio/Exceptions/WorkflowAlreadyStartedException.cs index 191ec388..a304420b 100644 --- a/src/Temporalio/Exceptions/WorkflowAlreadyStartedException.cs +++ b/src/Temporalio/Exceptions/WorkflowAlreadyStartedException.cs @@ -1,3 +1,5 @@ +using System; + namespace Temporalio.Exceptions { /// @@ -9,12 +11,27 @@ public class WorkflowAlreadyStartedException : FailureException /// Initializes a new instance of the class. /// /// Error message. - /// Already started workflow ID. - /// Already started run ID. + /// See . + /// See . + [Obsolete("Use other constructor")] public WorkflowAlreadyStartedException(string message, string workflowId, string runId) + : this(message, workflowId, "", runId) + { + } + + /// + /// Initializes a new instance of the class. + /// + /// Error message. + /// See . + /// See . + /// See . + public WorkflowAlreadyStartedException( + string message, string workflowId, string workflowType, string runId) : base(message) { WorkflowId = workflowId; + WorkflowType = workflowType; RunId = runId; } @@ -24,7 +41,13 @@ public WorkflowAlreadyStartedException(string message, string workflowId, string public string WorkflowId { get; private init; } /// - /// Gets the run ID that was already started. + /// Gets the workflow type that was attempted to start. + /// + public string WorkflowType { get; private init; } + + /// + /// Gets the run ID that was already started. This may be <unknown> when this + /// error is thrown for a child workflow from inside a workflow. /// public string RunId { get; private init; } } diff --git a/src/Temporalio/Worker/WorkflowInstance.cs b/src/Temporalio/Worker/WorkflowInstance.cs index 162441dc..9b77f971 100644 --- a/src/Temporalio/Worker/WorkflowInstance.cs +++ b/src/Temporalio/Worker/WorkflowInstance.cs @@ -2003,8 +2003,10 @@ public override Task> StartChildWorkflow handleSource.SetException( new WorkflowAlreadyStartedException( "Child workflow already started", - startRes.Failed.WorkflowId, - startRes.Failed.WorkflowType)); + workflowId: startRes.Failed.WorkflowId, + workflowType: startRes.Failed.WorkflowType, + // Pending https://github.com/temporalio/temporal/issues/6961 + runId: "")); return; default: handleSource.SetException(new InvalidOperationException( diff --git a/src/Temporalio/Worker/WorkflowWorker.cs b/src/Temporalio/Worker/WorkflowWorker.cs index 27730aea..6671b13a 100644 --- a/src/Temporalio/Worker/WorkflowWorker.cs +++ b/src/Temporalio/Worker/WorkflowWorker.cs @@ -49,7 +49,9 @@ public WorkflowWorker( { if (!defn.Instantiable) { - throw new ArgumentException($"Workflow named {defn.Name} is not instantiable"); + throw new ArgumentException($"Workflow named {defn.Name} is not instantiable. " + + "Note, dependency injection is not supported in workflows. " + + "Workflows must be deterministic and self-contained with a lifetime controlled by Temporal."); } if (defn.Name == null) { diff --git a/tests/Temporalio.Tests/Worker/WorkflowWorkerTests.cs b/tests/Temporalio.Tests/Worker/WorkflowWorkerTests.cs index bec04e4a..e813959d 100644 --- a/tests/Temporalio.Tests/Worker/WorkflowWorkerTests.cs +++ b/tests/Temporalio.Tests/Worker/WorkflowWorkerTests.cs @@ -6011,6 +6011,53 @@ await ExecuteWorkerAsync( new TemporalWorkerOptions().AddAllActivities(activities)); } + [Workflow] + public class ChildWorkflowAlreadyExists1Workflow + { + [WorkflowRun] + public Task RunAsync() => Workflow.WaitConditionAsync(() => false); + } + + [Workflow] + public class ChildWorkflowAlreadyExists2Workflow + { + [WorkflowRun] + public async Task RunAsync(string workflowId) + { + try + { + await Workflow.StartChildWorkflowAsync( + (ChildWorkflowAlreadyExists2Workflow wf) => wf.RunAsync("ignored"), + new() { Id = workflowId }); + throw new ApplicationFailureException("Should not be reached"); + } + catch (WorkflowAlreadyStartedException e) + { + return $"already started, type: {e.WorkflowType}, run ID: {e.RunId}"; + } + } + } + + [Fact] + public async Task ExecuteWorkflowAsync_ChildWorkflowAlreadyExists_ErrorsProperly() + { + await ExecuteWorkerAsync( + async worker => + { + // Start workflow + var handle = await Client.StartWorkflowAsync( + (ChildWorkflowAlreadyExists1Workflow wf) => wf.RunAsync(), + new(id: $"workflow-{Guid.NewGuid()}", taskQueue: worker.Options.TaskQueue!)); + + // Try to run other one + var result = await Client.ExecuteWorkflowAsync( + (ChildWorkflowAlreadyExists2Workflow wf) => wf.RunAsync(handle.Id), + new(id: $"workflow-{Guid.NewGuid()}", taskQueue: worker.Options.TaskQueue!)); + Assert.Equal("already started, type: ChildWorkflowAlreadyExists2Workflow, run ID: ", result); + }, + new TemporalWorkerOptions().AddWorkflow()); + } + internal static Task AssertTaskFailureContainsEventuallyAsync( WorkflowHandle handle, string messageContains) {