Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix child workflow already exists and minor README updates #379

Merged
merged 2 commits into from
Dec 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading