Skip to content

Commit

Permalink
Fix child workflow already exists and minor README updates (#379)
Browse files Browse the repository at this point in the history
Fixes #371
Fixes #368
Fixes #361
  • Loading branch information
cretz authored Dec 11, 2024
1 parent aac363d commit 301062a
Show file tree
Hide file tree
Showing 6 changed files with 85 additions and 9 deletions.
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.**
Expand Down
5 changes: 3 additions & 2 deletions src/Temporalio/Client/TemporalClient.Workflow.cs
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,9 @@ public override async Task<WorkflowHandle<TWorkflow, TResult>> StartWorkflowAsyn
{
throw new WorkflowAlreadyStartedException(
e.Message,
input.Options.Id!,
failure.RunId);
workflowId: input.Options.Id!,
workflowType: input.Workflow,
runId: failure.RunId);
}
}
throw;
Expand Down
29 changes: 26 additions & 3 deletions src/Temporalio/Exceptions/WorkflowAlreadyStartedException.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
using System;

namespace Temporalio.Exceptions
{
/// <summary>
Expand All @@ -9,12 +11,27 @@ public class WorkflowAlreadyStartedException : FailureException
/// Initializes a new instance of the <see cref="WorkflowAlreadyStartedException"/> class.
/// </summary>
/// <param name="message">Error message.</param>
/// <param name="workflowId">Already started workflow ID.</param>
/// <param name="runId">Already started run ID.</param>
/// <param name="workflowId">See <see cref="WorkflowId"/>.</param>
/// <param name="runId">See <see cref="RunId"/>.</param>
[Obsolete("Use other constructor")]
public WorkflowAlreadyStartedException(string message, string workflowId, string runId)
: this(message, workflowId, "<unknown>", runId)
{
}

/// <summary>
/// Initializes a new instance of the <see cref="WorkflowAlreadyStartedException"/> class.
/// </summary>
/// <param name="message">Error message.</param>
/// <param name="workflowId">See <see cref="WorkflowId"/>.</param>
/// <param name="workflowType">See <see cref="WorkflowType"/>.</param>
/// <param name="runId">See <see cref="RunId"/>.</param>
public WorkflowAlreadyStartedException(
string message, string workflowId, string workflowType, string runId)
: base(message)
{
WorkflowId = workflowId;
WorkflowType = workflowType;
RunId = runId;
}

Expand All @@ -24,7 +41,13 @@ public WorkflowAlreadyStartedException(string message, string workflowId, string
public string WorkflowId { get; private init; }

/// <summary>
/// Gets the run ID that was already started.
/// Gets the workflow type that was attempted to start.
/// </summary>
public string WorkflowType { get; private init; }

/// <summary>
/// Gets the run ID that was already started. This may be <c>&lt;unknown&gt;</c> when this
/// error is thrown for a child workflow from inside a workflow.
/// </summary>
public string RunId { get; private init; }
}
Expand Down
6 changes: 4 additions & 2 deletions src/Temporalio/Worker/WorkflowInstance.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2061,8 +2061,10 @@ public override Task<ChildWorkflowHandle<TWorkflow, TResult>> 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: "<unknown>"));
return;
default:
handleSource.SetException(new InvalidOperationException(
Expand Down
4 changes: 3 additions & 1 deletion src/Temporalio/Worker/WorkflowWorker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
47 changes: 47 additions & 0 deletions tests/Temporalio.Tests/Worker/WorkflowWorkerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6011,6 +6011,53 @@ await ExecuteWorkerAsync<DetachedCancellationWorkflow>(
new TemporalWorkerOptions().AddAllActivities(activities));
}

[Workflow]
public class ChildWorkflowAlreadyExists1Workflow
{
[WorkflowRun]
public Task RunAsync() => Workflow.WaitConditionAsync(() => false);
}

[Workflow]
public class ChildWorkflowAlreadyExists2Workflow
{
[WorkflowRun]
public async Task<string> 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<ChildWorkflowAlreadyExists1Workflow>(
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: <unknown>", result);
},
new TemporalWorkerOptions().AddWorkflow<ChildWorkflowAlreadyExists2Workflow>());
}

[Workflow]
public class UserMetadataWorkflow
{
Expand Down

0 comments on commit 301062a

Please sign in to comment.