Skip to content

Commit 3db1617

Browse files
committed
fix(executor): harden JSON parsing and error visibility in block handlers
Replace bare JSON.parse() calls across 5 executor handler files with the project's own parseJSON/parseJSONOrThrow utilities from executor/utils/json.ts. Add logger.warn to 2 empty catch blocks in the router handler that previously swallowed errors silently. The utilities already exist and are imported by some of these files (for parseObjectStrings). This makes handlers consistent with the project's established parsing patterns. Also adds test coverage for error paths: 2 new test files (response-handler, human-in-the-loop-handler) and error-path test cases for 3 existing test files (router, api, generic handlers).
1 parent 503432c commit 3db1617

10 files changed

Lines changed: 445 additions & 43 deletions

File tree

apps/sim/executor/handlers/api/api-handler.test.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,21 @@ describe('ApiBlockHandler', () => {
195195
)
196196
})
197197

198+
it('should handle malformed JSON body by keeping original string', async () => {
199+
const inputs = {
200+
url: 'https://example.com/api',
201+
body: '{invalid json body',
202+
}
203+
204+
await handler.execute(mockContext, mockBlock, inputs)
205+
206+
expect(mockExecuteTool).toHaveBeenCalledWith(
207+
'http_request',
208+
expect.objectContaining({ body: '{invalid json body' }),
209+
{ executionContext: mockContext }
210+
)
211+
})
212+
198213
it('should handle null body by converting to undefined', async () => {
199214
const inputs = {
200215
url: 'https://example.com/api',

apps/sim/executor/handlers/api/api-handler.ts

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { createLogger } from '@sim/logger'
22
import { validateUrlWithDNS } from '@/lib/core/security/input-validation.server'
33
import { BlockType, HTTP } from '@/executor/constants'
44
import type { BlockHandler, ExecutionContext } from '@/executor/types'
5+
import { parseJSON } from '@/executor/utils/json'
56
import type { SerializedBlock } from '@/serializer/types'
67
import { executeTool } from '@/tools'
78
import { getTool } from '@/tools/utils'
@@ -53,12 +54,10 @@ export class ApiBlockHandler implements BlockHandler {
5354

5455
if (processedInputs.body !== undefined) {
5556
if (typeof processedInputs.body === 'string') {
56-
try {
57-
const trimmedBody = processedInputs.body.trim()
58-
if (trimmedBody.startsWith('{') || trimmedBody.startsWith('[')) {
59-
processedInputs.body = JSON.parse(trimmedBody)
60-
}
61-
} catch (e) {}
57+
const trimmedBody = processedInputs.body.trim()
58+
if (trimmedBody.startsWith('{') || trimmedBody.startsWith('[')) {
59+
processedInputs.body = parseJSON(trimmedBody, processedInputs.body)
60+
}
6261
} else if (processedInputs.body === null) {
6362
processedInputs.body = undefined
6463
}

apps/sim/executor/handlers/generic/generic-handler.test.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import '@sim/testing/mocks/executor'
22

33
import { beforeEach, describe, expect, it, type Mock, vi } from 'vitest'
4+
import { getBlock } from '@/blocks/index'
45
import { BlockType } from '@/executor/constants'
56
import { GenericBlockHandler } from '@/executor/handlers/generic/generic-handler'
67
import type { ExecutionContext } from '@/executor/types'
@@ -144,4 +145,38 @@ describe('GenericBlockHandler', () => {
144145
'Block execution of Some Custom Tool failed with no error message'
145146
)
146147
})
148+
149+
it('should handle malformed json field input by keeping original string', async () => {
150+
const mockGetBlock = vi.mocked(getBlock)
151+
mockGetBlock.mockReturnValue({
152+
type: 'custom-type',
153+
name: 'Custom Block',
154+
description: 'Test block',
155+
category: 'tools',
156+
bgColor: '#000',
157+
icon: () => null,
158+
subBlocks: [],
159+
tools: {
160+
access: ['some_custom_tool'],
161+
config: {
162+
tool: () => 'some_custom_tool',
163+
params: (p: Record<string, unknown>) => p,
164+
},
165+
},
166+
inputs: {
167+
data: { type: 'json' },
168+
},
169+
outputs: {},
170+
} as unknown as ReturnType<typeof getBlock>)
171+
172+
const inputs = { data: '{not valid json' }
173+
174+
await handler.execute(mockContext, mockBlock, inputs)
175+
176+
expect(mockExecuteTool).toHaveBeenCalledWith(
177+
'some_custom_tool',
178+
expect.objectContaining({ data: '{not valid json' }),
179+
{ executionContext: mockContext }
180+
)
181+
})
147182
})

apps/sim/executor/handlers/generic/generic-handler.ts

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
import { createLogger } from '@sim/logger'
2-
import { toError } from '@sim/utils/errors'
32
import { getBlock } from '@/blocks/index'
43
import { isMcpTool } from '@/executor/constants'
54
import type { BlockHandler, ExecutionContext } from '@/executor/types'
5+
import { parseJSON } from '@/executor/utils/json'
66
import type { SerializedBlock } from '@/serializer/types'
77
import { executeTool } from '@/tools'
88
import { getTool } from '@/tools/utils'
@@ -45,13 +45,7 @@ export class GenericBlockHandler implements BlockHandler {
4545
if (typeof value === 'string' && value.trim().length > 0) {
4646
const inputType = typeof inputSchema === 'object' ? inputSchema.type : inputSchema
4747
if (inputType === 'json' || inputType === 'array') {
48-
try {
49-
finalInputs[key] = JSON.parse(value.trim())
50-
} catch (error) {
51-
logger.warn(`Failed to parse ${inputType} field "${key}":`, {
52-
error: toError(error).message,
53-
})
54-
}
48+
finalInputs[key] = parseJSON(value, value)
5549
}
5650
}
5751
}
Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,176 @@
1+
import '@sim/testing/mocks/executor'
2+
3+
import { urlsMock, urlsMockFns } from '@sim/testing'
4+
import { beforeEach, describe, expect, it, type Mock, vi } from 'vitest'
5+
import { BlockType } from '@/executor/constants'
6+
import { HumanInTheLoopBlockHandler } from '@/executor/handlers/human-in-the-loop/human-in-the-loop-handler'
7+
import type { ExecutionContext } from '@/executor/types'
8+
import type { SerializedBlock } from '@/serializer/types'
9+
import { executeTool } from '@/tools'
10+
11+
vi.mock('@/lib/core/utils/urls', () => urlsMock)
12+
13+
const { mockGeneratePauseContextId, mockMapNodeMetadataToPauseScopes } = vi.hoisted(() => ({
14+
mockGeneratePauseContextId: vi.fn(() => 'test-pause-context-id'),
15+
mockMapNodeMetadataToPauseScopes: vi.fn(() => ({
16+
parallelScope: undefined,
17+
loopScope: undefined,
18+
})),
19+
}))
20+
21+
vi.mock('@/executor/human-in-the-loop/utils', () => ({
22+
generatePauseContextId: mockGeneratePauseContextId,
23+
mapNodeMetadataToPauseScopes: mockMapNodeMetadataToPauseScopes,
24+
}))
25+
26+
vi.mock('@/executor/utils/builder-data', () => ({
27+
convertBuilderDataToJson: vi.fn(() => ({ key: 'value' })),
28+
convertPropertyValue: vi.fn((prop: any) => prop.value),
29+
}))
30+
31+
vi.mock('@/executor/utils/block-data', () => ({
32+
collectBlockData: vi.fn(() => ({
33+
blockData: {},
34+
blockNameMapping: {},
35+
})),
36+
}))
37+
38+
const mockExecuteTool = executeTool as Mock
39+
40+
describe('HumanInTheLoopBlockHandler', () => {
41+
let handler: HumanInTheLoopBlockHandler
42+
let mockBlock: SerializedBlock
43+
let mockContext: ExecutionContext
44+
45+
beforeEach(() => {
46+
vi.clearAllMocks()
47+
48+
handler = new HumanInTheLoopBlockHandler()
49+
50+
mockBlock = {
51+
id: 'hitl-block-1',
52+
metadata: { id: BlockType.HUMAN_IN_THE_LOOP, name: 'Test HITL Block' },
53+
position: { x: 0, y: 0 },
54+
config: { tool: BlockType.HUMAN_IN_THE_LOOP, params: {} },
55+
inputs: {},
56+
outputs: {},
57+
enabled: true,
58+
}
59+
60+
mockContext = {
61+
workflowId: 'test-workflow-id',
62+
blockStates: new Map(),
63+
blockLogs: [],
64+
metadata: { duration: 0 },
65+
environmentVariables: {},
66+
decisions: { router: new Map(), condition: new Map() },
67+
loopExecutions: new Map(),
68+
executedBlocks: new Set(),
69+
activeExecutionPath: new Set(),
70+
completedLoops: new Set(),
71+
}
72+
73+
urlsMockFns.mockGetBaseUrl.mockReturnValue('http://localhost:3000')
74+
mockExecuteTool.mockResolvedValue({ success: true, output: {} })
75+
mockGeneratePauseContextId.mockReturnValue('test-pause-context-id')
76+
mockMapNodeMetadataToPauseScopes.mockReturnValue({
77+
parallelScope: undefined,
78+
loopScope: undefined,
79+
})
80+
})
81+
82+
it('should return true for human-in-the-loop blocks', () => {
83+
expect(handler.canHandle(mockBlock)).toBe(true)
84+
})
85+
86+
it('should return false for non-hitl blocks', () => {
87+
const nonHitlBlock: SerializedBlock = {
88+
...mockBlock,
89+
metadata: { id: 'other-block' },
90+
}
91+
expect(handler.canHandle(nonHitlBlock)).toBe(false)
92+
})
93+
94+
it('should execute with human operation and return correct response shape', async () => {
95+
const inputs = {
96+
operation: 'human',
97+
inputFormat: [{ id: 'field-1', name: 'username', label: 'Username', type: 'string' }],
98+
builderData: [{ id: '1', name: 'result', type: 'string', value: 'test' }],
99+
}
100+
101+
const result = await handler.execute(mockContext, mockBlock, inputs)
102+
103+
expect(result.response).toBeDefined()
104+
expect(result.response.status).toBe(200)
105+
expect(result.response.headers).toHaveProperty('Content-Type')
106+
expect(result.response.data).toHaveProperty('operation', 'human')
107+
expect(result.response.data).toHaveProperty('responseStructure')
108+
expect(result.response.data).toHaveProperty('inputFormat')
109+
expect(result.response.data).toHaveProperty('submission', null)
110+
expect(result._pauseMetadata).toBeDefined()
111+
expect(result._pauseMetadata.pauseKind).toBe('human')
112+
})
113+
114+
it('should handle malformed JSON data in api operation mode', async () => {
115+
const inputs = {
116+
operation: 'api',
117+
dataMode: 'json',
118+
data: '{invalid json}',
119+
}
120+
121+
const result = await handler.execute(mockContext, mockBlock, inputs)
122+
123+
expect(result).toBeDefined()
124+
expect(result.response).toBeDefined()
125+
expect(result.response.data).toBe('{invalid json}')
126+
})
127+
128+
it('should handle valid JSON data in api operation mode', async () => {
129+
const inputs = {
130+
operation: 'api',
131+
dataMode: 'json',
132+
data: '{"message":"hello"}',
133+
}
134+
135+
const result = await handler.execute(mockContext, mockBlock, inputs)
136+
137+
expect(result.response.data).toMatchObject({ message: 'hello' })
138+
})
139+
140+
it('should return error response on execution failure', async () => {
141+
const inputs = {
142+
operation: 'human',
143+
inputFormat: 'not-an-array',
144+
builderData: 'not-an-array',
145+
}
146+
147+
mockMapNodeMetadataToPauseScopes.mockImplementation(() => {
148+
throw new Error('Metadata mapping failed')
149+
})
150+
151+
const result = await handler.execute(mockContext, mockBlock, inputs)
152+
153+
expect(result.response).toBeDefined()
154+
expect(result.response.status).toBe(500)
155+
expect(result.response.data).toHaveProperty('error')
156+
expect(result.response.data.message).toBe('Metadata mapping failed')
157+
})
158+
159+
it('should include resume links when executionId and workflowId exist', async () => {
160+
const contextWithExecution: ExecutionContext = {
161+
...mockContext,
162+
executionId: 'exec-123',
163+
}
164+
165+
const inputs = {
166+
operation: 'human',
167+
inputFormat: [],
168+
}
169+
170+
const result = await handler.execute(contextWithExecution, mockBlock, inputs)
171+
172+
expect(result.response.data._resume).toBeDefined()
173+
expect(result.url).toBeDefined()
174+
expect(result.resumeEndpoint).toBeDefined()
175+
})
176+
})

apps/sim/executor/handlers/human-in-the-loop/human-in-the-loop-handler.ts

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import {
1717
import type { BlockHandler, ExecutionContext, PauseMetadata } from '@/executor/types'
1818
import { collectBlockData } from '@/executor/utils/block-data'
1919
import { convertBuilderDataToJson, convertPropertyValue } from '@/executor/utils/builder-data'
20-
import { parseObjectStrings } from '@/executor/utils/json'
20+
import { parseJSON, parseObjectStrings } from '@/executor/utils/json'
2121
import type { SerializedBlock } from '@/serializer/types'
2222
import { executeTool } from '@/tools'
2323

@@ -253,13 +253,9 @@ export class HumanInTheLoopBlockHandler implements BlockHandler {
253253

254254
if (dataMode === 'json' && inputs.data) {
255255
if (typeof inputs.data === 'string') {
256-
try {
257-
return JSON.parse(inputs.data)
258-
} catch (error) {
259-
logger.warn('Failed to parse JSON data, returning as string:', error)
260-
return inputs.data
261-
}
262-
} else if (typeof inputs.data === 'object' && inputs.data !== null) {
256+
return parseJSON(inputs.data, inputs.data)
257+
}
258+
if (typeof inputs.data === 'object' && inputs.data !== null) {
263259
return inputs.data
264260
}
265261
return inputs.data

0 commit comments

Comments
 (0)