Fix pauseLiveUpdate logic and remove currentEventHistory store#3404
Fix pauseLiveUpdate logic and remove currentEventHistory store#3404Alex-Tideman wants to merge 2 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Alex-Tideman
changed the title
| } | ||
|
|
||
| $fullEventHistory = await fetchAllEvents({ | ||
| const events = await fetchAllEvents({ |
There was a problem hiding this comment.
Do we maybe want a try/catch here now? Same with getWorkflowMetadata above? Something that ignores the abort errors but rethrows anything else?
| let workflowId = $derived(page.params.workflow); | ||
| let runId = $derived(page.params.run); | ||
| let showJson = $derived(page.url.searchParams.has('json')); | ||
| let pauseLiveUpdates = $derived(parseEventFilterParams(page.url).refresh_off); |
There was a problem hiding this comment.
Looks like we're still just setting $pauseLiveUpdates = false in workflow-hsitory-layout.svelte. Do we now need to update to something like this ⬇️ ?
if (isNotPending && $pauseLiveUpdates) {
updateEventFilterParams(page.url, { refresh_off: false }, goto);
}| workflowRunController.abort(); | ||
| } | ||
| untrack(() => { | ||
| getOnlyWorkflowWithPendingActivities(refreshValue, pause); |
There was a problem hiding this comment.
After pause (and abort) and then unpausing are we actually starting polling again? Or do we need to re-trigger getWorkflowAndEventHistory after it was paused and then unpaused?
In the network tab, it doesn't look the /history is being called again (just the workflow summary endpoint).
| @@ -114,6 +110,7 @@ | |||
| $workflowRun = { ...$workflowRun, workflow, workers, workersLoaded: true }; | |||
|
|
|||
| workflowRunController = new AbortController(); | |||
There was a problem hiding this comment.
Not sure if this scenario is any real cause for concern, but probably best practice to abort any existing controller before constructing a new one
| workflowRunController = new AbortController(); | |
| workflowRunController?.abort(); | |
| workflowRunController = new AbortController(); |
| $fullEventHistory = []; | ||
| if (workflowRunController) { | ||
| workflowRunController.abort(); | ||
| } |
There was a problem hiding this comment.
NIT: Abort and then clear.
| if (workflowRunController) { | |
| workflowRunController.abort(); | |
| } | |
| $fullEventHistory = []; |
| await mockWorkflowApis(page); | ||
| await page.goto(`${workflowUrl}?refresh_off=true`); | ||
|
|
||
| await expect(page.getByTestId('pause')).toContainText('Auto Refresh Off'); | ||
| await expect( | ||
| page | ||
| .getByTestId('event-summary-row') | ||
| .filter({ hasText: 'Workflow Execution Started' }) | ||
| .first(), | ||
| ).toBeVisible(); |
There was a problem hiding this comment.
NIT: Could we also test for the mid-flight toggle path? Maybe something like this ⬇️?
| await mockWorkflowApis(page); | |
| await page.goto(`${workflowUrl}?refresh_off=true`); | |
| await expect(page.getByTestId('pause')).toContainText('Auto Refresh Off'); | |
| await expect( | |
| page | |
| .getByTestId('event-summary-row') | |
| .filter({ hasText: 'Workflow Execution Started' }) | |
| .first(), | |
| ).toBeVisible(); | |
| await mockWorkflowApis(page); | |
| const longPollPromise = page.waitForRequest( | |
| (req) => req.url().includes('waitNewEvent=true'), | |
| { timeout: 2000 }, | |
| ).catch(() => null); | |
| await page.goto(`${workflowUrl}?refresh_off=true`); | |
| await expect(page.getByTestId('pause')).toContainText('Auto Refresh Off'); | |
| await expect( | |
| page | |
| .getByTestId('event-summary-row') | |
| .filter({ hasText: 'Workflow Execution Started' }) | |
| .first(), | |
| ).toBeVisible(); | |
| const eventRows = await page.getByTestId('event-summary-row').count(); | |
| expect(eventRows).toBeGreaterThan(1); | |
| expect(await longPollPromise).toBeNull(); |
2 participants
Description & motivation 💭
This PR updates workflow run loading so live polling respects pauseLiveUpdates.
When live updates are paused, workflow history and metadata requests are made without the live polling abort signal, which prevents the event history endpoint from using
waitNewEvent=true. The layout also aborts any existing live polling controller when pause is enabled so an already-running long poll does not continue in the background.It also removes currentEventHistory store: currentEventHistory was a workaround for the older behavior: keep the rendered event list frozen while fullEventHistory continued to update in the background. Now the fix is at the source: when pauseLiveUpdates is true, live polling is not started, and any in-flight polling controller is aborted. With the guard:
if (signal?.aborted) return;
The aborted fetch no longer writes [] back into $fullEventHistory.
Why
The history page correctly stopped updating when
pauseLiveUpdateswas enabled, but an in-flight workflow run poll could still continue after the user paused live updates. This caused background history refetch behavior even though the UI indicated auto refresh was off. Also, if a user refreshed the page with refresh_off=true, the history would not load, showing only pending events.This change makes the pause state apply consistently to workflow details, metadata, and event history loading.
/history?refresh_off=truestill loads initial event history.paused.
Screenshots (if applicable) 📸
Design Considerations 🎨
Testing 🧪
Pause Live Updates during a running workflow and ensure no new events render and the workflow doesn't update.
Pause Live Updates during a running workflow and then unpause and ensur new events render and the workflow does update.
Pause Live Updates during a running workflow, wait, and refresh the page. Ensure new events are rendered from the pause to the refresh, but ensure no new events render.
How was this tested 👻
Steps for others to test: 🚶🏽♂️🚶🏽♀️
Checklists
Draft Checklist
Merge Checklist
Issue(s) closed
Docs
Any docs updates needed?