diff --git a/apps/sim/executor/handlers/condition/condition-handler.test.ts b/apps/sim/executor/handlers/condition/condition-handler.test.ts index fa24cec1bfa..42e1890159f 100644 --- a/apps/sim/executor/handlers/condition/condition-handler.test.ts +++ b/apps/sim/executor/handlers/condition/condition-handler.test.ts @@ -24,6 +24,17 @@ import { executeTool } from '@/tools' const mockExecuteTool = executeTool as ReturnType const mockCollectBlockData = collectBlockData as ReturnType + +/** The handler evaluates every testable branch in one call, so a whole condition list resolves to a single verdict. */ +const matchedAt = (index: number) => ({ + success: true, + output: { result: { matchedIndex: index } }, +}) +const noMatch = () => ({ success: true, output: { result: { matchedIndex: -1 } } }) +const threwAt = (index: number, message: string) => ({ + success: true, + output: { result: { matchedIndex: -1, threwAtIndex: index, message } }, +}) const mockConditionLogger = vi.mocked(loggerMock.createLogger).mock.results[ vi .mocked(loggerMock.createLogger) @@ -123,8 +134,8 @@ describe('ConditionBlockHandler', () => { vi.clearAllMocks() - // Default: condition evaluates to false (else path). Individual tests override with mockResolvedValueOnce. - mockExecuteTool.mockResolvedValue({ success: true, output: { result: false } }) + // Default: no branch matches (else path). Individual tests override with mockResolvedValueOnce. + mockExecuteTool.mockResolvedValue(noMatch()) }) it('should handle condition blocks', () => { @@ -135,7 +146,7 @@ describe('ConditionBlockHandler', () => { it('should execute condition block correctly and select first path', async () => { // Mock executeTool to return true for the condition - mockExecuteTool.mockResolvedValueOnce({ success: true, output: { result: true } }) + mockExecuteTool.mockResolvedValueOnce(matchedAt(0)) const conditions = [ { id: 'cond1', title: 'if', value: 'context.value > 5' }, @@ -162,7 +173,7 @@ describe('ConditionBlockHandler', () => { }) it('should pass correct parameters to function_execute tool', async () => { - mockExecuteTool.mockResolvedValueOnce({ success: true, output: { result: true } }) + mockExecuteTool.mockResolvedValueOnce(matchedAt(0)) const conditions = [ { id: 'cond1', title: 'if', value: 'context.value > 5' }, @@ -195,7 +206,7 @@ describe('ConditionBlockHandler', () => { blockData: { 'huge-block': { payload: 'x'.repeat(1024) } }, blockNameMapping: { hugeblock: 'huge-block' }, }) - mockExecuteTool.mockResolvedValueOnce({ success: true, output: { result: true } }) + mockExecuteTool.mockResolvedValueOnce(matchedAt(0)) const conditions = [ { id: 'cond1', title: 'if', value: 'true' }, @@ -209,7 +220,7 @@ describe('ConditionBlockHandler', () => { }) it('should select the else path if other conditions fail', async () => { - mockExecuteTool.mockResolvedValueOnce({ success: true, output: { result: false } }) + mockExecuteTool.mockResolvedValueOnce(noMatch()) const conditions = [ { id: 'cond1', title: 'if', value: 'context.value < 0' }, @@ -247,7 +258,7 @@ describe('ConditionBlockHandler', () => { }) it('finds whitespace and mixed-case else branches during fallback', async () => { - mockExecuteTool.mockResolvedValueOnce({ success: true, output: { result: false } }) + mockExecuteTool.mockResolvedValueOnce(noMatch()) const conditions = [ { id: 'cond1', title: 'if', value: 'false' }, @@ -282,7 +293,7 @@ describe('ConditionBlockHandler', () => { it('should handle evaluation errors gracefully', async () => { const secret = 'condition-runtime-secret-value' - mockExecuteTool.mockResolvedValueOnce({ + mockExecuteTool.mockResolvedValue({ success: false, error: `Cannot read ${secret} through __var_API_KEY`, }) @@ -300,8 +311,25 @@ describe('ConditionBlockHandler', () => { expect(JSON.stringify(mockConditionLogger.error.mock.calls)).not.toContain('__var_API_KEY') }) + it('names the branch an expression threw on without leaking the failure into logs', async () => { + const secret = 'condition-throw-secret-value' + mockExecuteTool.mockResolvedValueOnce(threwAt(1, `Cannot read ${secret}`)) + + const conditions = [ + { id: 'cond1', title: 'if', value: 'context.value > 5' }, + { id: 'cond2', title: 'else if', value: 'context.missing.deep()' }, + { id: 'else1', title: 'else', value: '' }, + ] + + await expect( + handler.execute(mockContext, mockBlock, { conditions: JSON.stringify(conditions) }) + ).rejects.toThrow(/Evaluation error in condition "else if": Cannot read/) + expect(mockExecuteTool).toHaveBeenCalledOnce() + expect(JSON.stringify(mockConditionLogger.error.mock.calls)).not.toContain(secret) + }) + it('should handle missing source block output gracefully', async () => { - mockExecuteTool.mockResolvedValueOnce({ success: true, output: { result: true } }) + mockExecuteTool.mockResolvedValueOnce(matchedAt(0)) const conditions = [{ id: 'cond1', title: 'if', value: 'true' }] const inputs = { conditions: JSON.stringify(conditions) } @@ -318,7 +346,7 @@ describe('ConditionBlockHandler', () => { }) it('should throw error if target block is missing', async () => { - mockExecuteTool.mockResolvedValueOnce({ success: true, output: { result: true } }) + mockExecuteTool.mockResolvedValueOnce(matchedAt(0)) const conditions = [{ id: 'cond1', title: 'if', value: 'true' }] const inputs = { conditions: JSON.stringify(conditions) } @@ -331,8 +359,7 @@ describe('ConditionBlockHandler', () => { }) it('should return no-match result if no condition matches and no else exists', async () => { - mockExecuteTool.mockResolvedValueOnce({ success: true, output: { result: false } }) - mockExecuteTool.mockResolvedValueOnce({ success: true, output: { result: false } }) + mockExecuteTool.mockResolvedValueOnce(noMatch()) const conditions = [ { id: 'cond1', title: 'if', value: 'false' }, @@ -358,7 +385,7 @@ describe('ConditionBlockHandler', () => { }) it('falls back to else path when loop context data is unavailable', async () => { - mockExecuteTool.mockResolvedValueOnce({ success: true, output: { result: false } }) + mockExecuteTool.mockResolvedValueOnce(noMatch()) const conditions = [ { id: 'cond1', title: 'if', value: 'context.item === "apple"' }, @@ -373,7 +400,7 @@ describe('ConditionBlockHandler', () => { }) it('should use collectBlockData to gather block state', async () => { - mockExecuteTool.mockResolvedValueOnce({ success: true, output: { result: true } }) + mockExecuteTool.mockResolvedValueOnce(matchedAt(0)) const conditions = [ { id: 'cond1', title: 'if', value: 'true' }, @@ -403,9 +430,166 @@ describe('ConditionBlockHandler', () => { ) }) + describe('Batched evaluation', () => { + const manyConditions = [ + { id: 'cond1', title: 'if', value: 'context.value === 1' }, + { id: 'cond2', title: 'else if', value: 'context.value === 2' }, + { id: 'cond3', title: 'else if', value: 'context.value === 3' }, + { id: 'cond4', title: 'else if', value: 'context.value === 4' }, + { id: 'else1', title: 'else', value: '' }, + ] + + it('evaluates every testable branch in a single call', async () => { + mockExecuteTool.mockResolvedValueOnce(noMatch()) + + await handler.execute(mockContext, mockBlock, { + conditions: JSON.stringify(manyConditions), + }) + + expect(mockExecuteTool).toHaveBeenCalledOnce() + }) + + it('tests the expressions in declaration order and stops at the first truthy one', async () => { + mockExecuteTool.mockResolvedValueOnce(noMatch()) + + await handler.execute(mockContext, mockBlock, { + conditions: JSON.stringify(manyConditions), + }) + + const [, toolParams] = mockExecuteTool.mock.calls[0] + const code = toolParams.code as string + const positions = manyConditions.slice(0, 4).map((condition) => code.indexOf(condition.value)) + + expect(positions.every((position) => position >= 0)).toBe(true) + expect(positions).toEqual([...positions].sort((a, b) => a - b)) + // The else branch carries no expression and must never reach the sandbox. + expect(code).toContain('return { matchedIndex: -1 }') + }) + + it('never sends the else branch for evaluation', async () => { + mockExecuteTool.mockResolvedValueOnce(noMatch()) + + await handler.execute(mockContext, mockBlock, { + conditions: JSON.stringify([ + { id: 'cond1', title: 'if', value: 'context.value === 1' }, + { id: 'else1', title: 'else', value: 'SHOULD_NEVER_BE_EVALUATED' }, + { id: 'cond2', title: 'else if', value: 'context.value === 2' }, + ]), + }) + + const [, toolParams] = mockExecuteTool.mock.calls[0] + expect(toolParams.code).not.toContain('SHOULD_NEVER_BE_EVALUATED') + expect(toolParams.code).not.toContain('context.value === 2') + }) + + it('re-evaluates one branch at a time when the batch returns no verdict', async () => { + // A syntax error anywhere fails the whole script at parse time, so the + // fallback must still take the branch an earlier condition matches. + mockExecuteTool.mockResolvedValueOnce({ + success: false, + error: 'SyntaxError: Unexpected identifier', + }) + mockExecuteTool.mockResolvedValueOnce({ success: true, output: { result: true } }) + + const result = await handler.execute(mockContext, mockBlock, { + conditions: JSON.stringify(manyConditions), + }) + + expect((result as any).selectedOption).toBe('cond1') + expect(mockExecuteTool).toHaveBeenCalledTimes(2) + }) + + it('emits a batch script a trailing line comment cannot break', async () => { + mockExecuteTool.mockResolvedValueOnce(matchedAt(0)) + + await handler.execute(mockContext, mockBlock, { + conditions: JSON.stringify([ + { id: 'cond1', title: 'if', value: 'context.value > 5 // gate' }, + { id: 'else1', title: 'else', value: '' }, + ]), + }) + + const [, toolParams] = mockExecuteTool.mock.calls[0] + // Compiling is the assertion: a comment that swallowed the closing + // parenthesis would fail the whole script at parse time. + expect(() => new Function(toolParams.code as string)).not.toThrow() + }) + + it('wraps a comment-bearing expression the same way when falling back', async () => { + // The fallback recovers a batch the sandbox could not parse, so it has to + // accept every expression the batch accepts — otherwise the recovery path + // rejects a branch the primary path would have matched. + mockExecuteTool.mockResolvedValueOnce({ + success: false, + error: 'Invalid JavaScript syntax: Unexpected token', + }) + mockExecuteTool.mockResolvedValueOnce({ success: true, output: { result: true } }) + + const result = await handler.execute(mockContext, mockBlock, { + conditions: JSON.stringify([ + { id: 'cond1', title: 'if', value: 'context.value > 5 // gate' }, + { id: 'cond2', title: 'else if', value: 'context.value ===' }, + { id: 'else1', title: 'else', value: '' }, + ]), + }) + + expect((result as any).selectedOption).toBe('cond1') + const [, fallbackParams] = mockExecuteTool.mock.calls[1] + expect(() => new Function(fallbackParams.code as string)).not.toThrow() + }) + + it('does not fan a timed-out batch out into one call per branch', async () => { + mockExecuteTool.mockResolvedValue({ + success: false, + error: 'Request timed out after 5000ms', + }) + + await expect( + handler.execute(mockContext, mockBlock, { conditions: JSON.stringify(manyConditions) }) + ).rejects.toThrow(/Evaluation error in condition "if".*Request timed out/) + expect(mockExecuteTool).toHaveBeenCalledOnce() + }) + + it('falls back when the batch reports a branch index outside the list', async () => { + mockExecuteTool.mockResolvedValueOnce(matchedAt(99)) + mockExecuteTool.mockResolvedValueOnce({ success: true, output: { result: true } }) + + const result = await handler.execute(mockContext, mockBlock, { + conditions: JSON.stringify(manyConditions), + }) + + expect((result as any).selectedOption).toBe('cond1') + expect(mockExecuteTool).toHaveBeenCalledTimes(2) + }) + + it('does not take the else path on a reply that carries no verdict', async () => { + // Reading a garbled reply as "nothing matched" would silently reroute the + // run, so an unrecognized shape has to fall back rather than fall through. + mockExecuteTool.mockResolvedValueOnce({ success: true, output: { result: { ok: true } } }) + mockExecuteTool.mockResolvedValueOnce({ success: true, output: { result: true } }) + + const result = await handler.execute(mockContext, mockBlock, { + conditions: JSON.stringify(manyConditions), + }) + + expect((result as any).selectedOption).toBe('cond1') + expect(mockExecuteTool).toHaveBeenCalledTimes(2) + }) + + it('does not retry per branch once the run has been cancelled', async () => { + mockContext.abortSignal = AbortSignal.abort() + mockExecuteTool.mockResolvedValue({ success: false, error: 'Execution cancelled' }) + + await expect( + handler.execute(mockContext, mockBlock, { conditions: JSON.stringify(manyConditions) }) + ).rejects.toThrow(/Evaluation error in condition "if".*Execution cancelled/) + expect(mockExecuteTool).toHaveBeenCalledOnce() + }) + }) + describe('Multiple branches to same target', () => { it('should handle if and else pointing to same target', async () => { - mockExecuteTool.mockResolvedValueOnce({ success: true, output: { result: true } }) + mockExecuteTool.mockResolvedValueOnce(matchedAt(0)) const conditions = [ { id: 'cond1', title: 'if', value: 'context.value > 5' }, @@ -431,7 +615,7 @@ describe('ConditionBlockHandler', () => { }) it('should select else branch to same target when if fails', async () => { - mockExecuteTool.mockResolvedValueOnce({ success: true, output: { result: false } }) + mockExecuteTool.mockResolvedValueOnce(noMatch()) const conditions = [ { id: 'cond1', title: 'if', value: 'context.value < 0' }, @@ -457,10 +641,8 @@ describe('ConditionBlockHandler', () => { }) it('should handle if→A, elseif→B, else→A pattern', async () => { - // First condition (cond1): false - mockExecuteTool.mockResolvedValueOnce({ success: true, output: { result: false } }) - // Second condition (cond2): false - mockExecuteTool.mockResolvedValueOnce({ success: true, output: { result: false } }) + // Neither cond1 nor cond2 matches, so the else branch wins. + mockExecuteTool.mockResolvedValueOnce(noMatch()) const conditions = [ { id: 'cond1', title: 'if', value: 'context.value === 1' }, @@ -486,7 +668,7 @@ describe('ConditionBlockHandler', () => { describe('Condition evaluation with different data types', () => { it('should evaluate string comparison conditions', async () => { - mockExecuteTool.mockResolvedValueOnce({ success: true, output: { result: true } }) + mockExecuteTool.mockResolvedValueOnce(matchedAt(0)) ;(mockContext.blockStates as any).set(mockSourceBlock.id, { output: { name: 'test', status: 'active' }, @@ -506,7 +688,7 @@ describe('ConditionBlockHandler', () => { }) it('should evaluate boolean conditions', async () => { - mockExecuteTool.mockResolvedValueOnce({ success: true, output: { result: true } }) + mockExecuteTool.mockResolvedValueOnce(matchedAt(0)) ;(mockContext.blockStates as any).set(mockSourceBlock.id, { output: { isEnabled: true, count: 5 }, @@ -526,7 +708,7 @@ describe('ConditionBlockHandler', () => { }) it('should evaluate array length conditions', async () => { - mockExecuteTool.mockResolvedValueOnce({ success: true, output: { result: true } }) + mockExecuteTool.mockResolvedValueOnce(matchedAt(0)) ;(mockContext.blockStates as any).set(mockSourceBlock.id, { output: { items: [1, 2, 3, 4, 5] }, @@ -546,7 +728,7 @@ describe('ConditionBlockHandler', () => { }) it('should evaluate null/undefined check conditions', async () => { - mockExecuteTool.mockResolvedValueOnce({ success: true, output: { result: true } }) + mockExecuteTool.mockResolvedValueOnce(matchedAt(0)) ;(mockContext.blockStates as any).set(mockSourceBlock.id, { output: { data: null }, @@ -568,8 +750,7 @@ describe('ConditionBlockHandler', () => { describe('Multiple else-if conditions', () => { it('should evaluate multiple else-if conditions in order', async () => { - mockExecuteTool.mockResolvedValueOnce({ success: true, output: { result: false } }) - mockExecuteTool.mockResolvedValueOnce({ success: true, output: { result: true } }) + mockExecuteTool.mockResolvedValueOnce(matchedAt(1)) ;(mockContext.blockStates as any).set(mockSourceBlock.id, { output: { score: 75 }, @@ -612,9 +793,7 @@ describe('ConditionBlockHandler', () => { }) it('should skip to else when all else-if fail', async () => { - mockExecuteTool.mockResolvedValueOnce({ success: true, output: { result: false } }) - mockExecuteTool.mockResolvedValueOnce({ success: true, output: { result: false } }) - mockExecuteTool.mockResolvedValueOnce({ success: true, output: { result: false } }) + mockExecuteTool.mockResolvedValueOnce(noMatch()) ;(mockContext.blockStates as any).set(mockSourceBlock.id, { output: { score: 30 }, @@ -638,7 +817,7 @@ describe('ConditionBlockHandler', () => { describe('Condition with no outgoing edge', () => { it('should set selectedOption when condition matches but has no edge', async () => { - mockExecuteTool.mockResolvedValueOnce({ success: true, output: { result: true } }) + mockExecuteTool.mockResolvedValueOnce(matchedAt(0)) const conditions = [ { id: 'cond1', title: 'if', value: 'true' }, @@ -660,7 +839,7 @@ describe('ConditionBlockHandler', () => { }) it('should set selectedOption when else is selected but has no edge', async () => { - mockExecuteTool.mockResolvedValueOnce({ success: true, output: { result: false } }) + mockExecuteTool.mockResolvedValueOnce(noMatch()) const conditions = [ { id: 'cond1', title: 'if', value: 'false' }, @@ -682,7 +861,7 @@ describe('ConditionBlockHandler', () => { }) it('should deactivate if-path when else is selected with no edge', async () => { - mockExecuteTool.mockResolvedValueOnce({ success: true, output: { result: false } }) + mockExecuteTool.mockResolvedValueOnce(noMatch()) const conditions = [ { id: 'cond1', title: 'if', value: 'context.value > 100' }, @@ -715,7 +894,7 @@ describe('ConditionBlockHandler', () => { }) it('should handle conditions passed as array directly', async () => { - mockExecuteTool.mockResolvedValueOnce({ success: true, output: { result: true } }) + mockExecuteTool.mockResolvedValueOnce(matchedAt(0)) const conditions = [ { id: 'cond1', title: 'if', value: 'true' }, @@ -731,7 +910,7 @@ describe('ConditionBlockHandler', () => { describe('Source output filtering', () => { it('should not propagate error field from source block output', async () => { - mockExecuteTool.mockResolvedValueOnce({ success: true, output: { result: true } }) + mockExecuteTool.mockResolvedValueOnce(matchedAt(0)) ;(mockContext.blockStates as any).set(mockSourceBlock.id, { output: { value: 10, text: 'hello', error: 'upstream block failed' }, @@ -753,7 +932,7 @@ describe('ConditionBlockHandler', () => { }) it('should not propagate _pauseMetadata from source block output', async () => { - mockExecuteTool.mockResolvedValueOnce({ success: true, output: { result: true } }) + mockExecuteTool.mockResolvedValueOnce(matchedAt(0)) ;(mockContext.blockStates as any).set(mockSourceBlock.id, { output: { value: 10, _pauseMetadata: { contextId: 'abc' } }, @@ -774,7 +953,7 @@ describe('ConditionBlockHandler', () => { }) it('should still pass through non-control fields from source output', async () => { - mockExecuteTool.mockResolvedValueOnce({ success: true, output: { result: true } }) + mockExecuteTool.mockResolvedValueOnce(matchedAt(0)) ;(mockContext.blockStates as any).set(mockSourceBlock.id, { output: { value: 10, text: 'hello', customData: { nested: true } }, @@ -798,7 +977,7 @@ describe('ConditionBlockHandler', () => { describe('Virtual block ID handling', () => { it('should use currentVirtualBlockId for decision key when available', async () => { - mockExecuteTool.mockResolvedValueOnce({ success: true, output: { result: true } }) + mockExecuteTool.mockResolvedValueOnce(matchedAt(0)) mockContext.currentVirtualBlockId = 'virtual-block-123' @@ -817,7 +996,7 @@ describe('ConditionBlockHandler', () => { describe('Parallel branch handling', () => { it('should resolve connections and block data correctly when inside a parallel branch', async () => { - mockExecuteTool.mockResolvedValueOnce({ success: true, output: { result: true } }) + mockExecuteTool.mockResolvedValueOnce(matchedAt(0)) const parallelConditionBlock: SerializedBlock = { id: 'cond-block-1₍0₎', @@ -897,7 +1076,7 @@ describe('ConditionBlockHandler', () => { }) it('should find correct source block output in parallel branch context', async () => { - mockExecuteTool.mockResolvedValueOnce({ success: true, output: { result: true } }) + mockExecuteTool.mockResolvedValueOnce(matchedAt(0)) const parallelConditionBlock: SerializedBlock = { id: 'cond-block-1₍1₎', @@ -969,7 +1148,7 @@ describe('ConditionBlockHandler', () => { }) it('should fall back to else when condition is false in parallel branch', async () => { - mockExecuteTool.mockResolvedValueOnce({ success: true, output: { result: false } }) + mockExecuteTool.mockResolvedValueOnce(noMatch()) const parallelConditionBlock: SerializedBlock = { id: 'cond-block-1₍2₎', diff --git a/apps/sim/executor/handlers/condition/condition-handler.ts b/apps/sim/executor/handlers/condition/condition-handler.ts index d21702ae81f..671fbc73eca 100644 --- a/apps/sim/executor/handlers/condition/condition-handler.ts +++ b/apps/sim/executor/handlers/condition/condition-handler.ts @@ -1,4 +1,5 @@ import { createLogger } from '@sim/logger' +import { getErrorMessage, toError } from '@sim/utils/errors' import { normalizeStringRecord, normalizeWorkflowVariables } from '@/lib/core/utils/records' import { isElseConditionTitle } from '@/lib/workflows/conditions' import type { BlockOutput } from '@/blocks/types' @@ -13,72 +14,224 @@ import { } from '@/executor/utils/subflow-utils' import type { SerializedBlock } from '@/serializer/types' import { executeTool } from '@/tools' +import type { ToolResponse } from '@/tools/types' const logger = createLogger('ConditionBlockHandler') const CONDITION_TIMEOUT_MS = 5000 +interface ConditionEntry { + id: string + title: string + value: string +} + +/** Verdict for a whole condition list evaluated in one function execution. */ +type ConditionEvaluation = + | { status: 'matched'; index: number } + | { status: 'no-match' } + | { status: 'expression-threw'; index: number; message: string } + | { status: 'no-verdict'; message: string; timedOut: boolean } + /** - * Evaluates a single condition expression. - * The resolver preserves legacy Condition expression substitution before this function executes the - * resulting JavaScript through the shared function execution boundary. + * Wraps one expression as a boolean test, on its own line so a trailing line + * comment ends before the closing parenthesis instead of swallowing it. * - * `blockData` is deliberately empty: the resolver already inlines every `` reference - * into the expression before this runs, so shipping the run's accumulated block outputs would only - * inflate the request body. Sending them blew the 10MB body cap on wide subflows, where a single - * flat `blockStates` map holds every branch's outputs. + * The batched script and the per-branch fallback both wrap through here. Wrapping + * them separately let the two drift: an expression carrying a trailing comment + * parsed in the batch and failed in the fallback, so the recovery path rejected + * a branch the primary path accepted. + */ +function buildBooleanTest(expression: string): string { + return `Boolean(\n${expression}\n)` +} + +/** + * Builds one script that tests each expression in order and reports the index of + * the first truthy one. * - * Returns true if condition is met, false otherwise. + * Ordering and short-circuiting match evaluating the expressions one call at a + * time: an expression is only reached once every earlier one returned falsy, so + * a later expression that throws is still never reached and the run takes the + * same branch it takes today. The `catch` reports the index it was on as data + * rather than rethrowing, which is what lets the caller name the failing branch. */ -async function evaluateConditionExpression( +function buildConditionScript(expressions: string[], evalContext: Record): string { + const tests = expressions + .map( + (expression, index) => + ` __simConditionIndex = ${index}\n` + + ` if (${buildBooleanTest(expression)}) return { matchedIndex: ${index} }` + ) + .join('\n') + + return [ + `const context = ${JSON.stringify(evalContext)};`, + 'let __simConditionIndex = -1', + 'try {', + tests, + ' return { matchedIndex: -1 }', + '} catch (__simConditionError) {', + ' return {', + ' matchedIndex: -1,', + ' threwAtIndex: __simConditionIndex,', + ' message:', + ' __simConditionError && __simConditionError.message', + ' ? String(__simConditionError.message)', + ' : String(__simConditionError),', + ' }', + '}', + ].join('\n') +} + +/** + * Runs condition code through the shared function execution boundary. + * + * `blockData` is deliberately empty: the resolver already inlines every + * `` reference into the expression before this runs, so shipping + * the run's accumulated block outputs would only inflate the request body. + * Sending them blew the 10MB body cap on wide subflows, where a single flat + * `blockStates` map holds every branch's outputs. + */ +async function runConditionCode( ctx: ExecutionContext, - conditionExpression: string, - providedEvalContext?: Record, + code: string, currentNodeId?: string -): Promise { - const evalContext = providedEvalContext || {} - - try { - const contextSetup = `const context = ${JSON.stringify(evalContext)};` - const code = `${contextSetup}\nreturn Boolean(${conditionExpression})` - - const { blockNameMapping, blockOutputSchemas } = collectBlockData(ctx, currentNodeId) - - const result = await executeTool( - 'function_execute', - { - code, - timeout: CONDITION_TIMEOUT_MS, - envVars: normalizeStringRecord(ctx.environmentVariables), - workflowVariables: normalizeWorkflowVariables(ctx.workflowVariables), - blockData: {}, - blockNameMapping, - blockOutputSchemas, - _context: { - workflowId: ctx.workflowId, - workspaceId: ctx.workspaceId, - userId: ctx.userId, - isDeployedContext: ctx.isDeployedContext, - enforceCredentialAccess: ctx.enforceCredentialAccess, - }, +): Promise { + const { blockNameMapping, blockOutputSchemas } = collectBlockData(ctx, currentNodeId) + + return executeTool( + 'function_execute', + { + code, + timeout: CONDITION_TIMEOUT_MS, + envVars: normalizeStringRecord(ctx.environmentVariables), + workflowVariables: normalizeWorkflowVariables(ctx.workflowVariables), + blockData: {}, + blockNameMapping, + blockOutputSchemas, + _context: { + workflowId: ctx.workflowId, + workspaceId: ctx.workspaceId, + userId: ctx.userId, + isDeployedContext: ctx.isDeployedContext, + enforceCredentialAccess: ctx.enforceCredentialAccess, }, - { executionContext: ctx } - ) + }, + { executionContext: ctx } + ) +} - if (!result.success) { - logger.error('Failed to evaluate condition', { - hasRuntimeError: Boolean(result.error), - }) - throw new Error(`Evaluation error in condition: ${result.error}`) +/** + * A failed batch normally falls back to one call per condition, which is exactly + * what this handler did before batching. A timeout is the one failure where that + * is the wrong move: the fallback would re-run every remaining condition against + * the same stalled transport, turning one slow call into as many slow calls as + * there are branches. Classified by message because the transport reports giving + * up as text, the same way `isRetryableFailure` does in `@/tools`. + */ +function isTimeoutFailure(error: string | undefined): boolean { + if (!error) return false + const message = error.toLowerCase() + return message.includes('timed out') || message.includes('timeout') +} + +/** Evaluates the whole condition list in a single function execution. */ +async function evaluateConditionList( + ctx: ExecutionContext, + expressions: string[], + evalContext: Record, + currentNodeId?: string +): Promise { + const result = await runConditionCode( + ctx, + buildConditionScript(expressions, evalContext), + currentNodeId + ) + + if (!result.success) { + const message = result.error ?? 'Condition evaluation failed' + return { status: 'no-verdict', message, timedOut: isTimeoutFailure(result.error) } + } + + const output = result.output?.result + if (!output || typeof output !== 'object') { + return { + status: 'no-verdict', + message: 'Condition evaluation returned no verdict', + timedOut: false, + } + } + + const { matchedIndex, threwAtIndex, message } = output as { + matchedIndex?: unknown + threwAtIndex?: unknown + message?: unknown + } + + if (typeof threwAtIndex === 'number' && threwAtIndex >= 0) { + if (threwAtIndex >= expressions.length) { + return { + status: 'no-verdict', + message: 'Condition evaluation reported an unknown branch', + timedOut: false, + } + } + return { + status: 'expression-threw', + index: threwAtIndex, + message: typeof message === 'string' && message ? message : 'Unknown evaluation error', + } + } + + // The script always reports a `matchedIndex`, so anything else is a response + // this handler did not produce. Treating that as "no branch matched" would + // silently route the run down the else path on a garbled reply. + if (typeof matchedIndex !== 'number') { + return { + status: 'no-verdict', + message: 'Condition evaluation returned an unrecognized verdict', + timedOut: false, + } + } + if (matchedIndex < 0) { + return { status: 'no-match' } + } + if (matchedIndex >= expressions.length) { + return { + status: 'no-verdict', + message: 'Condition evaluation matched an unknown branch', + timedOut: false, } + } + + return { status: 'matched', index: matchedIndex } +} + +/** + * Evaluates one expression in its own call. Kept as the fallback for a batch + * that produced no verdict: a syntax error anywhere in the list fails the whole + * script at parse time, while evaluating one at a time only reaches — and so + * only fails on — the branches the run actually takes. + */ +async function evaluateSingleCondition( + ctx: ExecutionContext, + expression: string, + evalContext: Record, + currentNodeId?: string +): Promise { + const code = `const context = ${JSON.stringify(evalContext)};\nreturn ${buildBooleanTest(expression)}` + const result = await runConditionCode(ctx, code, currentNodeId) - return Boolean(result.output?.result) - } catch (evalError: any) { - logger.error('Failed to evaluate condition', { - errorName: evalError?.name, - }) - throw new Error(`Evaluation error in condition: ${evalError.message}`) + if (!result.success) { + throw new Error(result.error ?? 'Condition evaluation failed') } + + return Boolean(result.output?.result) +} + +function conditionError(condition: ConditionEntry, message: string): Error { + return new Error(`Evaluation error in condition "${condition.title}": ${message}`) } /** @@ -175,7 +328,7 @@ export class ConditionBlockHandler implements BlockHandler { return rest } - private parseConditions(input: any): Array<{ id: string; title: string; value: string }> { + private parseConditions(input: any): ConditionEntry[] { try { const conditions = Array.isArray(input) ? input : JSON.parse(input || '[]') return conditions @@ -192,8 +345,8 @@ export class ConditionBlockHandler implements BlockHandler { private buildEvaluationContext( ctx: ExecutionContext, sourceBlockId?: string - ): Record { - let evalContext: Record = {} + ): Record { + let evalContext: Record = {} if (sourceBlockId) { const sourceOutput = ctx.blockStates.get(sourceBlockId)?.output @@ -208,48 +361,108 @@ export class ConditionBlockHandler implements BlockHandler { return evalContext } + /** + * An else branch wins as soon as it is reached, so only the branches ahead of + * it are ever testable — matching the original loop, which returned on the + * first else it walked past rather than assuming else comes last. + */ private async evaluateConditions( - conditions: Array<{ id: string; title: string; value: string }>, + conditions: ConditionEntry[], outgoingConnections: Array<{ source: string; target: string; sourceHandle?: string }>, - evalContext: Record, + evalContext: Record, ctx: ExecutionContext, currentNodeId?: string ): Promise<{ selectedConnection: { target: string; sourceHandle?: string } | null - selectedCondition: { id: string; title: string; value: string } | null + selectedCondition: ConditionEntry | null }> { - for (const condition of conditions) { - if (isElseConditionTitle(condition.title)) { - const connection = this.findConnectionForCondition(outgoingConnections, condition.id) - return { selectedConnection: connection ?? null, selectedCondition: condition } + const elseIndex = conditions.findIndex((condition) => isElseConditionTitle(condition.title)) + const testable = elseIndex === -1 ? conditions : conditions.slice(0, elseIndex) + const elseCondition = elseIndex === -1 ? null : conditions[elseIndex] + + const matched = await this.findMatchingCondition(testable, evalContext, ctx, currentNodeId) + const selectedCondition = matched ?? elseCondition + + if (!selectedCondition) { + return { selectedConnection: null, selectedCondition: null } + } + + return { + selectedConnection: + this.findConnectionForCondition(outgoingConnections, selectedCondition.id) ?? null, + selectedCondition, + } + } + + private async findMatchingCondition( + conditions: ConditionEntry[], + evalContext: Record, + ctx: ExecutionContext, + currentNodeId?: string + ): Promise { + if (conditions.length === 0) return null + + const expressions = conditions.map((condition) => String(condition.value || '')) + + let evaluation: ConditionEvaluation + try { + evaluation = await evaluateConditionList(ctx, expressions, evalContext, currentNodeId) + } catch (error) { + evaluation = { + status: 'no-verdict', + message: getErrorMessage(error, 'Condition evaluation failed'), + timedOut: false, } + } + + switch (evaluation.status) { + case 'matched': + return conditions[evaluation.index] + case 'no-match': + return null + case 'expression-threw': + logger.error('Failed to evaluate condition', { conditionCount: conditions.length }) + throw conditionError(conditions[evaluation.index], evaluation.message) + case 'no-verdict': + // Retrying one branch at a time is what recovers a batch the sandbox + // could not parse. It is the wrong move for a stalled transport, which + // would re-run every branch against the same stall, or for a cancelled + // run, where every retry aborts on arrival — both surface the batch + // failure as it stands. The whole list was one call, so no single + // branch owns that failure; name the first, where evaluation started. + if (evaluation.timedOut || ctx.abortSignal?.aborted) { + logger.error('Failed to evaluate conditions', { conditionCount: conditions.length }) + throw conditionError(conditions[0], evaluation.message) + } + logger.warn('Batched condition evaluation produced no verdict, retrying one at a time', { + conditionCount: conditions.length, + }) + return this.findMatchingConditionIndividually(conditions, evalContext, ctx, currentNodeId) + } + } - const conditionValueString = String(condition.value || '') + private async findMatchingConditionIndividually( + conditions: ConditionEntry[], + evalContext: Record, + ctx: ExecutionContext, + currentNodeId?: string + ): Promise { + for (const condition of conditions) { try { - const conditionMet = await evaluateConditionExpression( + const conditionMet = await evaluateSingleCondition( ctx, - conditionValueString, + String(condition.value || ''), evalContext, currentNodeId ) - - if (conditionMet) { - const connection = this.findConnectionForCondition(outgoingConnections, condition.id) - if (connection) { - return { selectedConnection: connection, selectedCondition: condition } - } - return { selectedConnection: null, selectedCondition: condition } - } - } catch (error: any) { - logger.error('Failed to evaluate condition', { - errorName: error?.name, - hasTitle: typeof condition.title === 'string' && condition.title.length > 0, - }) - throw new Error(`Evaluation error in condition "${condition.title}": ${error.message}`) + if (conditionMet) return condition + } catch (error) { + logger.error('Failed to evaluate condition', { errorName: toError(error).name }) + throw conditionError(condition, getErrorMessage(error, 'Condition evaluation failed')) } } - return { selectedConnection: null, selectedCondition: null } + return null } private findConnectionForCondition( diff --git a/apps/sim/tools/index.test.ts b/apps/sim/tools/index.test.ts index bd8a5f3d9ea..c5e9b541799 100644 --- a/apps/sim/tools/index.test.ts +++ b/apps/sim/tools/index.test.ts @@ -2323,6 +2323,53 @@ describe('executeTool Function', () => { tools.function_execute = originalFunctionTool }) + it('gives an internal route transport headroom past its requested execution budget', async () => { + const originalFunctionTool = { ...tools.function_execute } + tools.function_execute = { + ...tools.function_execute, + transformResponse: vi.fn().mockResolvedValue({ success: true, output: {} }), + } + + let observedSignal: AbortSignal | undefined + global.fetch = Object.assign( + vi.fn().mockImplementation( + async (_url: string, init: RequestInit) => + new Promise((_resolve, reject) => { + observedSignal = init.signal as AbortSignal + observedSignal.addEventListener('abort', () => { + const err = new Error('aborted') + err.name = 'AbortError' + reject(err) + }) + }) + ), + { preconnect: vi.fn() } + ) as typeof fetch + + vi.useFakeTimers() + try { + const resultPromise = executeTool( + 'function_execute', + { code: 'return 1', timeout: 5000 }, + { skipPostProcess: true } + ) + + // The route owns the 5s execution budget and needs to outlive it to report + // its own timeout, so the transport must still be waiting at that instant. + await vi.advanceTimersByTimeAsync(5000) + expect(observedSignal?.aborted).toBe(false) + + await vi.advanceTimersByTimeAsync(30_000) + const result = await resultPromise + + expect(result.success).toBe(false) + expect(result.error).toMatch(/timed out after 35000ms/) + } finally { + vi.useRealTimers() + tools.function_execute = originalFunctionTool + } + }) + it('should add timing information to results', async () => { const result = await executeTool( 'http_request', diff --git a/apps/sim/tools/index.ts b/apps/sim/tools/index.ts index 7bcae647256..b2c6ce262d4 100644 --- a/apps/sim/tools/index.ts +++ b/apps/sim/tools/index.ts @@ -953,6 +953,24 @@ import { normalizeToolId } from '@/tools/normalize' const MAX_REQUEST_BODY_SIZE_BYTES = 10 * 1024 * 1024 // 10MB const MAX_TOOL_RESPONSE_BODY_BYTES = 10 * 1024 * 1024 // 10MB +/** + * Headroom added to an internal route's requested timeout before it becomes the + * transport deadline. + * + * A `timeout` param bounds the work the route was asked to do — the code a + * sandbox runs, the upstream call a proxy route makes. The fetch around it also + * pays authentication, body parsing, workspace authorization, worker + * acquisition, and response serialization, none of which that budget was sized + * for. Arming the client with the bare number makes the caller give up at the + * same instant the route's own deadline fires, so the route can never win the + * race and report which part actually ran long; the caller sees an + * unattributable `Request timed out` instead. + * + * Sized above the isolated-vm worker's own 10s startup budget so a cold worker + * spawn stays inside the transport deadline rather than aborting it. + */ +const INTERNAL_ROUTE_TRANSPORT_OVERHEAD_MS = 30_000 + /** * User-friendly error message for body size limit exceeded */ @@ -2540,9 +2558,11 @@ async function executeToolRequest( let didTimeout = false // With a caller/execution abort signal present, the plan-based timeout bounds the call and // this only acts as a ceiling; without one, keep the tighter default as the hang safety net. - const timeout = - requestParams.timeout || - (signal ? getMaxExecutionTimeout() : DEFAULT_EXECUTION_TIMEOUT_MS) + const timeout = requestParams.timeout + ? requestParams.timeout + INTERNAL_ROUTE_TRANSPORT_OVERHEAD_MS + : signal + ? getMaxExecutionTimeout() + : DEFAULT_EXECUTION_TIMEOUT_MS const timeoutId = setTimeout(() => { didTimeout = true controller.abort(new DOMException('timeout', 'AbortError'))