Skip to content

Commit ad2c4aa

Browse files
fix(async): avoid pre-starting queued execution logs
Let executeWorkflowCore own normal-path logging start so queued workflow and schedule executions persist the richer deployment and environment metadata instead of an earlier placeholder start record.
1 parent 29838ce commit ad2c4aa

File tree

3 files changed

+119
-25
lines changed

3 files changed

+119
-25
lines changed

apps/sim/background/async-preprocessing-correlation.test.ts

Lines changed: 119 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,28 @@
44

55
import { beforeEach, describe, expect, it, vi } from 'vitest'
66

7-
const { mockPreprocessExecution, mockTask, mockDbUpdate } = vi.hoisted(() => ({
7+
const {
8+
mockPreprocessExecution,
9+
mockTask,
10+
mockDbUpdate,
11+
mockExecuteWorkflowCore,
12+
mockLoggingSession,
13+
mockBlockExistsInDeployment,
14+
mockLoadDeployedWorkflowState,
15+
mockGetScheduleTimeValues,
16+
mockGetSubBlockValue,
17+
} = vi.hoisted(() => ({
818
mockPreprocessExecution: vi.fn(),
919
mockTask: vi.fn((config) => config),
1020
mockDbUpdate: vi.fn(() => ({
1121
set: vi.fn(() => ({ where: vi.fn().mockResolvedValue(undefined) })),
1222
})),
23+
mockExecuteWorkflowCore: vi.fn(),
24+
mockLoggingSession: vi.fn(),
25+
mockBlockExistsInDeployment: vi.fn(),
26+
mockLoadDeployedWorkflowState: vi.fn(),
27+
mockGetScheduleTimeValues: vi.fn(),
28+
mockGetSubBlockValue: vi.fn(),
1329
}))
1430

1531
vi.mock('@trigger.dev/sdk', () => ({ task: mockTask }))
@@ -30,11 +46,15 @@ vi.mock('@/lib/execution/preprocessing', () => ({
3046
}))
3147

3248
vi.mock('@/lib/logs/execution/logging-session', () => ({
33-
LoggingSession: vi.fn().mockImplementation(() => ({
34-
safeStart: vi.fn().mockResolvedValue(true),
35-
safeCompleteWithError: vi.fn().mockResolvedValue(undefined),
36-
markAsFailed: vi.fn().mockResolvedValue(undefined),
37-
})),
49+
LoggingSession: vi.fn().mockImplementation(() => {
50+
const instance = {
51+
safeStart: vi.fn().mockResolvedValue(true),
52+
safeCompleteWithError: vi.fn().mockResolvedValue(undefined),
53+
markAsFailed: vi.fn().mockResolvedValue(undefined),
54+
}
55+
mockLoggingSession(instance)
56+
return instance
57+
}),
3858
}))
3959

4060
vi.mock('@/lib/core/execution-limits', () => ({
@@ -52,7 +72,7 @@ vi.mock('@/lib/logs/execution/trace-spans/trace-spans', () => ({
5272
}))
5373

5474
vi.mock('@/lib/workflows/executor/execution-core', () => ({
55-
executeWorkflowCore: vi.fn(),
75+
executeWorkflowCore: mockExecuteWorkflowCore,
5676
wasExecutionFinalizedByCore: vi.fn().mockReturnValue(false),
5777
}))
5878

@@ -64,14 +84,14 @@ vi.mock('@/lib/workflows/executor/human-in-the-loop-manager', () => ({
6484
}))
6585

6686
vi.mock('@/lib/workflows/persistence/utils', () => ({
67-
blockExistsInDeployment: vi.fn(),
68-
loadDeployedWorkflowState: vi.fn(),
87+
blockExistsInDeployment: mockBlockExistsInDeployment,
88+
loadDeployedWorkflowState: mockLoadDeployedWorkflowState,
6989
}))
7090

7191
vi.mock('@/lib/workflows/schedules/utils', () => ({
7292
calculateNextRunTime: vi.fn(),
73-
getScheduleTimeValues: vi.fn(),
74-
getSubBlockValue: vi.fn(),
93+
getScheduleTimeValues: mockGetScheduleTimeValues,
94+
getSubBlockValue: mockGetSubBlockValue,
7595
}))
7696

7797
vi.mock('@/executor/execution/snapshot', () => ({
@@ -97,6 +117,94 @@ import { executeWorkflowJob } from './workflow-execution'
97117
describe('async preprocessing correlation threading', () => {
98118
beforeEach(() => {
99119
vi.clearAllMocks()
120+
mockLoadDeployedWorkflowState.mockResolvedValue({
121+
blocks: {
122+
'schedule-block': {
123+
type: 'schedule',
124+
},
125+
},
126+
edges: [],
127+
loops: {},
128+
parallels: {},
129+
deploymentVersionId: 'deployment-1',
130+
})
131+
mockGetSubBlockValue.mockReturnValue('daily')
132+
mockGetScheduleTimeValues.mockReturnValue({ timezone: 'UTC' })
133+
})
134+
135+
it('does not pre-start workflow logging before core execution', async () => {
136+
mockPreprocessExecution.mockResolvedValueOnce({
137+
success: true,
138+
actorUserId: 'actor-1',
139+
workflowRecord: {
140+
id: 'workflow-1',
141+
userId: 'owner-1',
142+
workspaceId: 'workspace-1',
143+
variables: {},
144+
},
145+
executionTimeout: {},
146+
})
147+
mockExecuteWorkflowCore.mockResolvedValueOnce({
148+
success: true,
149+
status: 'success',
150+
output: { ok: true },
151+
metadata: { duration: 10, userId: 'actor-1' },
152+
})
153+
154+
await executeWorkflowJob({
155+
workflowId: 'workflow-1',
156+
userId: 'user-1',
157+
triggerType: 'api',
158+
executionId: 'execution-1',
159+
requestId: 'request-1',
160+
})
161+
162+
const loggingSession = mockLoggingSession.mock.calls[0]?.[0]
163+
expect(loggingSession).toBeDefined()
164+
expect(loggingSession.safeStart).not.toHaveBeenCalled()
165+
expect(mockExecuteWorkflowCore).toHaveBeenCalledWith(
166+
expect.objectContaining({
167+
loggingSession,
168+
})
169+
)
170+
})
171+
172+
it('does not pre-start schedule logging before core execution', async () => {
173+
mockPreprocessExecution.mockResolvedValueOnce({
174+
success: true,
175+
actorUserId: 'actor-2',
176+
workflowRecord: {
177+
id: 'workflow-1',
178+
userId: 'owner-1',
179+
workspaceId: 'workspace-1',
180+
variables: {},
181+
},
182+
executionTimeout: {},
183+
})
184+
mockExecuteWorkflowCore.mockResolvedValueOnce({
185+
success: true,
186+
status: 'success',
187+
output: { ok: true },
188+
metadata: { duration: 12, userId: 'actor-2' },
189+
})
190+
191+
await executeScheduleJob({
192+
scheduleId: 'schedule-1',
193+
workflowId: 'workflow-1',
194+
executionId: 'execution-2',
195+
requestId: 'request-2',
196+
now: '2025-01-01T00:00:00.000Z',
197+
scheduledFor: '2025-01-01T00:00:00.000Z',
198+
})
199+
200+
const loggingSession = mockLoggingSession.mock.calls[0]?.[0]
201+
expect(loggingSession).toBeDefined()
202+
expect(loggingSession.safeStart).not.toHaveBeenCalled()
203+
expect(mockExecuteWorkflowCore).toHaveBeenCalledWith(
204+
expect.objectContaining({
205+
loggingSession,
206+
})
207+
)
100208
})
101209

102210
it('passes workflow correlation into preprocessing', async () => {

apps/sim/background/schedule-execution.ts

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -492,13 +492,6 @@ export async function executeScheduleJob(payload: ScheduleExecutionPayload) {
492492
throw new Error(`Workflow ${payload.workflowId} has no associated workspace`)
493493
}
494494

495-
await loggingSession.safeStart({
496-
userId: actorUserId,
497-
workspaceId: workflowRecord.workspaceId,
498-
variables: {},
499-
triggerData: { correlation },
500-
})
501-
502495
logger.info(`[${requestId}] Executing scheduled workflow ${payload.workflowId}`)
503496

504497
try {

apps/sim/background/workflow-execution.ts

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -95,13 +95,6 @@ export async function executeWorkflowJob(payload: WorkflowExecutionPayload) {
9595

9696
logger.info(`[${requestId}] Preprocessing passed. Using actor: ${actorUserId}`)
9797

98-
await loggingSession.safeStart({
99-
userId: actorUserId,
100-
workspaceId,
101-
variables: {},
102-
triggerData: { correlation },
103-
})
104-
10598
const workflow = preprocessResult.workflowRecord!
10699

107100
const metadata: ExecutionMetadata = {

0 commit comments

Comments
 (0)