Skip to content

Commit 154b9d0

Browse files
TheodoreSpeaksTheodoreSpeaks
authored
fix(vm): categorize user or server side errors (#4283)
* fix(vm): categorize user or server side errors * recategorize function syntax errors as 4xx
1 parent c95ac3b commit 154b9d0

9 files changed

Lines changed: 192 additions & 27 deletions

File tree

‎apps/sim/app/api/function/execute/route.test.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ describe('Function Execute API Route', () => {
191191
const response = await POST(req)
192192
const data = await response.json()
193193

194-
if (response.status === 500) {
194+
if (response.status === 422 || response.status === 500) {
195195
expect(data.success).toBe(false)
196196
} else {
197197
const result = data.output?.result
@@ -504,7 +504,7 @@ describe('Function Execute API Route', () => {
504504
const response = await POST(req)
505505
const data = await response.json()
506506

507-
expect(response.status).toBe(500)
507+
expect(response.status).toBe(422)
508508
expect(data.success).toBe(false)
509509
expect(data.error).toBeTruthy()
510510
})
@@ -518,7 +518,7 @@ describe('Function Execute API Route', () => {
518518
const response = await POST(req)
519519
const data = await response.json()
520520

521-
expect(response.status).toBe(500)
521+
expect(response.status).toBe(422)
522522
expect(data.success).toBe(false)
523523
expect(data.error).toContain('Type Error')
524524
expect(data.error).toContain('Cannot read properties of null')
@@ -533,7 +533,7 @@ describe('Function Execute API Route', () => {
533533
const response = await POST(req)
534534
const data = await response.json()
535535

536-
expect(response.status).toBe(500)
536+
expect(response.status).toBe(422)
537537
expect(data.success).toBe(false)
538538
expect(data.error).toContain('Reference Error')
539539
expect(data.error).toContain('undefinedVariable is not defined')
@@ -548,7 +548,7 @@ describe('Function Execute API Route', () => {
548548
const response = await POST(req)
549549
const data = await response.json()
550550

551-
expect(response.status).toBe(500)
551+
expect(response.status).toBe(422)
552552
expect(data.success).toBe(false)
553553
expect(data.error).toContain('Custom error message')
554554
})
@@ -562,7 +562,7 @@ describe('Function Execute API Route', () => {
562562
const response = await POST(req)
563563
const data = await response.json()
564564

565-
expect(response.status).toBe(500)
565+
expect(response.status).toBe(422)
566566
expect(data.success).toBe(false)
567567
expect(data.error).toBeTruthy()
568568
})

‎apps/sim/app/api/function/execute/route.ts‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1088,9 +1088,12 @@ export const POST = withRouteHandler(async (req: NextRequest) => {
10881088
const executionTime = Date.now() - startTime
10891089

10901090
if (isolatedResult.error) {
1091-
logger.error(`[${requestId}] Function execution failed in isolated-vm`, {
1091+
const isSystemError = isolatedResult.error.isSystemError === true
1092+
const logFn = isSystemError ? logger.error.bind(logger) : logger.warn.bind(logger)
1093+
logFn(`[${requestId}] Function execution failed in isolated-vm`, {
10921094
error: isolatedResult.error,
10931095
executionTime,
1096+
isSystemError,
10941097
})
10951098

10961099
const ivmError = isolatedResult.error
@@ -1119,7 +1122,8 @@ export const POST = withRouteHandler(async (req: NextRequest) => {
11191122
resolvedCode
11201123
)
11211124

1122-
logger.error(`[${requestId}] Enhanced error details`, {
1125+
const detailLogFn = isSystemError ? logger.error.bind(logger) : logger.warn.bind(logger)
1126+
detailLogFn(`[${requestId}] Enhanced error details`, {
11231127
originalMessage: ivmError.message,
11241128
enhancedMessage: userFriendlyErrorMessage,
11251129
line: enhancedError.line,
@@ -1145,7 +1149,7 @@ export const POST = withRouteHandler(async (req: NextRequest) => {
11451149
stack: enhancedError.stack,
11461150
},
11471151
},
1148-
{ status: 500 }
1152+
{ status: isSystemError ? 500 : 422 }
11491153
)
11501154
}
11511155

‎apps/sim/app/api/workspaces/[id]/_preview/create-preview-route.ts‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { toError } from '@sim/utils/errors'
33
import { type NextRequest, NextResponse } from 'next/server'
44
import { getSession } from '@/lib/auth'
55
import { MAX_DOCUMENT_PREVIEW_CODE_BYTES } from '@/lib/execution/constants'
6-
import { runSandboxTask } from '@/lib/execution/sandbox/run-task'
6+
import { runSandboxTask, SandboxUserCodeError } from '@/lib/execution/sandbox/run-task'
77
import { verifyWorkspaceMembership } from '@/app/api/workflows/utils'
88
import type { SandboxTaskId } from '@/sandbox-tasks/registry'
99

@@ -83,6 +83,14 @@ export function createDocumentPreviewRoute(config: DocumentPreviewRouteConfig) {
8383
})
8484
} catch (err) {
8585
const message = toError(err).message
86+
if (err instanceof SandboxUserCodeError) {
87+
logger.warn(`${config.label} preview user code failed`, {
88+
error: message,
89+
errorName: err.name,
90+
workspaceId,
91+
})
92+
return NextResponse.json({ error: message, errorName: err.name }, { status: 422 })
93+
}
8694
logger.error(`${config.label} preview generation failed`, { error: message, workspaceId })
8795
return NextResponse.json({ error: message }, { status: 500 })
8896
}

‎apps/sim/app/api/workspaces/[id]/docx/preview/route.test.ts‎

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,16 +6,23 @@ import { NextRequest } from 'next/server'
66
import { beforeEach, describe, expect, it, vi } from 'vitest'
77
import { MAX_DOCUMENT_PREVIEW_CODE_BYTES } from '@/lib/execution/constants'
88

9-
const { mockRunSandboxTask } = vi.hoisted(() => ({
10-
mockRunSandboxTask: vi.fn(),
11-
}))
9+
const { mockRunSandboxTask, SandboxUserCodeError } = vi.hoisted(() => {
10+
class SandboxUserCodeError extends Error {
11+
constructor(message: string, name: string) {
12+
super(message)
13+
this.name = name
14+
}
15+
}
16+
return { mockRunSandboxTask: vi.fn(), SandboxUserCodeError }
17+
})
1218

1319
const mockVerifyWorkspaceMembership = workflowsApiUtilsMockFns.mockVerifyWorkspaceMembership
1420

1521
vi.mock('@/app/api/workflows/utils', () => workflowsApiUtilsMock)
1622

1723
vi.mock('@/lib/execution/sandbox/run-task', () => ({
1824
runSandboxTask: mockRunSandboxTask,
25+
SandboxUserCodeError,
1926
}))
2027

2128
import { POST } from '@/app/api/workspaces/[id]/docx/preview/route'
@@ -189,4 +196,31 @@ describe('DOCX preview API route', () => {
189196
expect(response.status).toBe(500)
190197
await expect(response.json()).resolves.toEqual({ error: 'boom: sandbox failed' })
191198
})
199+
200+
it('returns 422 when user code throws inside the sandbox', async () => {
201+
mockRunSandboxTask.mockRejectedValue(
202+
new SandboxUserCodeError('Invalid or unexpected token', 'SyntaxError')
203+
)
204+
205+
const request = new NextRequest(
206+
'http://localhost:3000/api/workspaces/workspace-1/docx/preview',
207+
{
208+
method: 'POST',
209+
headers: {
210+
'Content-Type': 'application/json',
211+
},
212+
body: JSON.stringify({ code: 'const x = ' }),
213+
}
214+
)
215+
216+
const response = await POST(request, {
217+
params: Promise.resolve({ id: 'workspace-1' }),
218+
})
219+
220+
expect(response.status).toBe(422)
221+
await expect(response.json()).resolves.toEqual({
222+
error: 'Invalid or unexpected token',
223+
errorName: 'SyntaxError',
224+
})
225+
})
192226
})

‎apps/sim/app/api/workspaces/[id]/pdf/preview/route.test.ts‎

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,16 +6,23 @@ import { NextRequest } from 'next/server'
66
import { beforeEach, describe, expect, it, vi } from 'vitest'
77
import { MAX_DOCUMENT_PREVIEW_CODE_BYTES } from '@/lib/execution/constants'
88

9-
const { mockRunSandboxTask } = vi.hoisted(() => ({
10-
mockRunSandboxTask: vi.fn(),
11-
}))
9+
const { mockRunSandboxTask, SandboxUserCodeError } = vi.hoisted(() => {
10+
class SandboxUserCodeError extends Error {
11+
constructor(message: string, name: string) {
12+
super(message)
13+
this.name = name
14+
}
15+
}
16+
return { mockRunSandboxTask: vi.fn(), SandboxUserCodeError }
17+
})
1218

1319
const mockVerifyWorkspaceMembership = workflowsApiUtilsMockFns.mockVerifyWorkspaceMembership
1420

1521
vi.mock('@/app/api/workflows/utils', () => workflowsApiUtilsMock)
1622

1723
vi.mock('@/lib/execution/sandbox/run-task', () => ({
1824
runSandboxTask: mockRunSandboxTask,
25+
SandboxUserCodeError,
1926
}))
2027

2128
import { POST } from '@/app/api/workspaces/[id]/pdf/preview/route'
@@ -187,4 +194,31 @@ describe('PDF preview API route', () => {
187194
expect(response.status).toBe(500)
188195
await expect(response.json()).resolves.toEqual({ error: 'boom: sandbox failed' })
189196
})
197+
198+
it('returns 422 when user code throws inside the sandbox', async () => {
199+
mockRunSandboxTask.mockRejectedValue(
200+
new SandboxUserCodeError('Invalid or unexpected token', 'SyntaxError')
201+
)
202+
203+
const request = new NextRequest(
204+
'http://localhost:3000/api/workspaces/workspace-1/pdf/preview',
205+
{
206+
method: 'POST',
207+
headers: {
208+
'Content-Type': 'application/json',
209+
},
210+
body: JSON.stringify({ code: 'const x = ' }),
211+
}
212+
)
213+
214+
const response = await POST(request, {
215+
params: Promise.resolve({ id: 'workspace-1' }),
216+
})
217+
218+
expect(response.status).toBe(422)
219+
await expect(response.json()).resolves.toEqual({
220+
error: 'Invalid or unexpected token',
221+
errorName: 'SyntaxError',
222+
})
223+
})
190224
})

‎apps/sim/app/api/workspaces/[id]/pptx/preview/route.test.ts‎

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,16 +6,23 @@ import { NextRequest } from 'next/server'
66
import { beforeEach, describe, expect, it, vi } from 'vitest'
77
import { MAX_DOCUMENT_PREVIEW_CODE_BYTES } from '@/lib/execution/constants'
88

9-
const { mockRunSandboxTask } = vi.hoisted(() => ({
10-
mockRunSandboxTask: vi.fn(),
11-
}))
9+
const { mockRunSandboxTask, SandboxUserCodeError } = vi.hoisted(() => {
10+
class SandboxUserCodeError extends Error {
11+
constructor(message: string, name: string) {
12+
super(message)
13+
this.name = name
14+
}
15+
}
16+
return { mockRunSandboxTask: vi.fn(), SandboxUserCodeError }
17+
})
1218

1319
const mockVerifyWorkspaceMembership = workflowsApiUtilsMockFns.mockVerifyWorkspaceMembership
1420

1521
vi.mock('@/app/api/workflows/utils', () => workflowsApiUtilsMock)
1622

1723
vi.mock('@/lib/execution/sandbox/run-task', () => ({
1824
runSandboxTask: mockRunSandboxTask,
25+
SandboxUserCodeError,
1926
}))
2027

2128
import { POST } from '@/app/api/workspaces/[id]/pptx/preview/route'
@@ -189,4 +196,31 @@ describe('PPTX preview API route', () => {
189196
expect(response.status).toBe(500)
190197
await expect(response.json()).resolves.toEqual({ error: 'boom: sandbox failed' })
191198
})
199+
200+
it('returns 422 when user code throws inside the sandbox', async () => {
201+
mockRunSandboxTask.mockRejectedValue(
202+
new SandboxUserCodeError('Invalid or unexpected token', 'SyntaxError')
203+
)
204+
205+
const request = new NextRequest(
206+
'http://localhost:3000/api/workspaces/workspace-1/pptx/preview',
207+
{
208+
method: 'POST',
209+
headers: {
210+
'Content-Type': 'application/json',
211+
},
212+
body: JSON.stringify({ code: 'const x = ' }),
213+
}
214+
)
215+
216+
const response = await POST(request, {
217+
params: Promise.resolve({ id: 'workspace-1' }),
218+
})
219+
220+
expect(response.status).toBe(422)
221+
await expect(response.json()).resolves.toEqual({
222+
error: 'Invalid or unexpected token',
223+
errorName: 'SyntaxError',
224+
})
225+
})
192226
})

‎apps/sim/executor/orchestrators/loop.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -717,10 +717,13 @@ export class LoopOrchestrator {
717717
})
718718

719719
if (vmResult.error) {
720-
logger.error('Failed to evaluate loop condition', {
720+
const isSystemError = vmResult.error.isSystemError === true
721+
const logFn = isSystemError ? logger.error.bind(logger) : logger.warn.bind(logger)
722+
logFn('Failed to evaluate loop condition', {
721723
condition,
722724
evaluatedCondition,
723725
error: vmResult.error,
726+
isSystemError,
724727
})
725728
return false
726729
}

‎apps/sim/lib/execution/isolated-vm.ts‎

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,15 @@ export interface IsolatedVMError {
9999
line?: number
100100
column?: number
101101
lineContent?: string
102+
/**
103+
* True when the failure is host-infrastructure caused (worker crash, IPC
104+
* failure, pool saturation, task misconfig) rather than anything the user's
105+
* code did. Callers use this to keep genuine server failures as 5xx while
106+
* translating user-caused failures (code errors, timeouts, aborts, per-owner
107+
* rate limits) into 4xx. Defaults to undefined/false — new error sites
108+
* default to user-caused unless explicitly marked.
109+
*/
110+
isSystemError?: boolean
102111
}
103112

104113
const POOL_SIZE = Number.parseInt(env.IVM_POOL_SIZE) || 4
@@ -838,7 +847,11 @@ function cleanupWorker(workerId: number) {
838847
pending.resolve({
839848
result: null,
840849
stdout: '',
841-
error: { message: 'Code execution failed unexpectedly. Please try again.', name: 'Error' },
850+
error: {
851+
message: 'Code execution failed unexpectedly. Please try again.',
852+
name: 'Error',
853+
isSystemError: true,
854+
},
842855
})
843856
workerInfo.pendingExecutions.delete(id)
844857
}
@@ -1125,7 +1138,11 @@ function dispatchToWorker(
11251138
resolve({
11261139
result: null,
11271140
stdout: '',
1128-
error: { message: 'Code execution failed to start. Please try again.', name: 'Error' },
1141+
error: {
1142+
message: 'Code execution failed to start. Please try again.',
1143+
name: 'Error',
1144+
isSystemError: true,
1145+
},
11291146
})
11301147
if (workerInfo.retiring && workerInfo.activeExecutions === 0) {
11311148
cleanupWorker(workerInfo.id)
@@ -1159,6 +1176,7 @@ function enqueueExecution(
11591176
error: {
11601177
message: 'Code execution is at capacity. Please try again in a moment.',
11611178
name: 'Error',
1179+
isSystemError: true,
11621180
},
11631181
})
11641182
return
@@ -1198,6 +1216,7 @@ function enqueueExecution(
11981216
error: {
11991217
message: 'Code execution timed out waiting for an available worker. Please try again.',
12001218
name: 'Error',
1219+
isSystemError: true,
12011220
},
12021221
})
12031222
}, QUEUE_TIMEOUT_MS)
@@ -1294,6 +1313,7 @@ export async function executeInIsolatedVM(
12941313
error: {
12951314
message: `Task "${req.task.id}" requires broker "${brokerName}" but none was provided`,
12961315
name: 'Error',
1316+
isSystemError: true,
12971317
},
12981318
}
12991319
}

0 commit comments

Comments
 (0)