Add e2e test for workflow failing when step exhausts max retries #436
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
When a step reaches max retries or encounters a FatalError, the workflow run is now properly updated to 'failed' status instead of staying in 'running' state. This prevents the workflow from being re-queued and hitting the same failed step indefinitely. Added test case for stepMaxRetriesWorkflow to verify the fix. Co-authored-by: pranaygp <[email protected]>
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
packages/core/e2e/e2e.test.ts
Outdated
| const run = await triggerWorkflow('stepMaxRetriesWorkflow', []); | ||
|
|
||
| // Wait for the workflow to complete (should fail) | ||
| await waitForRun(run.runId); |
There was a problem hiding this comment.
The test calls an undefined function waitForRun which will cause a ReferenceError at runtime.
View Details
📝 Patch Details
diff --git a/packages/core/e2e/e2e.test.ts b/packages/core/e2e/e2e.test.ts
index 41d3d65..79f240b 100644
--- a/packages/core/e2e/e2e.test.ts
+++ b/packages/core/e2e/e2e.test.ts
@@ -76,6 +76,25 @@ async function getWorkflowReturnValue(runId: string) {
}
}
+async function waitForRun(runId: string) {
+ // Poll the workflow run status until it completes (is no longer in 202 status)
+ // This is useful for tests that don't care about the return value, just that the workflow finishes
+ while (true) {
+ const url = new URL('/api/trigger', deploymentUrl);
+ url.searchParams.set('runId', runId);
+
+ const res = await fetch(url);
+
+ if (res.status === 202) {
+ // Workflow run is still running, so we need to wait and poll again
+ await new Promise((resolve) => setTimeout(resolve, 5_000));
+ continue;
+ }
+ // Workflow has completed, we're done
+ break;
+ }
+}
+
// NOTE: Temporarily disabling concurrent tests to avoid flakiness.
// TODO: Re-enable concurrent tests after conf when we have more time to investigate.
describe('e2e', () => {
Analysis
Undefined function waitForRun in stepMaxRetriesWorkflow test
What fails: The test stepMaxRetriesWorkflow in packages/core/e2e/e2e.test.ts calls an undefined function waitForRun(run.runId) on line 789 (now 808 after fix), causing a ReferenceError at runtime: waitForRun is not defined
How to reproduce:
cd packages/core
npm test e2e.test.ts -- stepMaxRetriesWorkflowResult: Test fails with ReferenceError: waitForRun is not defined because the function is called but never defined in the test file
Expected: The test should define a waitForRun helper function that polls the workflow API endpoint until the workflow reaches a terminal state (completed, failed, cancelled, etc.), following the same pattern as the existing getWorkflowReturnValue function already defined in the file
Solution: Added waitForRun function that polls /api/trigger endpoint with the runId parameter until the workflow completes (status is no longer 202), matching the established polling pattern used by getWorkflowReturnValue in the same file
The existing code already handles this correctly - when a step fails after max retries, the workflow properly fails. Added explicit test coverage to ensure this behavior is maintained. The workflow flow is: 1. Step fails 3x, gets marked as failed with fatal event 2. Workflow is queued and replays 3. During replay, step throws FatalError 4. If not caught, error bubbles to workflowEntrypoint 5. Workflow run is marked as failed This test ensures local world correctly handles step retry exhaustion. Co-authored-by: pranaygp <[email protected]>
|
@copilot So there's no code changes required at all? How about we at least add an e2e test to verify the "failed" run status. |
Added a test workflow (stepExhaustsRetriesWorkflow) that has a step which always throws a regular error. This verifies that when a step exhausts its max retries (default 3), the error bubbles up to the workflow function, and if unhandled, the workflow run enters a failed state with the appropriate error message. Co-authored-by: TooTallNate <[email protected]>
Added e2e test in 2964d3b. The test |
Adds e2e test coverage to verify that workflows properly enter a failed state when a step exhausts its max retries.
Background
The original issue reported that workflows remained in 'running' state indefinitely when a step failed after max retries. After investigation, the existing codebase behavior was found to be correct:
fatal: truein the eventChanges Made
Added e2e test coverage to verify this behavior:
stepExhaustsRetriesWorkflow): Contains a step that always throws a regular error, triggering retry exhaustionWorkflowRunFailedErrorfailedThis confirms the existing behavior is correct - when a step exhausts retries, the error bubbles up to the workflow function, and if unhandled, the workflow run properly enters a failed state.
View original Slack conversation
View original Slack conversation
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.