Skip to content

Commit 58caadb

Browse files
committed
fix(tables): keep table cells working through lost database connections
1 parent 6c21f82 commit 58caadb

9 files changed

Lines changed: 990 additions & 57 deletions
Lines changed: 304 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,304 @@
1+
/**
2+
* What a table cell does when a database socket is reset underneath it, written from the ways it
3+
* has failed in production:
4+
*
5+
* - A reset on a read before the run crashed the task and left the cell `pending` for good.
6+
* - A reset on the read that advances the cascade stranded the row's remaining groups.
7+
* - Driver text, and sometimes SQL with bound values, was stored as the cell's visible error.
8+
*
9+
* The guard that must never regress runs the other way: a block has effects, so nothing here may
10+
* start a workflow twice. What a repeated state write does is proven against real PostgreSQL in
11+
* `lib/table/cell-write.integration.ts`.
12+
*/
13+
import { beforeEach, describe, expect, it, vi } from 'vitest'
14+
15+
const mocks = vi.hoisted(() => ({
16+
getTableById: vi.fn(),
17+
getRowById: vi.fn(),
18+
getRowSummaryById: vi.fn(),
19+
readDispatch: vi.fn(),
20+
writeWorkflowGroupState: vi.fn(),
21+
markWorkflowGroupPickedUp: vi.fn(),
22+
executeWorkflow: vi.fn(),
23+
loadDeployedWorkflowState: vi.fn(),
24+
preprocessExecution: vi.fn(),
25+
pickNextEligibleGroupForRow: vi.fn(),
26+
selectWorkflow: vi.fn(),
27+
}))
28+
29+
vi.mock('@sim/utils/helpers', async (importOriginal) => ({
30+
...(await importOriginal<typeof import('@sim/utils/helpers')>()),
31+
sleep: vi.fn(async () => {}),
32+
}))
33+
vi.mock('@sim/db', () => ({
34+
db: {
35+
select: () => ({ from: () => ({ where: () => ({ limit: mocks.selectWorkflow }) }) }),
36+
},
37+
}))
38+
vi.mock('@/lib/table/service', () => ({ getTableById: mocks.getTableById }))
39+
vi.mock('@/lib/table/rows/service', () => ({
40+
getRowById: mocks.getRowById,
41+
getRowSummaryById: mocks.getRowSummaryById,
42+
}))
43+
vi.mock('@/lib/table/rows/executions', () => ({ readStampedCapabilitySubject: vi.fn() }))
44+
vi.mock('@/lib/table/dispatcher', () => ({ readDispatch: mocks.readDispatch }))
45+
vi.mock('@/lib/table/workflow-columns', () => ({
46+
buildWorkflowGroupExecutionCorrelation: vi.fn(),
47+
pickNextEligibleGroupForRow: mocks.pickNextEligibleGroupForRow,
48+
stashCellContextForResume: vi.fn(),
49+
}))
50+
vi.mock('@/lib/table/cell-write', () => ({
51+
buildCancelledExecution: vi.fn(),
52+
writeWorkflowGroupState: vi.fn(),
53+
writeWorkflowGroupStateDurably: mocks.writeWorkflowGroupState,
54+
markWorkflowGroupPickedUp: mocks.markWorkflowGroupPickedUp,
55+
createWorkflowCellProgressWriter: () => ({
56+
onBlockStart: vi.fn(),
57+
onBlockComplete: vi.fn(),
58+
finish: vi.fn(async () => {}),
59+
getEventOutputs: () => ({}),
60+
getPendingDataPatch: () => ({ out: 'value' }),
61+
getPendingSecretProvenance: () => ({}),
62+
getBlockErrors: () => ({}),
63+
}),
64+
}))
65+
vi.mock('@/lib/workflows/executor/execute-workflow', () => ({
66+
executeWorkflow: mocks.executeWorkflow,
67+
}))
68+
vi.mock('@/lib/workflows/persistence/utils', () => ({
69+
loadDeployedWorkflowState: mocks.loadDeployedWorkflowState,
70+
}))
71+
vi.mock('@/lib/execution/preprocessing', () => ({
72+
preprocessExecution: mocks.preprocessExecution,
73+
}))
74+
vi.mock('@/lib/core/rate-limiter/rate-limiter', () => ({
75+
RateLimiter: class {
76+
checkRateLimitWithSubscription = vi.fn(async () => ({ allowed: true }))
77+
},
78+
}))
79+
vi.mock('@/lib/table/workflow-cell-result', () => ({
80+
classifyWorkflowCellTerminalResult: () => ({ status: 'completed', error: null }),
81+
}))
82+
83+
import { runRowCascadeLoop } from '@/background/workflow-column-execution'
84+
85+
const GROUP = { id: 'group-1', workflowId: 'workflow-1', outputs: [] }
86+
const NEXT_GROUP = { id: 'group-2', workflowId: 'workflow-2', outputs: [] }
87+
const TABLE = {
88+
id: 'table-1',
89+
schema: { columns: [], workflowGroups: [GROUP, NEXT_GROUP] },
90+
}
91+
const ROW = { id: 'row-1', data: {}, executions: {} }
92+
const PAYLOAD = {
93+
tableId: 'table-1',
94+
tableName: 'Table',
95+
rowId: 'row-1',
96+
groupId: 'group-1',
97+
workflowId: 'workflow-1',
98+
workspaceId: 'workspace-1',
99+
executionId: 'execution-1',
100+
dispatchId: 'dispatch-1',
101+
executionTimeoutMs: 60_000,
102+
capabilityGovernedUserId: null,
103+
billingAttribution: {
104+
actorUserId: 'user-1',
105+
workspaceId: 'workspace-1',
106+
organizationId: null,
107+
billedAccountUserId: 'user-1',
108+
billingEntity: { type: 'user' as const, id: 'user-1' },
109+
billingPeriod: { start: '2026-07-01T00:00:00.000Z', end: '2026-08-01T00:00:00.000Z' },
110+
payerSubscription: null,
111+
},
112+
}
113+
114+
/** Drizzle's wrapper around a reset socket, the shape a reset read rejects with in production. */
115+
function resetRead(): Error {
116+
return Object.assign(
117+
new Error('Failed query: select "id" from "x"\nparams: secret-value', {
118+
cause: Object.assign(new Error('read ECONNRESET'), { code: 'ECONNRESET', syscall: 'read' }),
119+
}),
120+
{ query: 'select "id" from "x"', params: ['secret-value'] }
121+
)
122+
}
123+
124+
/** The bare driver error a reset during `BEGIN` rejects with: no Drizzle wrapper, no cause. */
125+
function resetBegin(): Error {
126+
const error = Object.assign(new Error('read ECONNRESET'), {
127+
code: 'ECONNRESET',
128+
syscall: 'read',
129+
})
130+
for (const property of ['query', 'parameters', 'args', 'types']) {
131+
Object.defineProperty(error, property, { value: undefined })
132+
}
133+
return error
134+
}
135+
136+
const writtenStatuses = () =>
137+
mocks.writeWorkflowGroupState.mock.calls.map(([, write]) => write.executionState.status)
138+
const lastWrite = () => mocks.writeWorkflowGroupState.mock.calls.at(-1)?.[1]
139+
140+
beforeEach(() => {
141+
vi.clearAllMocks()
142+
mocks.getTableById.mockResolvedValue(TABLE)
143+
mocks.getRowById.mockResolvedValue(ROW)
144+
mocks.getRowSummaryById.mockResolvedValue({ id: 'row-1', data: {} })
145+
mocks.readDispatch.mockResolvedValue({ id: 'dispatch-1', status: 'active' })
146+
mocks.writeWorkflowGroupState.mockResolvedValue('wrote')
147+
mocks.markWorkflowGroupPickedUp.mockResolvedValue('wrote')
148+
mocks.executeWorkflow.mockResolvedValue({ status: 'completed', success: true, output: {} })
149+
mocks.loadDeployedWorkflowState.mockResolvedValue({
150+
blocks: { start: { id: 'start', type: 'start_trigger', subBlocks: {}, outputs: {} } },
151+
edges: [],
152+
loops: {},
153+
parallels: {},
154+
})
155+
mocks.preprocessExecution.mockResolvedValue({
156+
success: true,
157+
actorUserId: 'user-1',
158+
actorSubscription: null,
159+
billingAttribution: PAYLOAD.billingAttribution,
160+
})
161+
mocks.pickNextEligibleGroupForRow.mockReturnValue(null)
162+
mocks.selectWorkflow.mockResolvedValue([
163+
{ id: 'workflow-1', userId: 'user-1', workspaceId: 'workspace-1', variables: {} },
164+
])
165+
})
166+
167+
describe('a table cell losing a database socket', () => {
168+
it('recovers a reset on the dispatch read and runs the cell', async () => {
169+
mocks.readDispatch.mockRejectedValueOnce(resetRead())
170+
171+
await runRowCascadeLoop(PAYLOAD)
172+
173+
expect(mocks.executeWorkflow).toHaveBeenCalledOnce()
174+
expect(writtenStatuses()).toEqual(['completed'])
175+
})
176+
177+
it('terminalizes the cell when the dispatch read never recovers, instead of stranding it', async () => {
178+
mocks.readDispatch.mockRejectedValue(resetRead())
179+
180+
await expect(runRowCascadeLoop(PAYLOAD)).resolves.toBeUndefined()
181+
182+
expect(mocks.executeWorkflow).not.toHaveBeenCalled()
183+
expect(lastWrite()?.executionState.status).toBe('error')
184+
})
185+
186+
it('fails the run loudly when the cell cannot be terminalized either', async () => {
187+
const lost = resetBegin()
188+
mocks.readDispatch.mockRejectedValue(resetRead())
189+
mocks.writeWorkflowGroupState.mockRejectedValue(lost)
190+
191+
await expect(runRowCascadeLoop(PAYLOAD)).rejects.toBe(lost)
192+
})
193+
194+
it('never restarts a workflow whose run lost its connection', async () => {
195+
mocks.executeWorkflow.mockRejectedValue(resetRead())
196+
197+
await runRowCascadeLoop(PAYLOAD)
198+
199+
expect(mocks.executeWorkflow).toHaveBeenCalledOnce()
200+
expect(lastWrite()?.executionState.status).toBe('error')
201+
})
202+
203+
it('marks the cell failed when its terminal write cannot be completed', async () => {
204+
mocks.writeWorkflowGroupState.mockRejectedValueOnce(resetBegin())
205+
206+
await runRowCascadeLoop(PAYLOAD)
207+
208+
expect(mocks.executeWorkflow).toHaveBeenCalledOnce()
209+
expect(writtenStatuses()).toEqual(['completed', 'error'])
210+
})
211+
212+
it('ends the carrier instead of starting the next group when a Stop owns the failed cell', async () => {
213+
mocks.readDispatch.mockRejectedValue(resetRead())
214+
mocks.writeWorkflowGroupState.mockResolvedValue('skipped')
215+
216+
await runRowCascadeLoop(PAYLOAD)
217+
218+
expect(mocks.pickNextEligibleGroupForRow).not.toHaveBeenCalled()
219+
expect(mocks.executeWorkflow).not.toHaveBeenCalled()
220+
})
221+
222+
it('records a pre-run failure on an enrichment cell under its enrichment id', async () => {
223+
mocks.readDispatch.mockRejectedValue(resetRead())
224+
mocks.getTableById.mockResolvedValue({
225+
...TABLE,
226+
schema: {
227+
columns: [],
228+
workflowGroups: [{ ...GROUP, type: 'enrichment', enrichmentId: 'work-email' }],
229+
},
230+
})
231+
232+
await runRowCascadeLoop(PAYLOAD)
233+
234+
expect(lastWrite()?.executionState).toMatchObject({ status: 'error', workflowId: 'work-email' })
235+
})
236+
237+
it('never stores a thrown message from before the cell reached a terminal state', async () => {
238+
mocks.readDispatch.mockRejectedValue(
239+
new Error(
240+
'Coalesced producer for "usage-gate:org-1:billed-user" did not settle within 5000ms'
241+
)
242+
)
243+
244+
await runRowCascadeLoop(PAYLOAD)
245+
246+
expect(lastWrite()?.executionState.error).not.toMatch(/usage-gate|org-1|billed-user/)
247+
})
248+
249+
it('advances the cascade past a reset on the read that picks the next group', async () => {
250+
mocks.getRowById
251+
.mockResolvedValueOnce(ROW)
252+
.mockRejectedValueOnce(resetRead())
253+
.mockResolvedValue(ROW)
254+
mocks.pickNextEligibleGroupForRow.mockReturnValueOnce(NEXT_GROUP).mockReturnValue(null)
255+
256+
await runRowCascadeLoop(PAYLOAD)
257+
258+
expect(mocks.executeWorkflow).toHaveBeenCalledTimes(2)
259+
})
260+
261+
it.each([
262+
['a reset read', resetRead],
263+
['a reset BEGIN', resetBegin],
264+
])('does not store driver text or SQL as the cell error after %s', async (_name, failure) => {
265+
mocks.executeWorkflow.mockRejectedValue(failure())
266+
267+
await runRowCascadeLoop(PAYLOAD)
268+
269+
const stored = lastWrite()?.executionState.error
270+
expect(stored).not.toMatch(/ECONNRESET|Failed query|select|secret-value/)
271+
})
272+
273+
it('keeps a block error that names the block over a database cause', async () => {
274+
const message = 'Agent 1: An internal error occurred while executing the block.'
275+
mocks.executeWorkflow.mockRejectedValue(new Error(message, { cause: resetRead() }))
276+
277+
await runRowCascadeLoop(PAYLOAD)
278+
279+
expect(lastWrite()?.executionState.error).toBe(message)
280+
})
281+
282+
it('does not tell the user to re-run a query that will fail the same way', async () => {
283+
mocks.executeWorkflow.mockRejectedValue(
284+
Object.assign(new Error('Failed query: update "x"\nparams: secret-value'), {
285+
query: 'update "x"',
286+
params: ['secret-value'],
287+
cause: Object.assign(new Error('unsupported Unicode escape'), { code: '22P05' }),
288+
})
289+
)
290+
291+
await runRowCascadeLoop(PAYLOAD)
292+
293+
const stored = lastWrite()?.executionState.error
294+
expect(stored).not.toMatch(/Failed query|secret-value|again/)
295+
})
296+
297+
it('keeps the message of a failure that has nothing to do with the database', async () => {
298+
mocks.executeWorkflow.mockRejectedValue(new Error('Workflow has no blocks'))
299+
300+
await runRowCascadeLoop(PAYLOAD)
301+
302+
expect(lastWrite()?.executionState.error).toBe('Workflow has no blocks')
303+
})
304+
})

0 commit comments

Comments
 (0)