Skip to content

Fix TemporalWorker::ExecuteAsync(Func<Task>) swallowing exceptions - #600

Merged
cretz merged 6 commits into
temporalio:mainfrom
adeal:issues/bug/599
Jan 29, 2026
Merged

cretz merged 6 commits into
temporalio:mainfrom
adeal:issues/bug/599

Conversation

@adeal

@adeal adeal commented Jan 22, 2026

Copy link
Copy Markdown
Contributor

What was changed

  • Fix TemporalWorker::ExecuteAsync(Func<Task>) swallowing exceptions

Why?

The non-generic ExecuteAsync(Func<Task>) overload was using ContinueWith with TaskContinuationOptions.OnlyOnRanToCompletion. When the caller's task threw an exception, this option prevented the continuation from running, causing the continuation task to be canceled rather than faulted. This resulted in TaskCanceledException being thrown instead of the original exception (e.g., WorkflowFailedException).

Checklist

  1. Closes [Bug] TemporalWorker::ExecuteAsync(Func<Task>) swallows exceptions, throws TaskCanceledException instead #599

  2. How was this tested:
    Added regression test ExecuteAsync_PropagatesException.

  3. Any docs updates needed?
    No.

@adeal
adeal requested a review from a team as a code owner January 22, 2026 02:15
@CLAassistant

CLAassistant commented Jan 22, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Austin Deal seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

Comment thread tests/Temporalio.Tests/Testing/WorkflowEnvironmentTests.cs Outdated

@cretz cretz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Just one minor pedantic suggestion to simplify the test.

And don't forget to sign the CLA (link is above here in PR)

Comment thread tests/Temporalio.Tests/Testing/WorkflowEnvironmentTests.cs Outdated
Comment thread tests/Temporalio.Tests/Testing/WorkflowEnvironmentTests.cs Outdated
public Task ExecuteAsync(
Func<Task> untilComplete, CancellationToken stoppingToken = default) =>
ExecuteInternalAsync(
() => untilComplete().ContinueWith(

@cretz cretz Jan 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yeah, it is confusing that .NET doesn't really have a "map-on-success" combinator and that continue-with inadvertently swallows failures here. Thanks for the fix!

I see one other place we'll need to fix this type of thinking (Temporalio.Workflows.Semaphore for a failure in a wait condition), but we can handle that in a separate issue, no worries.

@adeal adeal changed the title Fix TemporalWorker::ExecuteAsync(Func<Task>) swallowing exceptions from void-returning workflows Fix TemporalWorker::ExecuteAsync(Func<Task>) swallowing exceptions Jan 22, 2026

@cretz cretz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One change needed, to use the existing shared environment instead of making a new one for just this test

Comment thread tests/Temporalio.Tests/Worker/GeneralWorkerTests.cs Outdated
Comment thread tests/Temporalio.Tests/Worker/GeneralWorkerTests.cs Outdated
@cretz

cretz commented Jan 22, 2026

Copy link
Copy Markdown
Contributor

@adeal - looks like some whitespace formatting failing in CI due to inadvertent commit to Semaphore.cs, can you remove that change? I opened #601 separately.

@cretz
cretz merged commit edb4ff9 into temporalio:main Jan 29, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] TemporalWorker::ExecuteAsync(Func<Task>) swallows exceptions, throws TaskCanceledException instead

3 participants