Skip to content

Commit 8740566

Browse files
fix(block-resolver): path lookup check (#2869)
* fix(block-resolver): path lookup check * remove comments
1 parent 5de7228 commit 8740566

File tree

2 files changed

+124
-10
lines changed

2 files changed

+124
-10
lines changed

apps/sim/executor/variables/resolvers/block.test.ts

Lines changed: 52 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,18 +6,22 @@ import type { ResolutionContext } from './reference'
66

77
vi.mock('@sim/logger', () => loggerMock)
88

9-
/**
10-
* Creates a minimal workflow for testing.
11-
*/
12-
function createTestWorkflow(blocks: Array<{ id: string; name?: string; type?: string }> = []) {
9+
function createTestWorkflow(
10+
blocks: Array<{
11+
id: string
12+
name?: string
13+
type?: string
14+
outputs?: Record<string, any>
15+
}> = []
16+
) {
1317
return {
1418
version: '1.0',
1519
blocks: blocks.map((b) => ({
1620
id: b.id,
1721
position: { x: 0, y: 0 },
1822
config: { tool: b.type ?? 'function', params: {} },
1923
inputs: {},
20-
outputs: {},
24+
outputs: b.outputs ?? {},
2125
metadata: { id: b.type ?? 'function', name: b.name ?? b.id },
2226
enabled: true,
2327
})),
@@ -126,7 +130,7 @@ describe('BlockResolver', () => {
126130
expect(resolver.resolve('<source.items.1.id>', ctx)).toBe(2)
127131
})
128132

129-
it.concurrent('should return undefined for non-existent path', () => {
133+
it.concurrent('should return undefined for non-existent path when no schema defined', () => {
130134
const workflow = createTestWorkflow([{ id: 'source' }])
131135
const resolver = new BlockResolver(workflow)
132136
const ctx = createTestContext('current', {
@@ -136,6 +140,48 @@ describe('BlockResolver', () => {
136140
expect(resolver.resolve('<source.nonexistent>', ctx)).toBeUndefined()
137141
})
138142

143+
it.concurrent('should throw error for path not in output schema', () => {
144+
const workflow = createTestWorkflow([
145+
{
146+
id: 'source',
147+
outputs: {
148+
validField: { type: 'string', description: 'A valid field' },
149+
nested: {
150+
child: { type: 'number', description: 'Nested child' },
151+
},
152+
},
153+
},
154+
])
155+
const resolver = new BlockResolver(workflow)
156+
const ctx = createTestContext('current', {
157+
source: { validField: 'value', nested: { child: 42 } },
158+
})
159+
160+
expect(() => resolver.resolve('<source.invalidField>', ctx)).toThrow(
161+
/"invalidField" doesn't exist on block "source"/
162+
)
163+
expect(() => resolver.resolve('<source.invalidField>', ctx)).toThrow(/Available fields:/)
164+
})
165+
166+
it.concurrent('should return undefined for path in schema but missing in data', () => {
167+
const workflow = createTestWorkflow([
168+
{
169+
id: 'source',
170+
outputs: {
171+
requiredField: { type: 'string', description: 'Always present' },
172+
optionalField: { type: 'string', description: 'Sometimes missing' },
173+
},
174+
},
175+
])
176+
const resolver = new BlockResolver(workflow)
177+
const ctx = createTestContext('current', {
178+
source: { requiredField: 'value' },
179+
})
180+
181+
expect(resolver.resolve('<source.requiredField>', ctx)).toBe('value')
182+
expect(resolver.resolve('<source.optionalField>', ctx)).toBeUndefined()
183+
})
184+
139185
it.concurrent('should return undefined for non-existent block', () => {
140186
const workflow = createTestWorkflow([{ id: 'existing' }])
141187
const resolver = new BlockResolver(workflow)

apps/sim/executor/variables/resolvers/block.ts

Lines changed: 72 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,75 @@ import {
99
type ResolutionContext,
1010
type Resolver,
1111
} from '@/executor/variables/resolvers/reference'
12-
import type { SerializedWorkflow } from '@/serializer/types'
12+
import type { SerializedBlock, SerializedWorkflow } from '@/serializer/types'
13+
14+
function isPathInOutputSchema(
15+
outputs: Record<string, any> | undefined,
16+
pathParts: string[]
17+
): boolean {
18+
if (!outputs || pathParts.length === 0) {
19+
return true
20+
}
21+
22+
let current: any = outputs
23+
for (let i = 0; i < pathParts.length; i++) {
24+
const part = pathParts[i]
25+
26+
if (/^\d+$/.test(part)) {
27+
continue
28+
}
29+
30+
if (current === null || current === undefined) {
31+
return false
32+
}
33+
34+
if (part in current) {
35+
current = current[part]
36+
continue
37+
}
38+
39+
if (current.properties && part in current.properties) {
40+
current = current.properties[part]
41+
continue
42+
}
43+
44+
if (current.type === 'array' && current.items) {
45+
if (current.items.properties && part in current.items.properties) {
46+
current = current.items.properties[part]
47+
continue
48+
}
49+
if (part in current.items) {
50+
current = current.items[part]
51+
continue
52+
}
53+
}
54+
55+
if ('type' in current && typeof current.type === 'string') {
56+
if (!current.properties && !current.items) {
57+
return false
58+
}
59+
}
60+
61+
return false
62+
}
63+
64+
return true
65+
}
66+
67+
function getSchemaFieldNames(outputs: Record<string, any> | undefined): string[] {
68+
if (!outputs) return []
69+
return Object.keys(outputs)
70+
}
1371

1472
export class BlockResolver implements Resolver {
1573
private nameToBlockId: Map<string, string>
74+
private blockById: Map<string, SerializedBlock>
1675

1776
constructor(private workflow: SerializedWorkflow) {
1877
this.nameToBlockId = new Map()
78+
this.blockById = new Map()
1979
for (const block of workflow.blocks) {
80+
this.blockById.set(block.id, block)
2081
if (block.metadata?.name) {
2182
this.nameToBlockId.set(normalizeName(block.metadata.name), block.id)
2283
}
@@ -47,7 +108,9 @@ export class BlockResolver implements Resolver {
47108
return undefined
48109
}
49110

111+
const block = this.blockById.get(blockId)
50112
const output = this.getBlockOutput(blockId, context)
113+
51114
if (output === undefined) {
52115
return undefined
53116
}
@@ -63,9 +126,6 @@ export class BlockResolver implements Resolver {
63126
return result
64127
}
65128

66-
// If failed, check if we should try backwards compatibility fallback
67-
const block = this.workflow.blocks.find((b) => b.id === blockId)
68-
69129
// Response block backwards compatibility:
70130
// Old: <responseBlock.response.data> -> New: <responseBlock.data>
71131
// Only apply fallback if:
@@ -108,6 +168,14 @@ export class BlockResolver implements Resolver {
108168
}
109169
}
110170

171+
const schemaFields = getSchemaFieldNames(block?.outputs)
172+
if (schemaFields.length > 0 && !isPathInOutputSchema(block?.outputs, pathParts)) {
173+
throw new Error(
174+
`"${pathParts.join('.')}" doesn't exist on block "${blockName}". ` +
175+
`Available fields: ${schemaFields.join(', ')}`
176+
)
177+
}
178+
111179
return undefined
112180
}
113181

0 commit comments

Comments
 (0)