feat: workflow try helpers & visualizer support#4413
feat: workflow try helpers & visualizer support#4413NathanFlurry wants to merge 6 commits intomainfrom
Conversation
|
🚅 Deployed to the rivet-pr-4413 environment in rivet-frontend
|
|
Screenshots relevant to the workflow try UI changes:
|
PR Review: feat: workflow try helpers & visualizer supportOverall this is a well-structured PR that adds a genuinely useful safety valve for workflow error handling. The implementation is thoughtful and the test coverage is solid. Here are my findings: Core Logic Issues 1. ctx.try does not record a history entry executeTry calls appendName to construct a child location but never writes a history entry for the try scope itself. Replays therefore cannot distinguish inside a try block from normal nesting without relying purely on location prefixes. If joins/loops/steps all get history entries, a try-scope marker entry would make history self-consistent and remove the need for the synthetic render-tree inference in workflow-to-xyflow.ts. 2. Missing name registration after checkDuplicateName in executeTry executeTry calls this.checkDuplicateName(config.name) but unlike step never registers the name afterward. If the same try-scope name were reused in a replay, the second call would still pass the duplicate check. 3. Race winner replay: winnerValue is T or undefined but cast as T The old code guarded with winnerValue !== null. After the refactor winnerValue is typed T or undefined and returned with winnerValue as T. If a branch legitimately returns undefined this is fine, but the cast hides intent. A comment clarifying that hasWinner is the authoritative guard would help. 4. shouldCatchTryBlockFailure fallthrough for source: "block" is non-obvious DEFAULT_TRY_BLOCK_CATCH is ["step", "join", "race"]. When a RollbackError is thrown directly inside a try block (not via a step), failure.source becomes "block" and the function falls through to return effectiveCatch.includes("rollback"). This is correct (rollback is opt-in), but deserves an explicit source === "block" branch for clarity. Correctness / Edge Cases 5. parseStoredWorkflowError regex edge case with plain string throws The regex works correctly for Error: message but a plain thrown string containing a colon (e.g. "timeout: 5000ms") will be parsed with the first word as name. Minor, but worth documenting or hardening. 6. StepFailedError in mergeSchedulerYield vs shouldRethrowTryError mergeSchedulerYield handles StepFailedError, but shouldRethrowTryError lists StepFailedError in the rethrow set, so it never reaches mergeSchedulerYield via the try-step path -- only via join/race branch paths. The behavior is correct, but a short inline comment explaining this would help future readers. Visualizer Issues 7. Redundant branch in retry badge border logic The retry badge border has a three-way conditional where two arms produce the same value. The conditional isHandledFailure ? "#f59e0b" : isFailed ? "#ef4444" : "#f59e0b" can be simplified to isFailed && !isHandledFailure ? "#ef4444" : "#f59e0b". 8. Duration label: Tailwind class conflicts with inline right style In WorkflowNode, the duration span has className with right-3 (12px) AND an inline style with right: isFailed ? 44 : 12. These fight each other (inline wins due to specificity). Remove right-3 from className since the inline style fully controls the value. 9. translateFragment and parentId child nodes Nodes using React Flow parentId use parent-relative positioning and should not have their absolute positions adjusted by translateFragment. Verify that branch group children do not use parentId after the refactor -- the old makeChildNode set both parentId and extent: "parent" on them. Documentation Issues 10. try example in workflows.mdx uses any types The runPaymentFlow function uses ctx: any and blockCtx: any. The codebase notes that all TypeScript code blocks are typechecked during the website build. This should use WorkflowContextInterface from rivetkit/workflow, or add @nocheck to the fence if the import chain is too heavy for an inline snippet. Minor / Style
Summary The feature is solid and the test coverage is comprehensive. The main items worth addressing before merge are the missing history entry for try scopes (item 1), the any-typed doc example (item 10), and the CSS class conflict (item 8). The rest are minor correctness hardening or style nits. |
|
Updated workflow UI screenshots after the alignment polish: Full canvas: Try header close-up: Failed node close-up: |
NathanFlurry
changed the title
NathanFlurry
force-pushed
the
workflow-try-step-api
branch
from
000ae2a to
c58143b
Compare
1 participant
Description
Adds
ctx.tryStep()andctx.try()to the workflow engine and RivetKit wrapper so workflows can recover from terminal step, join, and race failures without swallowing scheduler control flow. It also updates the workflow visualizer to render named try scopes and handled failures, plus adds docs, stories, and integration coverage for the new behavior.Type of change
How Has This Been Tested?
pnpm exec vitest run tests/try.test.ts tests/join.test.ts tests/race.test.tsinrivetkit-typescript/packages/workflow-enginepnpm exec vitest run tests/driver-memory.test.ts -t "tryStep and try recover terminal workflow failures"inrivetkit-typescript/packages/rivetkitpnpm test workflow-to-xyflowinfrontendpnpm exec biome check src/components/actors/workflow/workflow-to-xyflow.ts src/components/actors/workflow/workflow-to-xyflow.test.ts src/components/actors/workflow/xyflow-nodes.tsx src/components/actors/workflow/workflow-visualizer.tsx src/components/actors/workflow/workflow-example-data.ts src/components/actors/workflow/xyflow-nodes.stories.tsxinfrontendChecklist: