feat(ContextPropagator): Replace imperative set/unset context propagation with lexically-scoped runWithContext by rlei · Pull Request #1074 · cadence-workflow/cadence-java-client

rlei · 2026-08-12T16:25:56Z

Summary

Adds ContextPropagator.runWithContext(Object context, ContextRunnable task), a default method that pairs setCurrentContext/task.run()/unsetCurrentContext in a single try/finally, replacing the previous pattern where context was set at the top of a handler and unset in a separate finally block possibly several methods away.
ContextThreadLocal.runWithContext(...) composes all configured propagators into a single nested call chain executed lexically around the task.
Updates ActivityWorker, LocalActivityWorker, and WorkflowThreadImpl to run their task through this single call.

Why

JDK 25 finalizes ScopedValue (JEP 481), a lexically-scoped, immutable alternative to ThreadLocal: a value is bound and the task runs inside that binding in a single call (ScopedValue.where(...).call(task)), with no separate unset step. The previous ContextPropagator API couldn't express that shape. Restructuring around a single runWithContext(context, task) call:

Simplicity: a ScopedValue-backed propagator only needs to override runWithContext and call ScopedValue.where(VALUE, context).run(task) — no separate set/get/unset methods to keep consistent with each other.
Immutable context data: ScopedValue bindings are write-once for the dynamic extent of the call; nothing downstream can accidentally mutate a shared context object the way a mutable ThreadLocal value can be.
No unpaired unset: because the binding and the task execution happen in the same call frame, cleanup is guaranteed by stack unwinding — removing the risk of an early return, a thrown exception, or a future refactor leaving a propagator's context set with no matching unset.

Existing propagators that implement setCurrentContext/unsetCurrentContext are unaffected — runWithContext has a default implementation built on those methods, so this is additive for current implementations and opt-in for ScopedValue-based ones.

Test plan

./gradlew test (full suite) passes
New ContextThreadLocalTest covers nested propagator ordering, lexical-scope override behavior, and unset-after-failure behavior

…h lexically-scoped runWithContext Why: JDK 25 finalizes ScopedValue (JEP 481), a lexically-scoped, immutable alternative to ThreadLocal for propagating context: a value is bound and the task runs inside that binding in a single call (`ScopedValue.where(...).call(task)`), with no separate unset step -- the binding is torn down automatically when the call returns. The previous ContextPropagator API couldn't express that: context was installed via `setCurrentContext`/`deserializeContext` at the top of a handler (e.g. ActivityWorker.propagateContext, LocalActivityWorker.restoreContext, WorkflowThreadImpl's propagateContextToCurrentThread) and torn down via `unsetCurrentContext` in a `finally` block possibly several methods away, which doesn't map onto ScopedValue's single-call binding model. Restructuring around a single `runWithContext(context, task)` call gets us: - Simplicity: a ScopedValue-backed propagator only needs to override `runWithContext` and call `ScopedValue.where(VALUE, context).run(task)` -- no separate set/get/unset methods to keep consistent with each other. - Immutable context data: ScopedValue bindings are write-once for the dynamic extent of the call; nothing downstream can accidentally mutate a shared context object the way a mutable ThreadLocal value can be. - No unpaired unset: because the binding and the task execution happen in the same call frame, cleanup is guaranteed by stack unwinding. The old shape's risk -- an early return, a thrown exception before the try block, or a future refactor splitting set/unset further apart leaving a propagator's context set with no matching unset and leaking it onto whatever task a pooled thread picks up next -- goes away entirely. Change: - Add `ContextPropagator.runWithContext(Object context, ContextRunnable task)`, a default method that wraps `setCurrentContext`/`task.run()`/`unsetCurrentContext` in a single try/finally, so set and unset are always paired in one call frame. - `ContextThreadLocal.runWithContext(...)` composes all configured propagators into a single nested call chain and executes the task inside it, replacing the old `propagateContextToCurrentThread` + manual `unsetCurrentContext` pair. - Update the three call sites (ActivityWorker, LocalActivityWorker, WorkflowThreadImpl) to run their task through this single call instead of bracketing it with separate propagate/unset calls. Existing propagators that implement `setCurrentContext`/`unsetCurrentContext` are unaffected; `runWithContext` has a default implementation built on those methods, so this is additive for current implementations and opt-in for ScopedValue-based ones. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

shijiesheng · 2026-08-13T16:57:04Z


-  public static void propagateContextToCurrentThread(Map<String, Object> contextData) {
-    if (contextData == null || contextData.isEmpty()) {
+  public static void runWithContext(Map<String, Object> contextData, ContextRunnable task)


From AI: custom code can now swallow exceptions that WorkflowThread throws. Do you want to add some guard on this? For example we can compare the original result, throwable and compare them with the new output.
We also want to add some comments to discourage users to swallow and modify exceptions.

good point. discussed offline, the fix will be to detect such bad runWithContext implementations and fail it with a ContextPropagatorSwallowedExceptionError (extending Error) to alert the propagator owner.

…tion Why: Review feedback on cadence-workflow#1074 (github.com/cadence-workflow/pull/1074#pullrequestreview-4929567768) pointed out a real regression risk in the runWithContext redesign: because a ContextPropagator's runWithContext(context, task) now directly wraps the call to task.run(), a buggy implementation can structurally catch and suppress (or replace) whatever the task throws -- including workflow cancellation and thread-destruction signals from WorkflowThread. The old imperative setCurrentContext/unsetCurrentContext shape made this impossible, since propagators never touched the call site of the wrapped task. Change: - ContextThreadLocal.runWithContext now wraps the innermost task in a canary that records any Throwable it throws before rethrowing it. After the composed propagator chain returns, if the canary recorded a Throwable but none escaped the chain, some propagator in it swallowed the exception -- in which case a new ContextPropagatorSwallowedExceptionError is thrown (listing the propagators applied to that call) instead of silently continuing as if the task had succeeded. - ContextPropagatorSwallowedExceptionError extends Error, matching this codebase's existing convention for framework-level, must-not-be-retried conditions (NonDeterminisicWorkflowError, DestroyWorkflowThreadError, WorkflowRejectedExecutionError). This also means it composes correctly with existing handling: WorkflowThreadImpl already special-cases `catch (Error e)` to abort the decision rather than fail the workflow, and PollTaskExecutor routes any Throwable escaping an activity handler to the configured uncaughtExceptionHandler instead of a normal activity failure response. - Strengthened the ContextPropagator#runWithContext javadoc to state the contract explicitly: implementations must only wrap task.run() in try/finally, never try/catch. - Added a test with a deliberately misbehaving propagator that catches and suppresses the task's exception, verifying the new error is thrown with the original exception as its cause. This does not catch every misbehavior -- a propagator that catches and replaces the exception with a different one can't be reliably distinguished from a propagator intentionally translating exception types, so that half of the original concern is addressed by the javadoc contract instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…tion Backport of the equivalent fix on the master PR (cadence-workflow#1074) to this v3.13.x backport branch (cadence-workflow#1075). Why: Review feedback on cadence-workflow#1074 (github.com/cadence-workflow/pull/1074#pullrequestreview-4929567768) pointed out a real regression risk in the runWithContext redesign: because a ContextPropagator's runWithContext(context, task) now directly wraps the call to task.run(), a buggy implementation can structurally catch and suppress (or replace) whatever the task throws -- including workflow cancellation and thread-destruction signals from WorkflowThread. The old imperative setCurrentContext/unsetCurrentContext shape made this impossible, since propagators never touched the call site of the wrapped task. Change: - ContextThreadLocal.runWithContext now wraps the innermost task in a canary that records any Throwable it throws before rethrowing it. After the composed propagator chain returns, if the canary recorded a Throwable but none escaped the chain, some propagator in it swallowed the exception -- in which case a new ContextPropagatorSwallowedExceptionError is thrown (listing the propagators applied to that call) instead of silently continuing as if the task had succeeded. - ContextPropagatorSwallowedExceptionError extends Error, matching this codebase's existing convention for framework-level, must-not-be-retried conditions (NonDeterminisicWorkflowError, DestroyWorkflowThreadError, WorkflowRejectedExecutionError). This also means it composes correctly with existing handling: WorkflowThreadImpl already special-cases `catch (Error e)` to abort the decision rather than fail the workflow, and PollTaskExecutor routes any Throwable escaping an activity handler to the configured uncaughtExceptionHandler instead of a normal activity failure response. - Strengthened the ContextPropagator#runWithContext javadoc to state the contract explicitly: implementations must only wrap task.run() in try/finally, never try/catch. - Added a test with a deliberately misbehaving propagator that catches and suppresses the task's exception, verifying the new error is thrown with the original exception as its cause. This does not catch every misbehavior -- a propagator that catches and replaces the exception with a different one can't be reliably distinguished from a propagator intentionally translating exception types, so that half of the original concern is addressed by the javadoc contract instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…ong number of times Why: Follow-up to the swallow-detection fix, addressing github.com/cadence-workflow/pull/1074#discussion_r3778176907 (gitar-bot): the swallow-detection canary's `thrown` AtomicReference was set on the first task failure and never cleared, so a propagator that catches the task's exception and then retries it, eventually succeeding, would incorrectly be flagged as having swallowed the earlier failure. Rather than making the canary tolerate retries, retrying task is itself a contract violation -- ContextPropagator#runWithContext must call task.run() exactly once. So instead this adds a second, independent check: an invocation counter that must equal exactly one. It covers both a propagator that skips calling task entirely and one that calls it more than once (including the retry-then-succeed case), and it is checked before the existing swallow check, so a retry-then-succeed propagator is now correctly reported as an invocation-count violation rather than a misleading "swallowed exception" one. Change: - ContextThreadLocal.runWithContext tracks an AtomicInteger invocationCount alongside the existing AtomicReference<Throwable> thrown, incrementing it in the same canary that wraps task.run(). After the composed propagator chain returns normally, invocationCount != 1 is checked first (covers zero and multiple invocations), then the existing swallowed-exception check. - Renamed ContextPropagatorSwallowedExceptionError to ContextPropagatorContractViolationError (extends Error, same as before) since it now models two distinct contract violations via named static factory methods (swallowedException, unexpectedInvocationCount) instead of a single constructor. - Updated ContextPropagator#runWithContext's javadoc contract to state task.run() must be called exactly once -- never skipped, never retried. - Added a test with a propagator that retries task after catching its exception and eventually succeeds, verifying it is reported as an invocation-count violation. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…ong number of times Backport of the equivalent fix on the master PR (cadence-workflow#1074) to this v3.13.x backport branch (cadence-workflow#1075). Why: Follow-up to the swallow-detection fix, addressing github.com/cadence-workflow/pull/1074#discussion_r3778176907 (gitar-bot): the swallow-detection canary's `thrown` AtomicReference was set on the first task failure and never cleared, so a propagator that catches the task's exception and then retries it, eventually succeeding, would incorrectly be flagged as having swallowed the earlier failure. Rather than making the canary tolerate retries, retrying task is itself a contract violation -- ContextPropagator#runWithContext must call task.run() exactly once. So instead this adds a second, independent check: an invocation counter that must equal exactly one. It covers both a propagator that skips calling task entirely and one that calls it more than once (including the retry-then-succeed case), and it is checked before the existing swallow check, so a retry-then-succeed propagator is now correctly reported as an invocation-count violation rather than a misleading "swallowed exception" one. Change: - ContextThreadLocal.runWithContext tracks an AtomicInteger invocationCount alongside the existing AtomicReference<Throwable> thrown, incrementing it in the same canary that wraps task.run(). After the composed propagator chain returns normally, invocationCount != 1 is checked first (covers zero and multiple invocations), then the existing swallowed-exception check. - Renamed ContextPropagatorSwallowedExceptionError to ContextPropagatorContractViolationError (extends Error, same as before) since it now models two distinct contract violations via named static factory methods (swallowedException, unexpectedInvocationCount) instead of a single constructor. - Updated ContextPropagator#runWithContext's javadoc contract to state task.run() must be called exactly once -- never skipped, never retried. - Added a test with a propagator that retries task after catching its exception and eventually succeeds, verifying it is reported as an invocation-count violation. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…ion-count violation Why: Follow-up to the invocation-count check, addressing github.com/cadence-workflow/pull/1074#discussion_r3778369888 (gitar-bot): when a propagator retries the task after an exception (invocationCount != 1), ContextPropagatorContractViolationError was thrown with a null cause, even though `thrown` already held the exception from the attempt that triggered the retry. Attaching it makes it much easier to debug a misbehaving propagator, since the resulting stack trace shows both the contract violation and the original failure that provoked it. Change: - ContextPropagatorContractViolationError.unexpectedInvocationCount now takes the last captured Throwable and passes it through as the cause, instead of always passing null. - ContextThreadLocal.runWithContext passes thrown.get() into that call. - Updated the retry test to assert the original exception is preserved as the cause. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…ion-count violation Backport of the equivalent fix on the master PR (cadence-workflow#1074) to this v3.13.x backport branch (cadence-workflow#1075). Why: Follow-up to the invocation-count check, addressing github.com/cadence-workflow/pull/1074#discussion_r3778369888 (gitar-bot): when a propagator retries the task after an exception (invocationCount != 1), ContextPropagatorContractViolationError was thrown with a null cause, even though `thrown` already held the exception from the attempt that triggered the retry. Attaching it makes it much easier to debug a misbehaving propagator, since the resulting stack trace shows both the contract violation and the original failure that provoked it. Change: - ContextPropagatorContractViolationError.unexpectedInvocationCount now takes the last captured Throwable and passes it through as the cause, instead of always passing null. - ContextThreadLocal.runWithContext passes thrown.get() into that call. - Updated the retry test to assert the original exception is preserved as the cause. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

gitar-bot · 2026-08-13T20:16:06Z

CI failed: 1 PR title validation failure caused by missing conventional commit prefix in the pull request title.

Overview

1 unique configuration failure found across 2 logs where the PR title validation action rejected the pull request title due to a missing conventional commit prefix.

Failures

PR Title Validation Failure (confidence: high)

Type: configuration
Affected jobs: 94580409510, 94195911844
Related to change: yes
Root cause: The pull request title lacks a conventional commit prefix (e.g., feat:, fix:, refactor:).
Suggested fix: Rename the pull request title to include an appropriate conventional commit prefix (e.g., refactor: Replace imperative set/unset context propagation with lexically-scoped runWithContext).

Summary

Change-related failures: 1 failure due to PR title format validation
Infrastructure/flaky failures: 0 failures
Recommended action: Update the pull request title to follow the conventional commit format.

Code Review ✅ Approved 2 resolved / 2 findings

Refactors context propagation to use a lexically-scoped runWithContext method, addressing stale thrown triggers and ensuring original task exceptions are preserved on retry contract violation. No issues found.

✅ 2 resolved

✅ Edge Case: Stale thrown triggers false swallow-error on task re-invocation

📄 src/main/java/com/uber/cadence/internal/context/ContextThreadLocal.java:92-106
The thrown AtomicReference is set on the first task failure and never cleared. If a propagator catches the task exception, recovers, and then re-runs task successfully (or otherwise causes invocation.run() to return normally after a failed attempt), thrown.get() still holds the first exception and ContextPropagatorSwallowedExceptionError is thrown even though the chain ultimately completed without an escaping exception. This is contract-violating propagator behavior so it is low severity, but the failure mode is surprising. Consider documenting that re-invocation/recovery is unsupported, or clearing/scoping the reference per invocation.

✅ Quality: Original task exception discarded on retry contract violation

📄 src/main/java/com/uber/cadence/internal/context/ContextThreadLocal.java:118-125 📄 src/main/java/com/uber/cadence/internal/context/ContextPropagatorContractViolationError.java:49-60
When a propagator retries the task after an exception (count != 1), unexpectedInvocationCount is thrown with a null cause (ContextThreadLocal.java:118-121), even though thrown already holds the original failure that triggered the retry. Debugging a misbehaving propagator would be easier if that captured throwable were attached as the cause. Consider passing thrown.get() into unexpectedInvocationCount so the underlying task exception is preserved.

Tip

Comment Gitar fix CI or enable auto-apply: gitar auto-apply:on

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

`Auto-apply`	`Compact`
`gitar auto-apply:on`	`gitar display:verbose`

_{Was this helpful? React with 👍 / 👎 | Gitar}

…tion Backport of the equivalent fix on the master PR (cadence-workflow#1074) to this v3.13.x backport branch (cadence-workflow#1075). Why: Review feedback on cadence-workflow#1074 (github.com/cadence-workflow/pull/1074#pullrequestreview-4929567768) pointed out a real regression risk in the runWithContext redesign: because a ContextPropagator's runWithContext(context, task) now directly wraps the call to task.run(), a buggy implementation can structurally catch and suppress (or replace) whatever the task throws -- including workflow cancellation and thread-destruction signals from WorkflowThread. The old imperative setCurrentContext/unsetCurrentContext shape made this impossible, since propagators never touched the call site of the wrapped task. Change: - ContextThreadLocal.runWithContext now wraps the innermost task in a canary that records any Throwable it throws before rethrowing it. After the composed propagator chain returns, if the canary recorded a Throwable but none escaped the chain, some propagator in it swallowed the exception -- in which case a new ContextPropagatorSwallowedExceptionError is thrown (listing the propagators applied to that call) instead of silently continuing as if the task had succeeded. - ContextPropagatorSwallowedExceptionError extends Error, matching this codebase's existing convention for framework-level, must-not-be-retried conditions (NonDeterminisicWorkflowError, DestroyWorkflowThreadError, WorkflowRejectedExecutionError). This also means it composes correctly with existing handling: WorkflowThreadImpl already special-cases `catch (Error e)` to abort the decision rather than fail the workflow, and PollTaskExecutor routes any Throwable escaping an activity handler to the configured uncaughtExceptionHandler instead of a normal activity failure response. - Strengthened the ContextPropagator#runWithContext javadoc to state the contract explicitly: implementations must only wrap task.run() in try/finally, never try/catch. - Added a test with a deliberately misbehaving propagator that catches and suppresses the task's exception, verifying the new error is thrown with the original exception as its cause. This does not catch every misbehavior -- a propagator that catches and replaces the exception with a different one can't be reliably distinguished from a propagator intentionally translating exception types, so that half of the original concern is addressed by the javadoc contract instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Haofeng Lei <haofeng.lei@uber.com>

…ong number of times Backport of the equivalent fix on the master PR (cadence-workflow#1074) to this v3.13.x backport branch (cadence-workflow#1075). Why: Follow-up to the swallow-detection fix, addressing github.com/cadence-workflow/pull/1074#discussion_r3778176907 (gitar-bot): the swallow-detection canary's `thrown` AtomicReference was set on the first task failure and never cleared, so a propagator that catches the task's exception and then retries it, eventually succeeding, would incorrectly be flagged as having swallowed the earlier failure. Rather than making the canary tolerate retries, retrying task is itself a contract violation -- ContextPropagator#runWithContext must call task.run() exactly once. So instead this adds a second, independent check: an invocation counter that must equal exactly one. It covers both a propagator that skips calling task entirely and one that calls it more than once (including the retry-then-succeed case), and it is checked before the existing swallow check, so a retry-then-succeed propagator is now correctly reported as an invocation-count violation rather than a misleading "swallowed exception" one. Change: - ContextThreadLocal.runWithContext tracks an AtomicInteger invocationCount alongside the existing AtomicReference<Throwable> thrown, incrementing it in the same canary that wraps task.run(). After the composed propagator chain returns normally, invocationCount != 1 is checked first (covers zero and multiple invocations), then the existing swallowed-exception check. - Renamed ContextPropagatorSwallowedExceptionError to ContextPropagatorContractViolationError (extends Error, same as before) since it now models two distinct contract violations via named static factory methods (swallowedException, unexpectedInvocationCount) instead of a single constructor. - Updated ContextPropagator#runWithContext's javadoc contract to state task.run() must be called exactly once -- never skipped, never retried. - Added a test with a propagator that retries task after catching its exception and eventually succeeds, verifying it is reported as an invocation-count violation. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Haofeng Lei <haofeng.lei@uber.com>

…ion-count violation Backport of the equivalent fix on the master PR (cadence-workflow#1074) to this v3.13.x backport branch (cadence-workflow#1075). Why: Follow-up to the invocation-count check, addressing github.com/cadence-workflow/pull/1074#discussion_r3778369888 (gitar-bot): when a propagator retries the task after an exception (invocationCount != 1), ContextPropagatorContractViolationError was thrown with a null cause, even though `thrown` already held the exception from the attempt that triggered the retry. Attaching it makes it much easier to debug a misbehaving propagator, since the resulting stack trace shows both the contract violation and the original failure that provoked it. Change: - ContextPropagatorContractViolationError.unexpectedInvocationCount now takes the last captured Throwable and passes it through as the cause, instead of always passing null. - ContextThreadLocal.runWithContext passes thrown.get() into that call. - Updated the retry test to assert the original exception is preserved as the cause. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Haofeng Lei <haofeng.lei@uber.com>

…tion with lexically-scoped runWithContext (v3.13.x backport) (#1075) * [cadence-client] Replace imperative set/unset context propagation with lexically-scoped runWithContext Why: JDK 25 finalizes ScopedValue (JEP 481), a lexically-scoped, immutable alternative to ThreadLocal for propagating context: a value is bound and the task runs inside that binding in a single call (`ScopedValue.where(...).call(task)`), with no separate unset step -- the binding is torn down automatically when the call returns. The previous ContextPropagator API couldn't express that: context was installed via `setCurrentContext`/`deserializeContext` at the top of a handler (e.g. ActivityWorker.propagateContext, LocalActivityWorker.restoreContext, WorkflowThreadImpl's propagateContextToCurrentThread) and torn down via `unsetCurrentContext` in a `finally` block possibly several methods away, which doesn't map onto ScopedValue's single-call binding model. Restructuring around a single `runWithContext(context, task)` call gets us: - Simplicity: a ScopedValue-backed propagator only needs to override `runWithContext` and call `ScopedValue.where(VALUE, context).run(task)` -- no separate set/get/unset methods to keep consistent with each other. - Immutable context data: ScopedValue bindings are write-once for the dynamic extent of the call; nothing downstream can accidentally mutate a shared context object the way a mutable ThreadLocal value can be. - No unpaired unset: because the binding and the task execution happen in the same call frame, cleanup is guaranteed by stack unwinding. The old shape's risk -- an early return, a thrown exception before the try block, or a future refactor splitting set/unset further apart leaving a propagator's context set with no matching unset and leaking it onto whatever task a pooled thread picks up next -- goes away entirely. Change: - Add `ContextPropagator.runWithContext(Object context, ContextRunnable task)`, a default method that wraps `setCurrentContext`/`task.run()`/`unsetCurrentContext` in a single try/finally, so set and unset are always paired in one call frame. - `ContextThreadLocal.runWithContext(...)` composes all configured propagators into a single nested call chain and executes the task inside it, replacing the old `propagateContextToCurrentThread` + manual `unsetCurrentContext` pair. - Update the three call sites (ActivityWorker, LocalActivityWorker, WorkflowThreadImpl) to run their task through this single call instead of bracketing it with separate propagate/unset calls. Existing propagators that implement `setCurrentContext`/`unsetCurrentContext` are unaffected; `runWithContext` has a default implementation built on those methods, so this is additive for current implementations and opt-in for ScopedValue-based ones. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Haofeng Lei <haofeng.lei@uber.com> * [cadence-client] Fail fast when a ContextPropagator swallows an exception Backport of the equivalent fix on the master PR (#1074) to this v3.13.x backport branch (#1075). Why: Review feedback on #1074 (github.com//pull/1074#pullrequestreview-4929567768) pointed out a real regression risk in the runWithContext redesign: because a ContextPropagator's runWithContext(context, task) now directly wraps the call to task.run(), a buggy implementation can structurally catch and suppress (or replace) whatever the task throws -- including workflow cancellation and thread-destruction signals from WorkflowThread. The old imperative setCurrentContext/unsetCurrentContext shape made this impossible, since propagators never touched the call site of the wrapped task. Change: - ContextThreadLocal.runWithContext now wraps the innermost task in a canary that records any Throwable it throws before rethrowing it. After the composed propagator chain returns, if the canary recorded a Throwable but none escaped the chain, some propagator in it swallowed the exception -- in which case a new ContextPropagatorSwallowedExceptionError is thrown (listing the propagators applied to that call) instead of silently continuing as if the task had succeeded. - ContextPropagatorSwallowedExceptionError extends Error, matching this codebase's existing convention for framework-level, must-not-be-retried conditions (NonDeterminisicWorkflowError, DestroyWorkflowThreadError, WorkflowRejectedExecutionError). This also means it composes correctly with existing handling: WorkflowThreadImpl already special-cases `catch (Error e)` to abort the decision rather than fail the workflow, and PollTaskExecutor routes any Throwable escaping an activity handler to the configured uncaughtExceptionHandler instead of a normal activity failure response. - Strengthened the ContextPropagator#runWithContext javadoc to state the contract explicitly: implementations must only wrap task.run() in try/finally, never try/catch. - Added a test with a deliberately misbehaving propagator that catches and suppresses the task's exception, verifying the new error is thrown with the original exception as its cause. This does not catch every misbehavior -- a propagator that catches and replaces the exception with a different one can't be reliably distinguished from a propagator intentionally translating exception types, so that half of the original concern is addressed by the javadoc contract instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Haofeng Lei <haofeng.lei@uber.com> * [cadence-client] Also detect a ContextPropagator invoking task the wrong number of times Backport of the equivalent fix on the master PR (#1074) to this v3.13.x backport branch (#1075). Why: Follow-up to the swallow-detection fix, addressing github.com//pull/1074#discussion_r3778176907 (gitar-bot): the swallow-detection canary's `thrown` AtomicReference was set on the first task failure and never cleared, so a propagator that catches the task's exception and then retries it, eventually succeeding, would incorrectly be flagged as having swallowed the earlier failure. Rather than making the canary tolerate retries, retrying task is itself a contract violation -- ContextPropagator#runWithContext must call task.run() exactly once. So instead this adds a second, independent check: an invocation counter that must equal exactly one. It covers both a propagator that skips calling task entirely and one that calls it more than once (including the retry-then-succeed case), and it is checked before the existing swallow check, so a retry-then-succeed propagator is now correctly reported as an invocation-count violation rather than a misleading "swallowed exception" one. Change: - ContextThreadLocal.runWithContext tracks an AtomicInteger invocationCount alongside the existing AtomicReference<Throwable> thrown, incrementing it in the same canary that wraps task.run(). After the composed propagator chain returns normally, invocationCount != 1 is checked first (covers zero and multiple invocations), then the existing swallowed-exception check. - Renamed ContextPropagatorSwallowedExceptionError to ContextPropagatorContractViolationError (extends Error, same as before) since it now models two distinct contract violations via named static factory methods (swallowedException, unexpectedInvocationCount) instead of a single constructor. - Updated ContextPropagator#runWithContext's javadoc contract to state task.run() must be called exactly once -- never skipped, never retried. - Added a test with a propagator that retries task after catching its exception and eventually succeeds, verifying it is reported as an invocation-count violation. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Haofeng Lei <haofeng.lei@uber.com> * [cadence-client] Preserve original task exception as cause on invocation-count violation Backport of the equivalent fix on the master PR (#1074) to this v3.13.x backport branch (#1075). Why: Follow-up to the invocation-count check, addressing github.com//pull/1074#discussion_r3778369888 (gitar-bot): when a propagator retries the task after an exception (invocationCount != 1), ContextPropagatorContractViolationError was thrown with a null cause, even though `thrown` already held the exception from the attempt that triggered the retry. Attaching it makes it much easier to debug a misbehaving propagator, since the resulting stack trace shows both the contract violation and the original failure that provoked it. Change: - ContextPropagatorContractViolationError.unexpectedInvocationCount now takes the last captured Throwable and passes it through as the cause, instead of always passing null. - ContextThreadLocal.runWithContext passes thrown.get() into that call. - Updated the retry test to assert the original exception is preserved as the cause. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Haofeng Lei <haofeng.lei@uber.com> * [cadence-client] Fix flaky testLocalActivityAndQuery batch-boundary assertion ReplayDecider#executeLocalActivities caps how many local activities land in one decision batch by comparing elapsed processing time against 80% of the decision task timeout. With the test's 5s timeout that budget is exactly 4000ms -- precisely 2x the fixed 2000ms sleepActivity duration used here -- so whether a 3rd activity also completes within the first batch is decided by a few milliseconds of scheduling jitter around that exact tie, not anything the test should be asserting on. Tolerate the batch boundary landing on either side instead of asserting a single hardcoded snapshot, while still verifying that querying mid-execution returns real, non-decreasing progress before the workflow completes. Signed-off-by: Haofeng Lei <haofeng.lei@uber.com> * [cadence-client] Fix flaky testEnqueueSignalWithStart_includesTracing span count TChannel finishes its own per-request outbound span (in addition to the explicit "cadence-..." span WorkflowServiceTChannel creates) from a future completion callback registered on the TFuture. That callback is not guaranteed to run before the blocking RPC call returns to the caller, so the assertion on finishedSpans().size() could observe only 1 of the 2 expected spans immediately after enqueueSignalWithStart() returns, depending on which thread wins the race. Poll briefly for the expected span count before asserting instead of checking immediately. Signed-off-by: Haofeng Lei <haofeng.lei@uber.com> --------- Signed-off-by: Haofeng Lei <haofeng.lei@uber.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

rlei requested review from abhishekj720, demirkayaender, dkrotx, natemort and shijiesheng as code owners August 12, 2026 16:25

rlei force-pushed the codex/cadence-scoped-propagation-run-with-context branch from 01b4ed5 to b83f249 Compare August 12, 2026 16:46

shijiesheng reviewed Aug 12, 2026

View reviewed changes

Comment thread src/main/java/com/uber/cadence/context/ContextPropagator.java

rlei changed the title ~~Replace imperative set/unset context propagation with lexically-scoped runWithContext~~ [refactor Replace imperative set/unset context propagation with lexically-scoped runWithContext Aug 13, 2026

rlei changed the title ~~[refactor Replace imperative set/unset context propagation with lexically-scoped runWithContext~~ [refactor] Replace imperative set/unset context propagation with lexically-scoped runWithContext Aug 13, 2026

rlei force-pushed the codex/cadence-scoped-propagation-run-with-context branch from b83f249 to 513744d Compare August 13, 2026 12:05

rlei force-pushed the codex/cadence-scoped-propagation-run-with-context branch from 513744d to f3b269a Compare August 13, 2026 12:10

rlei mentioned this pull request Aug 13, 2026

feat(ContextPropagator): Replace imperative set/unset context propagation with lexically-scoped runWithContext (v3.13.x backport) #1075

Merged

1 task

shijiesheng reviewed Aug 13, 2026

View reviewed changes

gitar-bot Bot reviewed Aug 13, 2026

View reviewed changes

Comment thread src/main/java/com/uber/cadence/internal/context/ContextThreadLocal.java

gitar-bot Bot reviewed Aug 13, 2026

View reviewed changes

Comment thread src/main/java/com/uber/cadence/internal/context/ContextThreadLocal.java

rlei requested a review from shijiesheng August 13, 2026 19:59

shijiesheng approved these changes Aug 13, 2026

View reviewed changes

rlei changed the title ~~[refactor] Replace imperative set/unset context propagation with lexically-scoped runWithContext~~ refactor: Replace imperative set/unset context propagation with lexically-scoped runWithContext Aug 13, 2026

rlei changed the title ~~refactor: Replace imperative set/unset context propagation with lexically-scoped runWithContext~~ feat(contextpropagator): Replace imperative set/unset context propagation with lexically-scoped runWithContext Aug 13, 2026

rlei changed the title ~~feat(contextpropagator): Replace imperative set/unset context propagation with lexically-scoped runWithContext~~ feat(ContextPropagator): Replace imperative set/unset context propagation with lexically-scoped runWithContext Aug 13, 2026

shijiesheng enabled auto-merge (squash) August 13, 2026 20:52

shijiesheng merged commit 40e889d into cadence-workflow:master Aug 13, 2026
10 of 11 checks passed

Navigation Menu

Uh oh!

feat(ContextPropagator): Replace imperative set/unset context propagation with lexically-scoped runWithContext - #1074

feat(ContextPropagator): Replace imperative set/unset context propagation with lexically-scoped runWithContext#1074
shijiesheng merged 4 commits into
cadence-workflow:masterfrom
rlei:codex/cadence-scoped-propagation-run-with-context

rlei commented Aug 12, 2026

Uh oh!

Uh oh!

shijiesheng Aug 13, 2026

Uh oh!

rlei Aug 13, 2026

Uh oh!

Uh oh!

Uh oh!

gitar-bot Bot commented Aug 13, 2026

Overview

Failures

PR Title Validation Failure (confidence: high)

Summary

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

rlei commented Aug 12, 2026

Summary

Why

Test plan

Uh oh!

Uh oh!

shijiesheng Aug 13, 2026

Choose a reason for hiding this comment

Uh oh!

rlei Aug 13, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

gitar-bot Bot commented Aug 13, 2026

Overview

Failures

PR Title Validation Failure (confidence: high)

Summary

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants