Repository navigation
fix(runtime): preserve deliveries when workflow versions are missing #25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
1b63e8a
3cc8f4d
1d0c966
37538b2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| --- | ||
| '@tanstack/workflow-runtime': patch | ||
| --- | ||
|
|
||
| Reject signal, approval, and timer deliveries for a run whose workflow version | ||
| is not registered, returning `version-mismatch` and leaving the run paused | ||
| instead of consuming the delivery and stranding the run. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -212,6 +212,14 @@ async function deliverSignal< | |
| args.minYieldRemainingMs, | ||
| ) | ||
| const leaseMs = resolveLeaseMs(config, args.leaseMs) | ||
| const prepared = await prepareRunDelivery(config, telemetry, args.runId) | ||
| if (prepared.rejected) { | ||
| span.setAttribute( | ||
| 'tanstack.workflow.result_kind', | ||
| prepared.rejected.kind, | ||
| ) | ||
| return prepared.rejected | ||
| } | ||
| const delivery = { | ||
| signalId: args.signalId, | ||
| stepId: args.stepId, | ||
|
|
@@ -236,7 +244,9 @@ async function deliverSignal< | |
| return result | ||
| } | ||
|
|
||
| const workflow = await loadWorkflow(config, delivered.run.workflowId) | ||
| const workflow = | ||
| prepared.workflow ?? | ||
| (await loadWorkflow(config, delivered.run.workflowId)) | ||
| const result = await driveClaimedRun(config, telemetry, { | ||
| workflow, | ||
| workflowId: delivered.run.workflowId, | ||
|
|
@@ -279,6 +289,8 @@ async function deliverSignalFromRuntime< | |
| args.minYieldRemainingMs, | ||
| ) | ||
| const leaseMs = resolveLeaseMs(config, args.leaseMs) | ||
| const prepared = await prepareRunDelivery(config, telemetry, args.runId) | ||
| if (prepared.rejected) return prepared.rejected | ||
| const delivery = { | ||
| signalId: args.signalId, | ||
| stepId: args.stepId, | ||
|
|
@@ -301,7 +313,8 @@ async function deliverSignalFromRuntime< | |
| return resultFromSignalDelivery(args.runId, delivered) | ||
| } | ||
|
|
||
| const workflow = await loadWorkflow(config, delivered.run.workflowId) | ||
| const workflow = | ||
| prepared.workflow ?? (await loadWorkflow(config, delivered.run.workflowId)) | ||
| return driveClaimedRun(config, telemetry, { | ||
| workflow, | ||
| workflowId: delivered.run.workflowId, | ||
|
|
@@ -338,6 +351,14 @@ async function deliverApproval< | |
| args.minYieldRemainingMs, | ||
| ) | ||
| const leaseMs = resolveLeaseMs(config, args.leaseMs) | ||
| const prepared = await prepareRunDelivery(config, telemetry, args.runId) | ||
| if (prepared.rejected) { | ||
| span.setAttribute( | ||
| 'tanstack.workflow.result_kind', | ||
| prepared.rejected.kind, | ||
| ) | ||
| return prepared.rejected | ||
| } | ||
| const delivered = await traceStoreOperation( | ||
| telemetry, | ||
| 'store.deliver_approval', | ||
|
|
@@ -355,7 +376,9 @@ async function deliverApproval< | |
| return result | ||
| } | ||
|
|
||
| const workflow = await loadWorkflow(config, delivered.run.workflowId) | ||
| const workflow = | ||
| prepared.workflow ?? | ||
| (await loadWorkflow(config, delivered.run.workflowId)) | ||
| const result = await driveClaimedRun(config, telemetry, { | ||
| workflow, | ||
| workflowId: delivered.run.workflowId, | ||
|
|
@@ -419,6 +442,8 @@ async function sweep<TWorkflows extends Record<string, WorkflowRegistration>>( | |
| const recovered: Array<WorkflowRuntimeRunResult> = [] | ||
| const scheduled: Array<WorkflowRuntimeRunResult> = [] | ||
| const timers: Array<WorkflowRuntimeRunResult> = [] | ||
| const seenTimers = new Set<string>() | ||
| let rejectedTimer = false | ||
| let deadlineReached = false | ||
|
|
||
| while (recovered.length < maxRecoveredRuns) { | ||
|
|
@@ -554,24 +579,30 @@ async function sweep<TWorkflows extends Record<string, WorkflowRegistration>>( | |
| deadlineReached = true | ||
| break | ||
| } | ||
|
|
||
| const dueTimers = await traceStoreOperation( | ||
| telemetry, | ||
| 'store.claim_due_timers', | ||
| { leaseOwner }, | ||
| () => | ||
| config.store.claimDueTimers({ | ||
| now, | ||
| limit: 1, | ||
| limit: maxTimers, | ||
| leaseOwner, | ||
| leaseMs, | ||
| }), | ||
| ) | ||
| const timer = dueTimers[0] | ||
| if (!timer) break | ||
|
|
||
| timers.push( | ||
| await deliverTimer(config, telemetry, { | ||
| let advanced = false | ||
| for (const timer of dueTimers) { | ||
| const timerKey = JSON.stringify([timer.runId, timer.signalId]) | ||
| if (seenTimers.has(timerKey)) continue | ||
| if (timers.length >= maxTimers) break | ||
| if (shouldStopForDeadline(deadline, minYieldRemainingMs)) { | ||
| deadlineReached = true | ||
| break | ||
| } | ||
| seenTimers.add(timerKey) | ||
| advanced = true | ||
| const result = await deliverTimer(config, telemetry, { | ||
| timer, | ||
| now, | ||
| leaseOwner, | ||
|
|
@@ -582,8 +613,11 @@ async function sweep<TWorkflows extends Record<string, WorkflowRegistration>>( | |
| minYieldRemainingMs, | ||
| yieldResumeAt: now + 1, | ||
| publish: args.publish, | ||
| }), | ||
| ) | ||
| }) | ||
| if (result.kind === 'version-mismatch') rejectedTimer = true | ||
| timers.push(result) | ||
|
Comment on lines
+617
to
+618
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| } | ||
| if (!advanced || deadlineReached) break | ||
| } | ||
|
|
||
| const result = { | ||
|
|
@@ -593,6 +627,7 @@ async function sweep<TWorkflows extends Record<string, WorkflowRegistration>>( | |
| summary: summarizeSweep(recovered, scheduled, timers), | ||
| deadlineReached, | ||
| remainingMayExist: | ||
| rejectedTimer || | ||
| deadlineReached || | ||
| recovered.length >= maxRecoveredRuns || | ||
| scheduled.length >= maxScheduledRuns || | ||
|
|
@@ -853,6 +888,66 @@ async function syncTimerFromRunState< | |
| ) | ||
| } | ||
|
|
||
| // Deliveries are checked before the store consumes them: once consumed, | ||
| // a run whose version this runtime can't replay would be left `running` | ||
| // with no lease and nothing awaited, and redelivery would be refused. | ||
| async function prepareRunDelivery< | ||
| TWorkflows extends Record<string, WorkflowRegistration>, | ||
| >( | ||
| config: WorkflowRuntimeConfig<TWorkflows>, | ||
| telemetry: WorkflowTelemetry, | ||
| runId: string, | ||
| ): Promise<{ | ||
| workflow?: AnyWorkflowDefinition | ||
| rejected?: WorkflowRuntimeRunResult | ||
| }> { | ||
| const run = await traceStoreOperation( | ||
| telemetry, | ||
| 'store.load_run', | ||
| { runId }, | ||
| () => config.store.loadRun(runId), | ||
| ) | ||
| if (!run) { | ||
| return { | ||
| rejected: { kind: 'not-found', runId, events: [], eventCount: 0 }, | ||
| } | ||
| } | ||
| if ( | ||
| run.status === 'finished' || | ||
| run.status === 'errored' || | ||
| run.status === 'aborted' | ||
| ) { | ||
| return {} | ||
| } | ||
|
|
||
| const state = await traceStoreOperation( | ||
| telemetry, | ||
| 'store.load_run_state', | ||
| { runId }, | ||
| () => config.store.loadRunState(runId), | ||
| ) | ||
| const version = state?.workflowVersion ?? run.workflowVersion | ||
| const workflow = await loadWorkflow(config, run.workflowId) | ||
| if ( | ||
| !version || | ||
| workflow.version === version || | ||
| workflow.previousVersions?.some((previous) => previous.version === version) | ||
| ) { | ||
|
Comment on lines
+930
to
+935
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| return { workflow } | ||
| } | ||
|
|
||
| return { | ||
| rejected: { | ||
| kind: 'version-mismatch', | ||
| runId, | ||
| workflowId: run.workflowId, | ||
| run, | ||
| events: [], | ||
| eventCount: 0, | ||
| }, | ||
| } | ||
| } | ||
|
|
||
| async function loadWorkflow< | ||
| TWorkflows extends Record<string, WorkflowRegistration>, | ||
| >( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -445,6 +445,7 @@ export type WorkflowRuntimeRunResultKind = | |
| | 'not-claimable' | ||
| | 'not-waiting' | ||
| | 'duplicate' | ||
| | 'version-mismatch' | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
|
||
| export interface WorkflowRuntimeRunResult { | ||
| kind: WorkflowRuntimeRunResultKind | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Script executed:
Repository: TanStack/workflow
Length of output: 3892
---- MODULE ReviewModel ---- VARIABLES phase, pending, owned, stop (* Two compatible timers are claimed as one batch. Assume claimDueTimers leases every returned timer. Consuming a removes its timer claim; its execution exhausts the deadline before b is attempted. *) Init == /\ phase = "claim" /\ pending = {"a", "b"} /\ owned = {} /\ stop = FALSE Claim == /\ phase = "claim" /\ owned' = pending /\ phase' = "process" /\ UNCHANGED <<pending, stop>> ConsumeFirst == /\ phase = "process" /\ pending' = {"b"} /\ owned' = {"b"} /\ stop' = TRUE /\ phase' = "check" Return == /\ phase = "check" /\ stop /\ phase' = "done" /\ UNCHANGED <<pending, owned, stop>> Next == Claim \/ ConsumeFirst \/ Return (* Required cleanup property, separate from the modeled implementation. *) ReturnedBatchReleased == phase = "done" => owned = {} (* TLC INIT Init NEXT Next INVARIANT ReturnedBatchReleased *) ====Length of output: 98
Script executed:
Repository: TanStack/workflow
Length of output: 39007
Script executed:
Repository: TanStack/workflow
Length of output: 39463
Release timer claims that the sweep does not process.
If the deadline expires during a batch, the sweep leaves the remaining timers leased. Other workers cannot wake those runs until the leases expire. Release unprocessed claims when the sweep stops, while preserving progress past rejected versions.