Fix WorkflowReplayer for method-expression local activities - #2590
Open
chiliec wants to merge 1 commit into
Open
Fix WorkflowReplayer for method-expression local activities#2590chiliec wants to merge 1 commit into
chiliec wants to merge 1 commit into
Conversation
WorkflowReplayer could not replay workflows that execute a local activity referenced by a method expression, e.g. ExecuteLocalActivity(ctx, (*Activities).Foo). Such activities can never be registered on the replayer (it only exposes RegisterWorkflow), so the isMethod branch hit the registry miss and validated the unbound method expression, which carries an extra receiver argument, failing with "expected N args for function: Foo but found M". The IsReplayNamespace fallback that already existed for string-named local activities is now also applied to method-expression local activities, using the same dummy replayer function. Fixes temporalio#2589
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
Closes #2589.
WorkflowReplayercould not replay workflows that execute a local activity referenced by a method expression, e.g.Replaying such a workflow failed with:
Root cause
In
internal/workflow.go,ExecuteLocalActivitylooks the activity up in the registry on replay. The string-name branch (localCtx.fn == nil) already has anIsReplayNamespace(...)fallback that substitutes a dummy function when the activity is unregistered, so replay works. TheisMethodbranch had no such fallback: on a registry miss it calledvalidateFunctionArgs(localCtx.fn, args, ...)against the unbound method expression, which carries an extra receiver argument, and failed.WorkflowReplayeronly exposesRegisterWorkflow/RegisterDynamicWorkflow(activities can't be registered on it), so a method-expression local activity can never be made to hit the registry — the replay always failed.Fix
Extend the existing
IsReplayNamespacedummy-function fallback to theisMethodbranch, mirroring the string-name branch exactly. One branch added ininternal/workflow.go(+8 lines).Tests
Added
TestReplayWorkflowHistory_MethodLocalActivityininternal/internal_worker_test.go— a workflow that callsExecuteLocalActivity(ctx, (*replayLocalActivities).ReplayLocalActivity)replayed throughNewWorkflowReplayer, following the existingTestReplayWorkflowHistory_LocalActivitypattern.Validation (real results, Go 1.25.4)
Verified genuine RED→GREEN — reverting only the
internal/workflow.gofix (keeping the test) reproduces the exact reported error:Restoring the fix → green again. Full worker suite passes with no regressions:
go vet ./internal/clean; both changed.gofiles aregofmt-clean (I intentionally did not reformat unrelated lines). Added a### FixedCHANGELOG.md entry to satisfy the changelog check.First-time contributor here — happy to adjust the approach (the alternative discussed in the issue, adding
RegisterActivitytoWorkflowReplayer, is more involved and API-surface-changing; this mirrors the existing string-name fallback with minimal risk).