From 935359fe62c10b1ddae532b4f70aed7bf736fe90 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Thu, 14 Aug 2025 12:09:09 -0500 Subject: [PATCH 1/2] =?UTF-8?q?Revert=20"fix(workflow-block):=20revert=20c?= =?UTF-8?q?hange=20bubbling=20up=20error=20for=20workflow=20blo=E2=80=A6"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 9f0993ed57795cc1d0a6385a97bb6638734e7787. --- .../handlers/workflow/workflow-handler.ts | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/apps/sim/executor/handlers/workflow/workflow-handler.ts b/apps/sim/executor/handlers/workflow/workflow-handler.ts index 4a78eab28b3..0291cc26076 100644 --- a/apps/sim/executor/handlers/workflow/workflow-handler.ts +++ b/apps/sim/executor/handlers/workflow/workflow-handler.ts @@ -115,6 +115,12 @@ export class WorkflowBlockHandler implements BlockHandler { duration ) + // If the child workflow failed, throw an error to trigger proper error handling in the parent + if ((mappedResult as any).success === false) { + const childError = (mappedResult as any).error || 'Unknown error' + throw new Error(`Error in child workflow "${childWorkflowName}": ${childError}`) + } + return mappedResult } catch (error: any) { logger.error(`Error executing child workflow ${workflowId}:`, error) @@ -128,11 +134,15 @@ export class WorkflowBlockHandler implements BlockHandler { const workflowMetadata = workflows[workflowId] const childWorkflowName = workflowMetadata?.name || workflowId - return { - success: false, - error: error?.message || 'Child workflow execution failed', - childWorkflowName, - } as Record + // Enhance error message with child workflow context + const originalError = error.message || 'Unknown error' + + // Check if error message already has child workflow context to avoid duplication + if (originalError.startsWith('Error in child workflow')) { + throw error // Re-throw as-is to avoid duplication + } + + throw new Error(`Error in child workflow "${childWorkflowName}": ${originalError}`) } } From 9f4837f6081d043a3a86574e8af610fc39ccd1c4 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Thu, 14 Aug 2025 12:12:24 -0500 Subject: [PATCH 2/2] revert test changes --- .../workflow/workflow-handler.test.ts | 37 ++++++------------- apps/sim/executor/index.test.ts | 15 +++++--- 2 files changed, 22 insertions(+), 30 deletions(-) diff --git a/apps/sim/executor/handlers/workflow/workflow-handler.test.ts b/apps/sim/executor/handlers/workflow/workflow-handler.test.ts index 720e1918a61..e53dc8271f2 100644 --- a/apps/sim/executor/handlers/workflow/workflow-handler.test.ts +++ b/apps/sim/executor/handlers/workflow/workflow-handler.test.ts @@ -111,13 +111,9 @@ describe('WorkflowBlockHandler', () => { 'parent-workflow-id_sub_child-workflow-id_workflow-block-1' ) - const result = await handler.execute(mockBlock, inputs, mockContext) - expect(result).toEqual({ - success: false, - error: - 'Cyclic workflow dependency detected: parent-workflow-id_sub_child-workflow-id_workflow-block-1', - childWorkflowName: 'child-workflow-id', - }) + await expect(handler.execute(mockBlock, inputs, mockContext)).rejects.toThrow( + 'Error in child workflow "child-workflow-id": Cyclic workflow dependency detected: parent-workflow-id_sub_child-workflow-id_workflow-block-1' + ) }) it('should enforce maximum depth limit', async () => { @@ -130,12 +126,9 @@ describe('WorkflowBlockHandler', () => { 'level1_sub_level2_sub_level3_sub_level4_sub_level5_sub_level6_sub_level7_sub_level8_sub_level9_sub_level10_sub_level11', } - const result = await handler.execute(mockBlock, inputs, deepContext) - expect(result).toEqual({ - success: false, - error: 'Maximum workflow nesting depth of 10 exceeded', - childWorkflowName: 'child-workflow-id', - }) + await expect(handler.execute(mockBlock, inputs, deepContext)).rejects.toThrow( + 'Error in child workflow "child-workflow-id": Maximum workflow nesting depth of 10 exceeded' + ) }) it('should handle child workflow not found', async () => { @@ -147,12 +140,9 @@ describe('WorkflowBlockHandler', () => { statusText: 'Not Found', }) - const result = await handler.execute(mockBlock, inputs, mockContext) - expect(result).toEqual({ - success: false, - error: 'Child workflow non-existent-workflow not found', - childWorkflowName: 'non-existent-workflow', - }) + await expect(handler.execute(mockBlock, inputs, mockContext)).rejects.toThrow( + 'Error in child workflow "non-existent-workflow": Child workflow non-existent-workflow not found' + ) }) it('should handle fetch errors gracefully', async () => { @@ -160,12 +150,9 @@ describe('WorkflowBlockHandler', () => { mockFetch.mockRejectedValueOnce(new Error('Network error')) - const result = await handler.execute(mockBlock, inputs, mockContext) - expect(result).toEqual({ - success: false, - error: 'Child workflow child-workflow-id not found', - childWorkflowName: 'child-workflow-id', - }) + await expect(handler.execute(mockBlock, inputs, mockContext)).rejects.toThrow( + 'Error in child workflow "child-workflow-id": Child workflow child-workflow-id not found' + ) }) }) diff --git a/apps/sim/executor/index.test.ts b/apps/sim/executor/index.test.ts index e90734b1e7b..dd27587bf89 100644 --- a/apps/sim/executor/index.test.ts +++ b/apps/sim/executor/index.test.ts @@ -1448,7 +1448,7 @@ describe('Executor', () => { } ) - it.concurrent('should surface child workflow failure in result without throwing', async () => { + it.concurrent('should propagate errors from child workflows to parent workflow', async () => { const workflow = { version: '1.0', blocks: [ @@ -1488,12 +1488,17 @@ describe('Executor', () => { const result = await executor.execute('test-workflow-id') - // Verify that child workflow failure is surfaced in the overall result + // Verify that child workflow errors propagate to parent expect(result).toBeDefined() if ('success' in result) { - // With reverted behavior, parent execution may still be considered successful overall, - // but the workflow block output should capture the failure. Only assert structure here. - expect(typeof result.success).toBe('boolean') + // The workflow should fail due to child workflow failure + expect(result.success).toBe(false) + expect(result.error).toBeDefined() + + // Error message should indicate it came from a child workflow + if (result.error && typeof result.error === 'string') { + expect(result.error).toContain('Error in child workflow') + } } }) })