Skip to content

Commit 029a959

Browse files
icecrasher321claude
andcommitted
fix(execution): classify Chat stream drops and stop async launches writing a terminal pointer
Only useExecutionStream.execute wrapped a transport failure as SSEStreamInterruptedError; executeWorkflowWithFullLogging rethrew the raw TypeError, so a mid-run network drop on the Chat run-tool path took the generic branch: the tool reported "error" to Sim, the confirm route marked the row failed, and the pointer was cleared while the server kept running the workflow. The classifier is now one exported helper (toStreamInterruptedError) used by both execute paths and by the shared executor's post-acknowledgement catch, so the run tool reaches its recoverable branch, reports "background", keeps the pointer, and releases the run to the terminal reconnect. Async launches wrote the terminal execution pointer only so bindRunToolToExecution would find something after a reload, but an async run has no reconnectable stream, so any reconnect against that pointer 404'd into the same synthetic Run Error. The tab-local pending completion report already carries the execution id, so async launches no longer touch the pointer and recovery answers from the pending report first, falling back to the pointer only for a live run this tab was observing. The legacy clearExecutionPointerAfterReport flag is still honoured for pointers older clients left behind. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
1 parent c9c3651 commit 029a959

5 files changed

Lines changed: 245 additions & 71 deletions

File tree

‎apps/sim/app/workspace/[workspaceId]/w/[workflowId]/utils/workflow-execution-utils.test.ts‎

Lines changed: 86 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,10 @@ import {
1313
reconcileFinalBlockLogs,
1414
} from '@/app/workspace/[workspaceId]/w/[workflowId]/utils/workflow-execution-utils'
1515
import type { BlockLog } from '@/executor/types'
16-
import type { ExecutionStreamHttpError } from '@/hooks/use-execution-stream'
16+
import {
17+
type ExecutionStreamHttpError,
18+
SSEStreamInterruptedError,
19+
} from '@/hooks/use-execution-stream'
1720
import { useExecutionStore } from '@/stores/execution'
1821

1922
describe('workflow-execution-utils', () => {
@@ -61,6 +64,88 @@ describe('workflow-execution-utils', () => {
6164
expect(terminalConsoleMockFns.mockAddConsole).not.toHaveBeenCalled()
6265
})
6366

67+
describe('executeWorkflowWithFullLogging stream interruption', () => {
68+
/** A response whose server acknowledged the run and whose body then fails with `readError`. */
69+
function stubAcknowledgedStream(readError: unknown) {
70+
vi.stubGlobal(
71+
'fetch',
72+
vi.fn().mockResolvedValue({
73+
ok: true,
74+
status: 200,
75+
headers: { get: (name: string) => (name === 'X-Execution-Id' ? 'exec-server' : null) },
76+
body: {
77+
getReader: () => ({
78+
read: vi.fn().mockRejectedValue(readError),
79+
releaseLock: vi.fn(),
80+
}),
81+
},
82+
})
83+
)
84+
}
85+
86+
function stubExecutionStore() {
87+
const store = {
88+
getCurrentExecutionId: vi.fn(() => 'exec-server'),
89+
setActiveBlocks: vi.fn(),
90+
setBlockRunStatus: vi.fn(),
91+
setCurrentExecutionId: vi.fn(),
92+
setEdgeRunStatus: vi.fn(),
93+
setIsExecuting: vi.fn(),
94+
}
95+
vi.mocked(useExecutionStore.getState).mockReturnValue(store as any)
96+
return store
97+
}
98+
99+
it('classifies a transport drop after the server acknowledged the run as an interruption', async () => {
100+
/*
101+
* The Chat run tool only preserves a run for reconnect when it sees
102+
* SSEStreamInterruptedError; a raw TypeError from the body reader used to
103+
* fall through as a plain failure, reporting an error to Sim and tearing
104+
* the run down while the server kept executing it.
105+
*/
106+
const store = stubExecutionStore()
107+
stubAcknowledgedStream(new TypeError('network error'))
108+
109+
const promise = executeWorkflowWithFullLogging({
110+
workflowId: 'wf-1',
111+
executionId: 'exec-1',
112+
copilotToolCallId: 'tool-1',
113+
preserveExecutionOnTerminal: true,
114+
})
115+
116+
await expect(promise).rejects.toBeInstanceOf(SSEStreamInterruptedError)
117+
await expect(promise).rejects.toMatchObject({ executionId: 'exec-server' })
118+
expect(store.setCurrentExecutionId).toHaveBeenCalledWith('wf-1', 'exec-server')
119+
expect(store.setCurrentExecutionId).not.toHaveBeenCalledWith('wf-1', null)
120+
expect(store.setIsExecuting).not.toHaveBeenCalled()
121+
})
122+
123+
it.each([
124+
['a client abort', new DOMException('Aborted', 'AbortError')],
125+
[
126+
'the run tool stop reason, which aborts with a plain string',
127+
'user_stop:cancelRunToolExecution',
128+
],
129+
['a non-transport failure', new Error('Unexpected token in JSON')],
130+
])('rethrows %s unclassified', async (_label, readError) => {
131+
stubExecutionStore()
132+
stubAcknowledgedStream(readError)
133+
134+
const rejection = await executeWorkflowWithFullLogging({
135+
workflowId: 'wf-1',
136+
executionId: 'exec-1',
137+
preserveExecutionOnTerminal: true,
138+
}).then(
139+
() => {
140+
throw new Error('expected the stream failure to reject')
141+
},
142+
(error: unknown) => error
143+
)
144+
145+
expect(rejection).toBe(readError)
146+
})
147+
})
148+
64149
describe('createBlockEventHandlers', () => {
65150
it('skips duplicate block start rows during reconnect replay', () => {
66151
terminalConsoleMockFns.mockAddConsole({

‎apps/sim/app/workspace/[workspaceId]/w/[workflowId]/utils/workflow-execution-utils.ts‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { createLogger } from '@sim/logger'
2-
import { toError } from '@sim/utils/errors'
2+
import { getErrorMessage, toError } from '@sim/utils/errors'
33
import { generateId } from '@sim/utils/id'
44
import { isPlainRecord } from '@sim/utils/object'
55
import { normalizeWorkflowEdgeSourceHandle } from '@sim/workflow-types/workflow'
@@ -19,6 +19,7 @@ import {
1919
processSSEStream,
2020
SSEEventHandlerError,
2121
SSEStreamInterruptedError,
22+
toStreamInterruptedError,
2223
} from '@/hooks/use-execution-stream'
2324
import { useExecutionStore } from '@/stores/execution'
2425
import type { ConsoleEntry, ConsoleUpdate } from '@/stores/terminal'
@@ -1221,6 +1222,20 @@ export async function executeWorkflowWithFullLogging(
12211222
'CopilotExecution'
12221223
)
12231224
} catch (error) {
1225+
const interrupted = toStreamInterruptedError(
1226+
error,
1227+
executionIdRef.current,
1228+
'Execution stream interrupted before a terminal event was received'
1229+
)
1230+
if (interrupted) {
1231+
logger.warn('Execution stream interrupted; preserving execution for reconnect', {
1232+
workflowId: wfId,
1233+
executionId: executionIdRef.current,
1234+
error: getErrorMessage(error),
1235+
})
1236+
preserveExecutionForRecovery = true
1237+
throw interrupted
1238+
}
12241239
if (error instanceof SSEEventHandlerError || error instanceof SSEStreamInterruptedError) {
12251240
preserveExecutionForRecovery = true
12261241
}

‎apps/sim/hooks/use-execution-stream.ts‎

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,21 @@ function isRecoverableStreamError(error: any): boolean {
7979
)
8080
}
8181

82+
/**
83+
* Wraps a transport failure that cut a live execution stream before its
84+
* terminal event, so every consumer of a live stream classifies interruptions
85+
* the same way and recovery code can rely on one error type. Returns null for
86+
* client aborts and for anything that is not a transport failure.
87+
*/
88+
export function toStreamInterruptedError(
89+
error: unknown,
90+
executionId: string | undefined,
91+
message: string
92+
): SSEStreamInterruptedError | null {
93+
if (!isRecoverableStreamError(error)) return null
94+
return new SSEStreamInterruptedError(message, executionId, error)
95+
}
96+
8297
/**
8398
* Processes SSE events from a response body and invokes appropriate callbacks.
8499
* Exported for use by standalone (non-hook) execution paths like executeWorkflowWithFullLogging.
@@ -318,16 +333,17 @@ export function useExecutionStream() {
318333
logger.info('Execution stream disconnected (page unload or abort)')
319334
return
320335
}
321-
if (isRecoverableStreamError(error)) {
336+
const interrupted = toStreamInterruptedError(
337+
error,
338+
serverExecutionId,
339+
'Execution stream interrupted before a terminal event was received'
340+
)
341+
if (interrupted) {
322342
logger.warn('Execution stream interrupted; preserving execution for reconnect', {
323343
executionId: serverExecutionId,
324344
error: error.message,
325345
})
326-
throw new SSEStreamInterruptedError(
327-
'Execution stream interrupted before a terminal event was received',
328-
serverExecutionId,
329-
error
330-
)
346+
throw interrupted
331347
}
332348
logger.error('Execution stream error:', error)
333349
if (!(error instanceof SSEEventHandlerError)) {
@@ -423,16 +439,17 @@ export function useExecutionStream() {
423439
logger.info('Run-from-block stream disconnected (page unload or abort)')
424440
return
425441
}
426-
if (isRecoverableStreamError(error)) {
442+
const interrupted = toStreamInterruptedError(
443+
error,
444+
serverExecutionId,
445+
'Run-from-block stream interrupted before a terminal event was received'
446+
)
447+
if (interrupted) {
427448
logger.warn('Run-from-block stream interrupted; preserving execution for reconnect', {
428449
executionId: serverExecutionId,
429450
error: error.message,
430451
})
431-
throw new SSEStreamInterruptedError(
432-
'Run-from-block stream interrupted before a terminal event was received',
433-
serverExecutionId,
434-
error
435-
)
452+
throw interrupted
436453
}
437454
logger.error('Run-from-block execution error:', error)
438455
if (!(error instanceof SSEEventHandlerError)) {

‎apps/sim/lib/copilot/tools/client/run-tool-execution.test.ts‎

Lines changed: 73 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -185,32 +185,43 @@ describe('run tool execution cancellation', () => {
185185
expect(isRunToolActiveForWorkflow('wf-1')).toBe(false)
186186
})
187187

188-
it('releases an interrupted run to reconnect subscribers only after giving up ownership', async () => {
189-
const ownedAtRelease: boolean[] = []
190-
const listener = vi.fn((workflowId: string) => {
191-
ownedAtRelease.push(isRunToolActiveForWorkflow(workflowId))
192-
})
193-
const unsubscribe = subscribeToRunToolRelease(listener)
194-
executeWorkflowWithFullLogging.mockRejectedValueOnce(
195-
new MockSSEEventHandlerError('Block handler failed on event 7', 'exec-1')
196-
)
197-
198-
try {
199-
executeRunToolOnClient('tool-1', 'run_workflow', { workflowId: 'wf-1' })
200-
await vi.waitFor(() => expect(listener).toHaveBeenCalledWith('wf-1'))
201-
202-
expect(listener).toHaveBeenCalledTimes(1)
203-
expect(ownedAtRelease).toEqual([false])
204-
expect(setIsExecuting).toHaveBeenCalledWith('wf-1', false)
205-
expect(setCurrentExecutionId).toHaveBeenCalledWith('wf-1', null)
206-
expect(setIsExecuting.mock.invocationCallOrder.at(-1)).toBeLessThan(
207-
listener.mock.invocationCallOrder[0]
208-
)
209-
expect(clearExecutionPointer).not.toHaveBeenCalled()
210-
} finally {
211-
unsubscribe()
188+
it.each([
189+
['handler', new MockSSEEventHandlerError('Block handler failed on event 7', 'exec-1')],
190+
['transport', new MockSSEStreamInterruptedError('Execution stream interrupted', 'exec-1')],
191+
])(
192+
'releases a run whose stream was cut by a %s failure only after giving up ownership',
193+
async (_kind, interruption) => {
194+
const ownedAtRelease: boolean[] = []
195+
const listener = vi.fn((workflowId: string) => {
196+
ownedAtRelease.push(isRunToolActiveForWorkflow(workflowId))
197+
})
198+
const unsubscribe = subscribeToRunToolRelease(listener)
199+
executeWorkflowWithFullLogging.mockRejectedValueOnce(interruption)
200+
201+
try {
202+
executeRunToolOnClient('tool-1', 'run_workflow', { workflowId: 'wf-1' })
203+
await vi.waitFor(() => expect(listener).toHaveBeenCalledWith('wf-1'))
204+
205+
expect(listener).toHaveBeenCalledTimes(1)
206+
expect(ownedAtRelease).toEqual([false])
207+
expect(setIsExecuting).toHaveBeenCalledWith('wf-1', false)
208+
expect(setCurrentExecutionId).toHaveBeenCalledWith('wf-1', null)
209+
expect(setIsExecuting.mock.invocationCallOrder.at(-1)).toBeLessThan(
210+
listener.mock.invocationCallOrder[0]
211+
)
212+
expect(clearExecutionPointer).not.toHaveBeenCalled()
213+
expect(fetch).toHaveBeenCalledWith(
214+
'/api/copilot/confirm',
215+
expect.objectContaining({
216+
body: expect.stringContaining('"status":"background"'),
217+
})
218+
)
219+
expect(vi.mocked(fetch).mock.calls[0][1]?.body).toContain('"executionId":"exec-1"')
220+
} finally {
221+
unsubscribe()
222+
}
212223
}
213-
})
224+
)
214225

215226
it('does not release a run it observed to completion, even when the report fails', async () => {
216227
const listener = vi.fn()
@@ -331,12 +342,11 @@ describe('run tool execution cancellation', () => {
331342
expect(fetchMock.mock.calls[1][0]).toBe('/api/copilot/confirm')
332343
expect(fetchMock.mock.calls[1][1]?.body).toContain('"status":"background"')
333344
expect(fetchMock.mock.calls[1][1]?.body).toContain('"executionId":"exec-async"')
334-
expect(saveExecutionPointer).toHaveBeenCalledWith({
335-
workflowId: 'wf-1',
336-
executionId: 'exec-async',
337-
lastEventId: 0,
338-
})
339-
expect(clearExecutionPointer).toHaveBeenCalledWith('wf-1')
345+
// An async run has no reconnectable stream, so it must never leave the
346+
// terminal a pointer that a reconnect would 404 against.
347+
expect(saveExecutionPointer).not.toHaveBeenCalled()
348+
expect(clearExecutionPointer).not.toHaveBeenCalled()
349+
expect(window.sessionStorage.getItem('sim:copilot:run-tool-completion:tool-async')).toBeNull()
340350
})
341351

342352
it('recovers a queued async launch by re-reporting it without enqueueing again', async () => {
@@ -362,11 +372,10 @@ describe('run tool execution cancellation', () => {
362372

363373
await vi.waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(6))
364374
await vi.waitFor(() => expect(isRunToolActiveForId('tool-recover-async')).toBe(false))
365-
loadExecutionPointer.mockResolvedValueOnce({
366-
workflowId: 'wf-1',
367-
executionId: 'exec-recover-async',
368-
lastEventId: 0,
369-
})
375+
expect(saveExecutionPointer).not.toHaveBeenCalled()
376+
expect(
377+
window.sessionStorage.getItem('sim:copilot:run-tool-completion:tool-recover-async')
378+
).toContain('"executionId":"exec-recover-async"')
370379

371380
await expect(bindRunToolToExecution('tool-recover-async', 'wf-1')).resolves.toBe(true)
372381

@@ -377,6 +386,34 @@ describe('run tool execution cancellation', () => {
377386
expect(
378387
fetchMock.mock.calls.filter(([url]) => url === '/api/workflows/wf-1/execute')
379388
).toHaveLength(1)
389+
expect(clearExecutionPointer).not.toHaveBeenCalled()
390+
expect(
391+
window.sessionStorage.getItem('sim:copilot:run-tool-completion:tool-recover-async')
392+
).toBeNull()
393+
})
394+
395+
it('cleans up the terminal pointer an earlier client left for an async launch', async () => {
396+
const fetchMock = vi.fn().mockResolvedValue({ ok: true })
397+
vi.stubGlobal('fetch', fetchMock)
398+
loadExecutionPointer.mockResolvedValueOnce({
399+
workflowId: 'wf-1',
400+
executionId: 'exec-legacy-async',
401+
lastEventId: 0,
402+
})
403+
window.sessionStorage.setItem(
404+
'sim:copilot:run-tool-completion:tool-legacy-async',
405+
JSON.stringify({
406+
status: 'background',
407+
executionId: 'exec-legacy-async',
408+
clearExecutionPointerAfterReport: true,
409+
})
410+
)
411+
412+
await expect(bindRunToolToExecution('tool-legacy-async', 'wf-1')).resolves.toBe(true)
413+
414+
expect(fetchMock).toHaveBeenCalledTimes(1)
415+
expect(fetchMock.mock.calls[0][1]?.body).toContain('"status":"background"')
416+
expect(fetchMock.mock.calls[0][1]?.body).toContain('"executionId":"exec-legacy-async"')
380417
expect(clearExecutionPointer).toHaveBeenCalledWith('wf-1')
381418
})
382419

0 commit comments

Comments
 (0)