Skip to content

Commit 4ad9387

Browse files
fix(traces): make trace backfills bounded and resumable (#7974)
* wip * fix(traces): bound and resume trace backfill workers * fix(traces): use execution pool throughout backfill
1 parent 81cd643 commit 4ad9387

6 files changed

Lines changed: 1058 additions & 119 deletions

File tree

‎apps/sim/lib/execution/payloads/store.test.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,20 @@ describe('large execution payload store', () => {
248248
).rejects.toThrow('Failed to persist large execution value: storage down')
249249
})
250250

251+
it('preserves the database cause when metadata persistence fails after an upload', async () => {
252+
const cause = new Error('permission denied for table workspace_files')
253+
const error = new Error('Failed query', { cause })
254+
mockUploadFile.mockRejectedValueOnce(error)
255+
await expect(
256+
storeLargeValue({}, '{}', 2, {
257+
workspaceId: 'workspace-1',
258+
workflowId: 'workflow-1',
259+
executionId: 'execution-1',
260+
requireDurable: true,
261+
})
262+
).rejects.toMatchObject({ cause: error })
263+
})
264+
251265
it('materializes object-storage refs through the server helper', async () => {
252266
mockDownloadFile.mockResolvedValueOnce(Buffer.from(JSON.stringify({ ok: true }), 'utf8'))
253267

‎apps/sim/lib/execution/payloads/store.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,9 @@ async function persistValue(
9797
return fileInfo.key
9898
} catch (error) {
9999
if (context.requireDurable) {
100-
throw new Error(`Failed to persist large execution value: ${toError(error).message}`)
100+
throw new Error(`Failed to persist large execution value: ${toError(error).message}`, {
101+
cause: error,
102+
})
101103
}
102104
logger.warn('Failed to persist large execution value, keeping in memory only', {
103105
id,

‎apps/sim/lib/logs/execution/trace-store.test.ts‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,43 @@ beforeEach(() => {
5151
})
5252

5353
describe('execution data storage', () => {
54+
it('propagates the original storage failure for strict backfills', async () => {
55+
const cause = new Error('column "size_bytes" does not exist')
56+
const error = new Error('Failed query', { cause })
57+
storeLargeValueMock.mockRejectedValueOnce(error)
58+
59+
await expect(
60+
externalizeExecutionData({ traceSpans: [] }, CONTEXT, { throwOnError: true })
61+
).rejects.toBe(error)
62+
expect(mockLogger.warn).not.toHaveBeenCalled()
63+
})
64+
65+
it('rejects missing ownership before strict backfills write anything', async () => {
66+
await expect(
67+
externalizeExecutionData(
68+
{ traceSpans: [] },
69+
{ ...CONTEXT, userId: '' },
70+
{ throwOnError: true }
71+
)
72+
).rejects.toThrow('Trace storage requires workspaceId, workflowId, and userId')
73+
expect(storeLargeValueMock).not.toHaveBeenCalled()
74+
})
75+
76+
it('preserves inline completion data and logs the underlying database error', async () => {
77+
const data = { traceSpans: [] }
78+
storeLargeValueMock.mockRejectedValueOnce(
79+
new Error('Failed query\nparams: private-payload', {
80+
cause: new Error('permission denied for table workspace_files'),
81+
})
82+
)
83+
await expect(externalizeExecutionData(data, CONTEXT)).resolves.toBe(data)
84+
expect(mockLogger.warn).toHaveBeenCalledWith(expect.any(String), {
85+
executionId: CONTEXT.executionId,
86+
error: expect.objectContaining({ message: 'permission denied for table workspace_files' }),
87+
})
88+
expect(JSON.stringify(mockLogger.warn.mock.calls)).not.toContain('private-payload')
89+
})
90+
5491
it('keeps the trusted Copilot binding when an externalized payload is unavailable', async () => {
5592
const correlation = { copilotToolCallId: 'tool-call-1' }
5693
const ref = {

‎apps/sim/lib/logs/execution/trace-store.ts‎

Lines changed: 12 additions & 4 deletions
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 { describeError, toError } from '@sim/utils/errors'
33
import { isRecordLike, omit } from '@sim/utils/object'
44
import { isLargeValueRef } from '@/lib/execution/payloads/large-value-ref'
55
import { materializeLargeValueRef, storeLargeValue } from '@/lib/execution/payloads/store'
@@ -231,17 +231,24 @@ export function copyTraceSpansWithoutCosts(spans?: TraceSpan[]): TraceSpan[] | u
231231
*
232232
* On any failure (no scope, oversized, storage error) the original (already
233233
* cost-stripped) execution data is returned unchanged so the log is never lost.
234+
* Backfills pass `throwOnError` to stop instead of retaining inline data.
234235
*/
235236
export async function externalizeExecutionData(
236237
executionData: Record<string, unknown>,
237-
context: TraceStoreWriteContext
238+
context: TraceStoreWriteContext,
239+
options: { throwOnError?: boolean } = {}
238240
): Promise<Record<string, unknown>> {
239241
const { workspaceId, workflowId, executionId, userId } = context
240242
// workspaceId/workflowId build the storage key and can be null for
241243
// deleted-workflow rows. userId is type-guaranteed by TraceStoreWriteContext;
242244
// the falsy check is a defensive guard against an empty string. If any are
243245
// missing the durable write can't succeed, so keep the data inline.
244-
if (!workspaceId || !workflowId || !userId) return executionData
246+
if (!workspaceId || !workflowId || !userId) {
247+
if (options.throwOnError) {
248+
throw new Error('Trace storage requires workspaceId, workflowId, and userId')
249+
}
250+
return executionData
251+
}
245252

246253
try {
247254
const json = JSON.stringify(executionData)
@@ -266,9 +273,10 @@ export async function externalizeExecutionData(
266273
}
267274
return slim
268275
} catch (error) {
276+
if (options.throwOnError) throw error
269277
logger.warn('Failed to externalize execution data; keeping inline', {
270278
executionId,
271-
error: toError(error).message,
279+
error: describeError(error),
272280
})
273281
return executionData
274282
}

0 commit comments

Comments
 (0)