Skip to content

Commit f65d62e

Browse files
authored
improvement(async): improve error capturing for asynchronous workflow executions (#1808)
* improvement(async): improve error capturing for asynchronous workflow executions * surface more erros * added more logs * fix failing tests * ack DB comments
1 parent 670e63c commit f65d62e

File tree

8 files changed

+467
-211
lines changed

8 files changed

+467
-211
lines changed

apps/sim/app/api/chat/[identifier]/route.ts

Lines changed: 108 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@ import { db } from '@sim/db'
22
import { chat, workflow, workspace } from '@sim/db/schema'
33
import { eq } from 'drizzle-orm'
44
import { type NextRequest, NextResponse } from 'next/server'
5+
import { v4 as uuidv4 } from 'uuid'
56
import { createLogger } from '@/lib/logs/console/logger'
7+
import { LoggingSession } from '@/lib/logs/execution/logging-session'
68
import { ChatFiles } from '@/lib/uploads'
79
import { generateRequestId } from '@/lib/utils'
810
import {
@@ -59,6 +61,29 @@ export async function POST(
5961

6062
if (!deployment.isActive) {
6163
logger.warn(`[${requestId}] Chat is not active: ${identifier}`)
64+
65+
const executionId = uuidv4()
66+
const loggingSession = new LoggingSession(
67+
deployment.workflowId,
68+
executionId,
69+
'chat',
70+
requestId
71+
)
72+
73+
await loggingSession.safeStart({
74+
userId: deployment.userId,
75+
workspaceId: '', // Will be resolved if needed
76+
variables: {},
77+
})
78+
79+
await loggingSession.safeCompleteWithError({
80+
error: {
81+
message: 'This chat is currently unavailable. The chat has been disabled.',
82+
stackTrace: undefined,
83+
},
84+
traceSpans: [],
85+
})
86+
6287
return addCorsHeaders(createErrorResponse('This chat is currently unavailable', 403), request)
6388
}
6489

@@ -96,6 +121,29 @@ export async function POST(
96121

97122
if (workflowResult.length === 0 || !workflowResult[0].isDeployed) {
98123
logger.warn(`[${requestId}] Workflow not found or not deployed: ${deployment.workflowId}`)
124+
125+
const executionId = uuidv4()
126+
const loggingSession = new LoggingSession(
127+
deployment.workflowId,
128+
executionId,
129+
'chat',
130+
requestId
131+
)
132+
133+
await loggingSession.safeStart({
134+
userId: deployment.userId,
135+
workspaceId: workflowResult[0]?.workspaceId || '',
136+
variables: {},
137+
})
138+
139+
await loggingSession.safeCompleteWithError({
140+
error: {
141+
message: 'Chat workflow is not available. The workflow is not deployed.',
142+
stackTrace: undefined,
143+
},
144+
traceSpans: [],
145+
})
146+
99147
return addCorsHeaders(createErrorResponse('Chat workflow is not available', 503), request)
100148
}
101149

@@ -109,6 +157,29 @@ export async function POST(
109157

110158
if (workspaceData.length === 0) {
111159
logger.error(`[${requestId}] Workspace not found for workflow ${deployment.workflowId}`)
160+
161+
const executionId = uuidv4()
162+
const loggingSession = new LoggingSession(
163+
deployment.workflowId,
164+
executionId,
165+
'chat',
166+
requestId
167+
)
168+
169+
await loggingSession.safeStart({
170+
userId: deployment.userId,
171+
workspaceId: workflowResult[0].workspaceId || '',
172+
variables: {},
173+
})
174+
175+
await loggingSession.safeCompleteWithError({
176+
error: {
177+
message: 'Workspace not found. Critical configuration error - please contact support.',
178+
stackTrace: undefined,
179+
},
180+
traceSpans: [],
181+
})
182+
112183
return addCorsHeaders(createErrorResponse('Workspace not found', 500), request)
113184
}
114185

@@ -140,16 +211,43 @@ export async function POST(
140211
executionId,
141212
}
142213

143-
const uploadedFiles = await ChatFiles.processChatFiles(
144-
files,
145-
executionContext,
146-
requestId,
147-
deployment.userId
148-
)
149-
150-
if (uploadedFiles.length > 0) {
151-
workflowInput.files = uploadedFiles
152-
logger.info(`[${requestId}] Successfully processed ${uploadedFiles.length} files`)
214+
try {
215+
const uploadedFiles = await ChatFiles.processChatFiles(
216+
files,
217+
executionContext,
218+
requestId,
219+
deployment.userId
220+
)
221+
222+
if (uploadedFiles.length > 0) {
223+
workflowInput.files = uploadedFiles
224+
logger.info(`[${requestId}] Successfully processed ${uploadedFiles.length} files`)
225+
}
226+
} catch (fileError: any) {
227+
logger.error(`[${requestId}] Failed to process chat files:`, fileError)
228+
229+
const fileLoggingSession = new LoggingSession(
230+
deployment.workflowId,
231+
executionId,
232+
'chat',
233+
requestId
234+
)
235+
236+
await fileLoggingSession.safeStart({
237+
userId: workspaceOwnerId,
238+
workspaceId: workflowResult[0].workspaceId || '',
239+
variables: {},
240+
})
241+
242+
await fileLoggingSession.safeCompleteWithError({
243+
error: {
244+
message: `File upload failed: ${fileError.message || 'Unable to process uploaded files'}`,
245+
stackTrace: fileError.stack,
246+
},
247+
traceSpans: [],
248+
})
249+
250+
throw fileError
153251
}
154252
}
155253

apps/sim/app/api/webhooks/trigger/[path]/route.test.ts

Lines changed: 13 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,13 @@ import {
1111
mockTriggerDevSdk,
1212
} from '@/app/api/__test-utils__/utils'
1313

14-
// Prefer mocking the background module to avoid loading Trigger.dev at all during tests
14+
vi.mock('@trigger.dev/sdk', () => ({
15+
tasks: {
16+
trigger: vi.fn().mockResolvedValue({ id: 'mock-task-id' }),
17+
},
18+
task: vi.fn().mockReturnValue({}),
19+
}))
20+
1521
vi.mock('@/background/webhook-execution', () => ({
1622
executeWebhookJob: vi.fn().mockResolvedValue({
1723
success: true,
@@ -22,6 +28,10 @@ vi.mock('@/background/webhook-execution', () => ({
2228
}),
2329
}))
2430

31+
vi.mock('@/background/logs-webhook-delivery', () => ({
32+
logsWebhookDelivery: {},
33+
}))
34+
2535
const hasProcessedMessageMock = vi.fn().mockResolvedValue(false)
2636
const markMessageAsProcessedMock = vi.fn().mockResolvedValue(true)
2737
const closeRedisConnectionMock = vi.fn().mockResolvedValue(undefined)
@@ -78,27 +88,19 @@ vi.mock('@/executor', () => ({
7888
})),
7989
}))
8090

81-
// Set up environment before any imports
8291
process.env.DATABASE_URL = 'postgresql://test:test@localhost:5432/test'
8392

84-
// Mock postgres dependencies
8593
vi.mock('drizzle-orm/postgres-js', () => ({
8694
drizzle: vi.fn().mockReturnValue({}),
8795
}))
8896

8997
vi.mock('postgres', () => vi.fn().mockReturnValue({}))
9098

91-
// The @sim/db mock is handled in test utils via mockExecutionDependencies()
92-
93-
// (removed duplicate utils mock - defined above with specific handlers)
94-
9599
describe('Webhook Trigger API Route', () => {
96100
beforeEach(() => {
97-
// Ensure a fresh module graph so per-test vi.doMock() takes effect before imports
98101
vi.resetModules()
99102
vi.clearAllMocks()
100103

101-
// Clear global mock data
102104
globalMockData.webhooks.length = 0
103105
globalMockData.workflows.length = 0
104106
globalMockData.schedules.length = 0
@@ -172,43 +174,22 @@ describe('Webhook Trigger API Route', () => {
172174
vi.clearAllMocks()
173175
})
174176

175-
// Removed: WhatsApp verification test has complex mock setup issues
176-
177-
/**
178-
* Test POST webhook with workflow execution
179-
* Verifies that a webhook trigger properly initiates workflow execution
180-
*/
181-
// TODO: Fix failing test - returns 500 instead of 200
182-
// it('should trigger workflow execution via POST', async () => { ... })
183-
184-
/**
185-
* Test 404 handling for non-existent webhooks
186-
*/
187177
it('should handle 404 for non-existent webhooks', async () => {
188-
// The global @sim/db mock already returns empty arrays, so findWebhookAndWorkflow will return null
189-
190-
// Create a mock request
191178
const req = createMockRequest('POST', { event: 'test' })
192179

193-
// Mock the path param
194180
const params = Promise.resolve({ path: 'non-existent-path' })
195181

196-
// Import the handler
197182
const { POST } = await import('@/app/api/webhooks/trigger/[path]/route')
198183

199-
// Call the handler
200184
const response = await POST(req, { params })
201185

202-
// Check response - expect 404 since our implementation returns 404 when webhook is not found
203186
expect(response.status).toBe(404)
204187

205-
// Parse the response body
206188
const text = await response.text()
207-
expect(text).toMatch(/not found/i) // Response should contain "not found" message
189+
expect(text).toMatch(/not found/i)
208190
})
209191

210192
describe('Generic Webhook Authentication', () => {
211-
// Mock billing and rate limiting dependencies
212193
beforeEach(() => {
213194
vi.doMock('@/lib/billing/core/subscription', () => ({
214195
getHighestPrioritySubscription: vi.fn().mockResolvedValue({
@@ -222,11 +203,7 @@ describe('Webhook Trigger API Route', () => {
222203
}))
223204
})
224205

225-
/**
226-
* Test generic webhook without authentication (default behavior)
227-
*/
228206
it('should process generic webhook without authentication', async () => {
229-
// Configure mock data
230207
globalMockData.webhooks.push({
231208
id: 'generic-webhook-id',
232209
provider: 'generic',
@@ -249,18 +226,16 @@ describe('Webhook Trigger API Route', () => {
249226
const { POST } = await import('@/app/api/webhooks/trigger/[path]/route')
250227
const response = await POST(req, { params })
251228

252-
// Should succeed (200 OK with webhook processed message)
253229
expect(response.status).toBe(200)
254230

255231
const data = await response.json()
256232
expect(data.message).toBe('Webhook processed')
257233
})
258234

259235
/**
260-
* Test generic webhook with Bearer token authentication (no custom header)
236+
* Test generic webhook with Bearer token authentication
261237
*/
262238
it('should authenticate with Bearer token when no custom header is configured', async () => {
263-
// Configure mock data with Bearer token
264239
globalMockData.webhooks.push({
265240
id: 'generic-webhook-id',
266241
provider: 'generic',
@@ -288,9 +263,6 @@ describe('Webhook Trigger API Route', () => {
288263
expect(response.status).toBe(200)
289264
})
290265

291-
/**
292-
* Test generic webhook with custom header authentication
293-
*/
294266
it('should authenticate with custom header when configured', async () => {
295267
globalMockData.webhooks.push({
296268
id: 'generic-webhook-id',
@@ -323,9 +295,6 @@ describe('Webhook Trigger API Route', () => {
323295
expect(response.status).toBe(200)
324296
})
325297

326-
/**
327-
* Test case insensitive Bearer token authentication
328-
*/
329298
it('should handle case insensitive Bearer token authentication', async () => {
330299
globalMockData.webhooks.push({
331300
id: 'generic-webhook-id',
@@ -369,9 +338,6 @@ describe('Webhook Trigger API Route', () => {
369338
}
370339
})
371340

372-
/**
373-
* Test case insensitive custom header authentication
374-
*/
375341
it('should handle case insensitive custom header authentication', async () => {
376342
globalMockData.webhooks.push({
377343
id: 'generic-webhook-id',
@@ -414,9 +380,6 @@ describe('Webhook Trigger API Route', () => {
414380
}
415381
})
416382

417-
/**
418-
* Test rejection of wrong Bearer token
419-
*/
420383
it('should reject wrong Bearer token', async () => {
421384
globalMockData.webhooks.push({
422385
id: 'generic-webhook-id',
@@ -442,9 +405,6 @@ describe('Webhook Trigger API Route', () => {
442405
expect(processWebhookMock).not.toHaveBeenCalled()
443406
})
444407

445-
/**
446-
* Test rejection of wrong custom header token
447-
*/
448408
it('should reject wrong custom header token', async () => {
449409
globalMockData.webhooks.push({
450410
id: 'generic-webhook-id',
@@ -474,9 +434,6 @@ describe('Webhook Trigger API Route', () => {
474434
expect(processWebhookMock).not.toHaveBeenCalled()
475435
})
476436

477-
/**
478-
* Test rejection of missing authentication
479-
*/
480437
it('should reject missing authentication when required', async () => {
481438
globalMockData.webhooks.push({
482439
id: 'generic-webhook-id',
@@ -498,9 +455,6 @@ describe('Webhook Trigger API Route', () => {
498455
expect(processWebhookMock).not.toHaveBeenCalled()
499456
})
500457

501-
/**
502-
* Test exclusivity - Bearer token should be rejected when custom header is configured
503-
*/
504458
it('should reject Bearer token when custom header is configured', async () => {
505459
globalMockData.webhooks.push({
506460
id: 'generic-webhook-id',
@@ -530,9 +484,6 @@ describe('Webhook Trigger API Route', () => {
530484
expect(processWebhookMock).not.toHaveBeenCalled()
531485
})
532486

533-
/**
534-
* Test wrong custom header name is rejected
535-
*/
536487
it('should reject wrong custom header name', async () => {
537488
globalMockData.webhooks.push({
538489
id: 'generic-webhook-id',
@@ -562,9 +513,6 @@ describe('Webhook Trigger API Route', () => {
562513
expect(processWebhookMock).not.toHaveBeenCalled()
563514
})
564515

565-
/**
566-
* Test authentication required but no token configured
567-
*/
568516
it('should reject when auth is required but no token is configured', async () => {
569517
globalMockData.webhooks.push({
570518
id: 'generic-webhook-id',

0 commit comments

Comments
 (0)