mirror of
https://github.com/simstudioai/sim.git
synced 2026-04-06 03:00:16 -04:00
Revert "fix(workflow-block): revert change bubbling up error for workflow block" (#965)
* Revert "fix(workflow-block): revert change bubbling up error for workflow blo…"
This reverts commit 9f0993ed57.
* revert test changes
This commit is contained in:
committed by
GitHub
parent
2e8f051e58
commit
ac41bf8c17
@@ -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'
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
@@ -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<string, any>
|
||||
// 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}`)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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')
|
||||
}
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user