fix: Preserve workflow task failure errors through illegal call validator rescues - #504
Open
AnushkaKulkarni wants to merge 1 commit into
Conversation
…ator rescues Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was changed
The illegal call tracer's validator rescue blocks (
IllegalCallTracer#initialize, both branches) now re-raise exceptions marked withInternal::WorkflowTaskFailureErrorinstead of flattening them into the", reason: #{e}"string of aWorkflow::NondeterminismError.Why?
The deadlock detector raises
DeadlockErrorasynchronously into the workflow thread (Timeout.timeoutinthread_pool.rb). If that raise lands while anIllegalWorkflowCallValidatorblock is executing — in our production incidents, theThread::Mutex#synchronizevalidator hit via protobuf'sObjectCacheduringScheduleActivityarg serialization — the tracer'srescue Exceptionconverts it into aNondeterminismErrorstring. TheWorkflowTaskFailureErrormarker from #467 is destroyed, soFuture#initializestores the error instead of failing the workflow task, and the activation completes with a partial command batch (a consumed-but-unscheduled activity id). Replay then fails permanently with[TMPRL1100] Activity id of scheduled event 'N' does not match activity id of activity command 'N-1'.With this change the marker class survives the validator path, the existing #467 machinery re-raises it out of the future, and the workflow task fails and retries — the same contract as when the deadlock lands outside a traced call.
Checklist
Closes [Bug] Illegal call tracer converts DeadlockError to NondeterminismError, bypassing #467 and committing partial command batches #503
How was this tested:
Added
test_deadlock_during_illegal_call_validator_fails_workflow_tasktodeadlock_test.rb: a custom validator raisesDeadlockError(standing in for the async watchdog raise landing mid-validator) inside aWorkflow::Futurethat is never waited. Pre-fix, the error is silently stored and no workflow task ever fails; post-fix, the task fails with the deadlock message (and the test asserts the failure is not the laundered"Cannot access ..."form). I could not run the suite locally (no Rust toolchain for the bridge), so relying on CI here; the equivalent behavior change has been validated in production via a monkeypatch on 1.5.0.N/A
Made with Cursor