Skip to content

Commit 8bed4ae

Browse files
committed
fix(execution): publish the terminal event when its batch is dropped
A budget-rejected batch carrying the terminal event dropped it with the rest of the backlog, so a run could end without ever publishing its final status even though the terminal event alone was small enough to fit. Retry it on its own once, gated on a budget rejection so a transient Redis error still leaves the batch queued for retry. Use the shared sleep helper in the tests instead of a raw setTimeout promise, which the utils enforcement gate rejects.
1 parent e2ea9c0 commit 8bed4ae

2 files changed

Lines changed: 51 additions & 2 deletions

File tree

apps/sim/lib/execution/event-buffer.test.ts

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
* @vitest-environment node
33
*/
44
import { redisConfigMockFns, resetRedisConfigMock } from '@sim/testing'
5+
import { sleep } from '@sim/utils/helpers'
56
import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest'
67
import type { ExecutionEventEntry } from '@/lib/execution/event-buffer'
78
import type { ExecutionEvent } from '@/lib/workflows/executor/execution-events'
@@ -490,13 +491,49 @@ describe('execution event buffer', () => {
490491
// Let writeTerminal's queued body actually enqueue its entry before the
491492
// in-flight flush resolves — otherwise the scheduled loop finds nothing left
492493
// to drain and the race under test never forms.
493-
await new Promise((resolve) => setTimeout(resolve, 5))
494+
await sleep(5)
494495
releaseFirstFlush?.()
495496
await terminalWrite
496497

497498
expect(observedTerminalStatuses).toContain('complete')
498499
})
499500

501+
/**
502+
* The backlog ahead of a terminal event can exceed the budget while the
503+
* terminal event itself still fits. Discarding it alongside the backlog would
504+
* leave readers without the final status for a run that could have published
505+
* one.
506+
*/
507+
it('still publishes the terminal event when the backlog ahead of it is dropped', async () => {
508+
mockRedis.incrby.mockResolvedValue(100)
509+
const observedTerminalStatuses: string[] = []
510+
mockRedis.eval.mockImplementation(async (script: string, ...args: unknown[]) => {
511+
if (!isFlushScript(script)) return [1, 'ok', 0, 0]
512+
const { terminalStatus, zaddArgs } = parseFlushEvalArgs(args)
513+
// Reject anything but a lone entry, standing in for a budget with only
514+
// enough headroom left for one small write.
515+
if (zaddArgs.length > 2) return [0, 'execution_redis_bytes', 64 * 1024 * 1024]
516+
observedTerminalStatuses.push(terminalStatus)
517+
for (let i = 0; i < zaddArgs.length; i += 2) {
518+
persistedEntries.push(JSON.parse(zaddArgs[i + 1] as string) as ExecutionEventEntry)
519+
}
520+
return [1, 1, 0]
521+
})
522+
523+
const writer = createExecutionEventWriter('exec-1')
524+
for (let i = 0; i < 5; i++) {
525+
await writer.write(makeEvent(`block-${i}`)).catch(() => {})
526+
}
527+
528+
await expect(writer.writeTerminal(makeEvent('terminal'), 'complete')).resolves.toMatchObject({
529+
executionId: 'exec-1',
530+
})
531+
expect(observedTerminalStatuses).toContain('complete')
532+
expect(
533+
persistedEntries.map((entry) => (entry.event.data as { blockId: string }).blockId)
534+
).toContain('terminal')
535+
})
536+
500537
/**
501538
* A terminal publish that threw must not be resurrected. Leaving the status
502539
* armed would let the next flush stamp the stream terminal for an event that
@@ -571,7 +608,7 @@ describe('execution event buffer', () => {
571608
const writer = createExecutionEventWriter('exec-1')
572609
await writer.write(makeEvent('a'))
573610

574-
await new Promise((resolve) => setTimeout(resolve, 60))
611+
await sleep(60)
575612

576613
await expect(writer.flush()).resolves.toBeUndefined()
577614
})

apps/sim/lib/execution/event-buffer.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1052,6 +1052,18 @@ export function createExecutionEventWriter(
10521052
let ok = false
10531053
try {
10541054
ok = await flushPending(false, status)
1055+
if (!ok && lastResourceLimitError && pendingTerminalStatus) {
1056+
// The batch carrying the terminal event exceeded the budget and was
1057+
// dropped. Those bytes are gone either way, and the terminal event is
1058+
// small enough to plausibly fit on its own — so give it one attempt
1059+
// alone rather than losing the run's final status with them. Gated on a
1060+
// budget rejection specifically: a transient Redis error leaves the batch
1061+
// queued for retry, and clearing it here would turn that into data loss.
1062+
const remaining = pending.filter((pendingEntry) => pendingEntry !== entry)
1063+
pending = [entry]
1064+
ok = await flushPending(false, status)
1065+
pending = pending.concat(remaining)
1066+
}
10551067
} catch (error) {
10561068
discardTerminalEntry(entry)
10571069
throw error

0 commit comments

Comments
 (0)