dapr / dapr/dotnet-sdk

Workflow `GetWorkflowStateAsync` / `GetWorkflowMetadataAsync` return `null` on transient errors, indistinguishable from a missing workflow

Open
#1,874 1 comment 1 reaction 1 assignee Claimed by @WhitWaldo View on GitHub
area/workflow kind/bug
Dominant language
C#
Stars
1.2k
Forks
378
Avg merge
1d 22h
Merged PRs (30d)
5

Description

**Dapr .NET SDK:** 1.18.4 (`Dapr.Workflow`).

**Related runtime issue: **[dapr runtime 10226](https://github.com/dapr/dotnet-sdk/issues/1876)**

## Expected Behavior

A caller of `DaprWorkflowClient.GetWorkflowStateAsync` (or `WorkflowClient.GetWorkflowMetadataAsync`) should be able to tell **"the workflow does not exist"** apart from **"a transient/recoverable error occurred while reading its state"**.

- Genuine not-found → `null` / `WorkflowState.Exists == false`.
- Transient error (e.g. gRPC `Unknown`, `Unavailable`, `DeadlineExceeded`, cancellation) → surfaced distinctly (thrown, or a result that signals "error, not absence") so the caller can retry instead of concluding the workflow is gone.

## Actual Behavior

Both methods collapse *every* error into the same `null` / `Exists == false` result that also means "not found", so the two are indistinguishable.

`WorkflowGrpcClient.GetWorkflowMetadataAsync` — the final `catch (Exception)` turns **any** non-`NotFound` failure into `null`:

```csharp
try
{
var response = await grpcClient.GetInstanceAsync(request, grpcCallOptions);
if (response is null) { logger.LogGetWorkflowMetadataInstanceNotFound(instanceId); return null; }
if (!response.Exists) { return null; } // genuine not-found
return ProtoConverters.ToWorkflowMetadata(response.WorkflowState, serializer);
}
catch (RpcException ex) when (ex.StatusCode == StatusCode.NotFound)
{
return null; // genuine not-found
}
catch (Exception ex)
{
logger.LogError(ex, "Error getting workflow metadata for instance '{InstanceId}'", instanceId);
return null; // ← ANY transient/unknown error → same null
}
```

`DaprWorkflowClient.GetWorkflowStateAsync` then wraps that (and its own `RpcException`) into a `WorkflowState`, and its XML doc explicitly conflates the two cases:

```csharp
///
/// A WorkflowState if the workflow instance exists, or null if the instance does not
/// exist or an error occurs retrieving the metadata.
/// This method never throws.
///
public async Task GetWorkflowStateAsync(string instanceId, bool getInputsAndOutputs = true, CancellationToken cancellation = default)
{
ArgumentException.ThrowIfNullOrEmpty(instanceId);
try
{
var metadata = await _innerClient.GetWorkflowMetadataAsync(instanceId, getInputsAndOutputs, cancellation);
return new WorkflowState(metadata); // metadata == null => WorkflowState.Exists == false
}
catch (RpcException)
{
return new WorkflowState(null); // error => also Exists == false
}
}
```

Since `WorkflowState.Exists => _metadata is not null`, a transient error and a truly-missing instance produce the **identical** `Exists == false`, with no exception and no signal. The caller cannot retry intelligently and may treat a still-running workflow as gone.

Notes:
- This is inconsistent with `WaitForWorkflowStartAsync` / `WaitForWorkflowCompletionAsync`, which **throw** `InvalidOperationException` when the instance does not exist — so the SDK already distinguishes existence in the wait path, but not in the get-state path.
- Concrete trigger: a companion `dapr/dapr` runtime issue [10226](https://github.com/dapr/dotnet-sdk/issues/1876) intermittently returns gRPC `Unknown` (`inbox key '…' declared in metadata … but missing from state store`) for a **running** workflow. That transient `Unknown` hits the `catch (Exception)` above and is reported to callers as `Exists == false`, i.e. as if the workflow no longer exists.

## Steps to Reproduce the Problem

1. Start a workflow and let it run.
2. While it is running, make the sidecar return a transient, non-`NotFound` error for `GetInstance` (e.g. reproduce the runtime `Unknown`/"missing from state store" race, or otherwise induce `Unavailable`/`DeadlineExceeded`).
3. Call `client.GetWorkflowStateAsync(instanceId)`.
4. Observe: it returns a `WorkflowState` with `Exists == false` — identical to `GetWorkflowStateAsync("does-not-exist")` — and never throws. There is no way to tell "transient error, retry" from "workflow is gone".

Minimal caller that is forced to guess:

```csharp
var state = await client.GetWorkflowStateAsync(instanceId); // never throws
if (!state.Exists)
{
// Was the workflow purged/never-created, or did a transient error just occur?
// The SDK gives no way to know — both look the same.
}
```

Suggested fix: in `GetWorkflowMetadataAsync`, only map `NotFound` / `!Exists` to `null`; rethrow (or otherwise surface) other `RpcException`s. Correspondingly, let `GetWorkflowStateAsync` either throw on non-not-found errors or expose the error distinctly, rather than documenting/implementing "never throws → null on any error".

## Release Note

RELEASE NOTE: **FIX** Workflow `GetWorkflowStateAsync`/`GetWorkflowMetadataAsync` no longer report transient errors as a missing workflow (`Exists == false`); not-found and recoverable errors are now distinguishable.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.