From 57f0837da7853835d81a7751df10b3a84d84fcb7 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Wed, 28 Jan 2026 14:54:35 -0800 Subject: [PATCH] fix(child-workflow-error-spans): pass trace-spans accurately in block logs (#3054) * fix(child-workflow): must bypass hiddenFromDisplay config * fix passing of spans to be in block log * keep fallback for backwards compat * fix error message formatting * clean up --- .../components/trace-spans/trace-spans.tsx | 60 +------ .../components/log-details/log-details.tsx | 8 +- .../w/components/preview/preview.tsx | 5 +- apps/sim/executor/execution/block-executor.ts | 7 + .../workflow/workflow-handler.test.ts | 8 +- .../handlers/workflow/workflow-handler.ts | 88 ++++++++-- apps/sim/executor/types.ts | 6 + apps/sim/executor/utils/output-filter.ts | 5 +- .../logs/execution/trace-spans/trace-spans.ts | 159 ++++++++---------- 9 files changed, 181 insertions(+), 165 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/logs/components/log-details/components/trace-spans/trace-spans.tsx b/apps/sim/app/workspace/[workspaceId]/logs/components/log-details/components/trace-spans/trace-spans.tsx index c5988dc23..e3e091f02 100644 --- a/apps/sim/app/workspace/[workspaceId]/logs/components/log-details/components/trace-spans/trace-spans.tsx +++ b/apps/sim/app/workspace/[workspaceId]/logs/components/log-details/components/trace-spans/trace-spans.tsx @@ -57,40 +57,6 @@ function useSetToggle() { ) } -/** - * Generates a unique key for a trace span - */ -function getSpanKey(span: TraceSpan): string { - if (span.id) { - return span.id - } - const name = span.name || 'span' - const start = span.startTime || 'unknown-start' - const end = span.endTime || 'unknown-end' - return `${name}|${start}|${end}` -} - -/** - * Merges multiple arrays of trace span children, deduplicating by span key - */ -function mergeTraceSpanChildren(...groups: TraceSpan[][]): TraceSpan[] { - const merged: TraceSpan[] = [] - const seen = new Set() - - groups.forEach((group) => { - group.forEach((child) => { - const key = getSpanKey(child) - if (seen.has(key)) { - return - } - seen.add(key) - merged.push(child) - }) - }) - - return merged -} - /** * Parses a time value to milliseconds */ @@ -116,34 +82,16 @@ function hasErrorInTree(span: TraceSpan): boolean { /** * Normalizes and sorts trace spans recursively. - * Merges children from both span.children and span.output.childTraceSpans, - * deduplicates them, and sorts by start time. + * Deduplicates children and sorts by start time. */ function normalizeAndSortSpans(spans: TraceSpan[]): TraceSpan[] { return spans .map((span) => { const enrichedSpan: TraceSpan = { ...span } - // Clean output by removing childTraceSpans after extracting - if (enrichedSpan.output && typeof enrichedSpan.output === 'object') { - enrichedSpan.output = { ...enrichedSpan.output } - if ('childTraceSpans' in enrichedSpan.output) { - const { childTraceSpans, ...cleanOutput } = enrichedSpan.output as { - childTraceSpans?: TraceSpan[] - } & Record - enrichedSpan.output = cleanOutput - } - } - - // Merge and deduplicate children from both sources - const directChildren = Array.isArray(span.children) ? span.children : [] - const outputChildren = Array.isArray(span.output?.childTraceSpans) - ? (span.output!.childTraceSpans as TraceSpan[]) - : [] - - const mergedChildren = mergeTraceSpanChildren(directChildren, outputChildren) - enrichedSpan.children = - mergedChildren.length > 0 ? normalizeAndSortSpans(mergedChildren) : undefined + // Process and deduplicate children + const children = Array.isArray(span.children) ? span.children : [] + enrichedSpan.children = children.length > 0 ? normalizeAndSortSpans(children) : undefined return enrichedSpan }) diff --git a/apps/sim/app/workspace/[workspaceId]/logs/components/log-details/log-details.tsx b/apps/sim/app/workspace/[workspaceId]/logs/components/log-details/log-details.tsx index 38f9317a1..43dd6492f 100644 --- a/apps/sim/app/workspace/[workspaceId]/logs/components/log-details/log-details.tsx +++ b/apps/sim/app/workspace/[workspaceId]/logs/components/log-details/log-details.tsx @@ -18,6 +18,7 @@ import { import { ScrollArea } from '@/components/ui/scroll-area' import { BASE_EXECUTION_CHARGE } from '@/lib/billing/constants' import { cn } from '@/lib/core/utils/cn' +import { filterHiddenOutputKeys } from '@/lib/logs/execution/trace-spans/trace-spans' import { ExecutionSnapshot, FileCards, @@ -274,16 +275,13 @@ export const LogDetails = memo(function LogDetails({ return isWorkflowExecutionLog && log?.cost }, [log, isWorkflowExecutionLog]) - // Extract and clean the workflow final output (remove childTraceSpans for cleaner display) + // Extract and clean the workflow final output (recursively remove hidden keys for cleaner display) const workflowOutput = useMemo(() => { const executionData = log?.executionData as | { finalOutput?: Record } | undefined if (!executionData?.finalOutput) return null - const { childTraceSpans, ...cleanOutput } = executionData.finalOutput as { - childTraceSpans?: unknown - } & Record - return cleanOutput + return filterHiddenOutputKeys(executionData.finalOutput) as Record }, [log?.executionData]) useEffect(() => { diff --git a/apps/sim/app/workspace/[workspaceId]/w/components/preview/preview.tsx b/apps/sim/app/workspace/[workspaceId]/w/components/preview/preview.tsx index 8780c6317..da6bafe03 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/components/preview/preview.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/components/preview/preview.tsx @@ -39,8 +39,8 @@ interface WorkflowStackEntry { /** * Extracts child trace spans from a workflow block's execution data. - * Checks both the `children` property (where trace span processing moves them) - * and the legacy `output.childTraceSpans` for compatibility. + * Checks `children` property (where trace-spans processing puts them), + * with fallback to `output.childTraceSpans` for old stored logs. */ function extractChildTraceSpans(blockExecution: BlockExecutionData | undefined): TraceSpan[] { if (!blockExecution) return [] @@ -49,6 +49,7 @@ function extractChildTraceSpans(blockExecution: BlockExecutionData | undefined): return blockExecution.children } + // Backward compat: old stored logs may have childTraceSpans in output if (blockExecution.output && typeof blockExecution.output === 'object') { const output = blockExecution.output as Record if (Array.isArray(output.childTraceSpans)) { diff --git a/apps/sim/executor/execution/block-executor.ts b/apps/sim/executor/execution/block-executor.ts index 616e96690..126d2ce5a 100644 --- a/apps/sim/executor/execution/block-executor.ts +++ b/apps/sim/executor/execution/block-executor.ts @@ -152,6 +152,9 @@ export class BlockExecutor { blockLog.durationMs = duration blockLog.success = true blockLog.output = filterOutputForLog(block.metadata?.id || '', normalizedOutput, { block }) + if (normalizedOutput.childTraceSpans && Array.isArray(normalizedOutput.childTraceSpans)) { + blockLog.childTraceSpans = normalizedOutput.childTraceSpans + } } this.state.setBlockOutput(node.id, normalizedOutput, duration) @@ -245,6 +248,10 @@ export class BlockExecutor { blockLog.error = errorMessage blockLog.input = this.sanitizeInputsForLog(input) blockLog.output = filterOutputForLog(block.metadata?.id || '', errorOutput, { block }) + + if (errorOutput.childTraceSpans && Array.isArray(errorOutput.childTraceSpans)) { + blockLog.childTraceSpans = errorOutput.childTraceSpans + } } logger.error( diff --git a/apps/sim/executor/handlers/workflow/workflow-handler.test.ts b/apps/sim/executor/handlers/workflow/workflow-handler.test.ts index 21d9cd3fd..9c8eb70a2 100644 --- a/apps/sim/executor/handlers/workflow/workflow-handler.test.ts +++ b/apps/sim/executor/handlers/workflow/workflow-handler.test.ts @@ -118,7 +118,7 @@ describe('WorkflowBlockHandler', () => { } await expect(handler.execute(deepContext, mockBlock, inputs)).rejects.toThrow( - 'Error in child workflow "child-workflow-id": Maximum workflow nesting depth of 10 exceeded' + '"child-workflow-id" failed: Maximum workflow nesting depth of 10 exceeded' ) }) @@ -132,7 +132,7 @@ describe('WorkflowBlockHandler', () => { }) await expect(handler.execute(mockContext, mockBlock, inputs)).rejects.toThrow( - 'Error in child workflow "non-existent-workflow": Child workflow non-existent-workflow not found' + '"non-existent-workflow" failed: Child workflow non-existent-workflow not found' ) }) @@ -142,7 +142,7 @@ describe('WorkflowBlockHandler', () => { mockFetch.mockRejectedValueOnce(new Error('Network error')) await expect(handler.execute(mockContext, mockBlock, inputs)).rejects.toThrow( - 'Error in child workflow "child-workflow-id": Network error' + '"child-workflow-id" failed: Network error' ) }) }) @@ -212,7 +212,7 @@ describe('WorkflowBlockHandler', () => { expect(() => (handler as any).mapChildOutputToParent(childResult, 'child-id', 'Child Workflow', 100) - ).toThrow('Error in child workflow "Child Workflow": Child workflow failed') + ).toThrow('"Child Workflow" failed: Child workflow failed') try { ;(handler as any).mapChildOutputToParent(childResult, 'child-id', 'Child Workflow', 100) diff --git a/apps/sim/executor/handlers/workflow/workflow-handler.ts b/apps/sim/executor/handlers/workflow/workflow-handler.ts index 3ec7319eb..b770c101a 100644 --- a/apps/sim/executor/handlers/workflow/workflow-handler.ts +++ b/apps/sim/executor/handlers/workflow/workflow-handler.ts @@ -52,6 +52,11 @@ export class WorkflowBlockHandler implements BlockHandler { throw new Error('No workflow selected for execution') } + // Initialize with registry name, will be updated with loaded workflow name + const { workflows } = useWorkflowRegistry.getState() + const workflowMetadata = workflows[workflowId] + let childWorkflowName = workflowMetadata?.name || workflowId + try { const currentDepth = (ctx.workflowId?.split('_sub_').length || 1) - 1 if (currentDepth >= DEFAULTS.MAX_WORKFLOW_DEPTH) { @@ -75,9 +80,8 @@ export class WorkflowBlockHandler implements BlockHandler { throw new Error(`Child workflow ${workflowId} not found`) } - const { workflows } = useWorkflowRegistry.getState() - const workflowMetadata = workflows[workflowId] - const childWorkflowName = workflowMetadata?.name || childWorkflow.name || 'Unknown Workflow' + // Update with loaded workflow name (more reliable than registry) + childWorkflowName = workflowMetadata?.name || childWorkflow.name || 'Unknown Workflow' logger.info( `Executing child workflow: ${childWorkflowName} (${workflowId}) at depth ${currentDepth}` @@ -142,11 +146,6 @@ export class WorkflowBlockHandler implements BlockHandler { } catch (error: unknown) { logger.error(`Error executing child workflow ${workflowId}:`, error) - const { workflows } = useWorkflowRegistry.getState() - const workflowMetadata = workflows[workflowId] - const childWorkflowName = workflowMetadata?.name || workflowId - - const originalError = error instanceof Error ? error.message : 'Unknown error' let childTraceSpans: WorkflowTraceSpan[] = [] let executionResult: ExecutionResult | undefined @@ -165,8 +164,11 @@ export class WorkflowBlockHandler implements BlockHandler { childTraceSpans = error.childTraceSpans } + // Build a cleaner error message for nested workflow errors + const errorMessage = this.buildNestedWorkflowErrorMessage(childWorkflowName, error) + throw new ChildWorkflowError({ - message: `Error in child workflow "${childWorkflowName}": ${originalError}`, + message: errorMessage, childWorkflowName, childTraceSpans, executionResult, @@ -175,6 +177,72 @@ export class WorkflowBlockHandler implements BlockHandler { } } + /** + * Builds a cleaner error message for nested workflow errors. + * Parses nested error messages to extract workflow chain and root error. + */ + private buildNestedWorkflowErrorMessage(childWorkflowName: string, error: unknown): string { + const originalError = error instanceof Error ? error.message : 'Unknown error' + + // Extract any nested workflow names from the error message + const { chain, rootError } = this.parseNestedWorkflowError(originalError) + + // Add current workflow to the beginning of the chain + chain.unshift(childWorkflowName) + + // If we have a chain (nested workflows), format nicely + if (chain.length > 1) { + return `Workflow chain: ${chain.join(' → ')} | ${rootError}` + } + + // Single workflow failure + return `"${childWorkflowName}" failed: ${rootError}` + } + + /** + * Parses a potentially nested workflow error message to extract: + * - The chain of workflow names + * - The actual root error message (preserving the block prefix for the failing block) + * + * Handles formats like: + * - "workflow-name" failed: error + * - [block_type] Block Name: "workflow-name" failed: error + * - Workflow chain: A → B | error + */ + private parseNestedWorkflowError(message: string): { chain: string[]; rootError: string } { + const chain: string[] = [] + const remaining = message + + // First, check if it's already in chain format + const chainMatch = remaining.match(/^Workflow chain: (.+?) \| (.+)$/) + if (chainMatch) { + const chainPart = chainMatch[1] + const errorPart = chainMatch[2] + chain.push(...chainPart.split(' → ').map((s) => s.trim())) + return { chain, rootError: errorPart } + } + + // Extract workflow names from patterns like: + // - "workflow-name" failed: + // - [block_type] Block Name: "workflow-name" failed: + const workflowPattern = /(?:\[[^\]]+\]\s*[^:]+:\s*)?"([^"]+)"\s*failed:\s*/g + let match: RegExpExecArray | null + let lastIndex = 0 + + match = workflowPattern.exec(remaining) + while (match !== null) { + chain.push(match[1]) + lastIndex = match.index + match[0].length + match = workflowPattern.exec(remaining) + } + + // The root error is everything after the last match + // Keep the block prefix (e.g., [function] Function 1:) so we know which block failed + const rootError = lastIndex > 0 ? remaining.slice(lastIndex) : remaining + + return { chain, rootError: rootError.trim() || 'Unknown error' } + } + private async loadChildWorkflow(workflowId: string) { const headers = await buildAuthHeaders() const url = buildAPIUrl(`/api/workflows/${workflowId}`) @@ -444,7 +512,7 @@ export class WorkflowBlockHandler implements BlockHandler { if (!success) { logger.warn(`Child workflow ${childWorkflowName} failed`) throw new ChildWorkflowError({ - message: `Error in child workflow "${childWorkflowName}": ${childResult.error || 'Child workflow execution failed'}`, + message: `"${childWorkflowName}" failed: ${childResult.error || 'Child workflow execution failed'}`, childWorkflowName, childTraceSpans: childTraceSpans || [], }) diff --git a/apps/sim/executor/types.ts b/apps/sim/executor/types.ts index 9752b7701..6c87eed25 100644 --- a/apps/sim/executor/types.ts +++ b/apps/sim/executor/types.ts @@ -114,6 +114,12 @@ export interface BlockLog { loopId?: string parallelId?: string iterationIndex?: number + /** + * Child workflow trace spans for nested workflow execution. + * Stored separately from output to keep output clean for display + * while preserving data for trace-spans processing. + */ + childTraceSpans?: TraceSpan[] } export interface ExecutionMetadata { diff --git a/apps/sim/executor/utils/output-filter.ts b/apps/sim/executor/utils/output-filter.ts index ca1e2a933..be28f48a5 100644 --- a/apps/sim/executor/utils/output-filter.ts +++ b/apps/sim/executor/utils/output-filter.ts @@ -1,3 +1,4 @@ +import { filterHiddenOutputKeys } from '@/lib/logs/execution/trace-spans/trace-spans' import { getBlock } from '@/blocks' import { isHiddenFromDisplay } from '@/blocks/types' import { isTriggerBehavior, isTriggerInternalKey } from '@/executor/constants' @@ -7,6 +8,7 @@ import type { SerializedBlock } from '@/serializer/types' /** * Filters block output for logging/display purposes. * Removes internal fields and fields marked with hiddenFromDisplay. + * Also recursively filters globally hidden keys from nested objects. * * @param blockType - The block type string (e.g., 'human_in_the_loop', 'workflow') * @param output - The raw block output to filter @@ -44,7 +46,8 @@ export function filterOutputForLog( continue } - filtered[key] = value + // Recursively filter globally hidden keys from nested objects + filtered[key] = filterHiddenOutputKeys(value) } return filtered diff --git a/apps/sim/lib/logs/execution/trace-spans/trace-spans.ts b/apps/sim/lib/logs/execution/trace-spans/trace-spans.ts index 33a3d5619..d729ff189 100644 --- a/apps/sim/lib/logs/execution/trace-spans/trace-spans.ts +++ b/apps/sim/lib/logs/execution/trace-spans/trace-spans.ts @@ -5,6 +5,39 @@ import type { ExecutionResult } from '@/executor/types' const logger = createLogger('TraceSpans') +/** + * Keys that should be recursively filtered from output display. + * These are internal fields used for execution tracking that shouldn't be shown to users. + */ +const HIDDEN_OUTPUT_KEYS = new Set(['childTraceSpans']) + +/** + * Recursively filters hidden keys from nested objects for cleaner display. + * Used by both executor (for log output) and UI (for display). + */ +export function filterHiddenOutputKeys(value: unknown): unknown { + if (value === null || value === undefined) { + return value + } + + if (Array.isArray(value)) { + return value.map((item) => filterHiddenOutputKeys(item)) + } + + if (typeof value === 'object') { + const filtered: Record = {} + for (const [key, val] of Object.entries(value as Record)) { + if (HIDDEN_OUTPUT_KEYS.has(key)) { + continue + } + filtered[key] = filterHiddenOutputKeys(val) + } + return filtered + } + + return value +} + function isSyntheticWorkflowWrapper(span: TraceSpan | undefined): boolean { if (!span || span.type !== 'workflow') return false return !span.blockId @@ -21,43 +54,34 @@ function flattenWorkflowChildren(spans: TraceSpan[]): TraceSpan[] { return } - const processedSpan = ensureNestedWorkflowsProcessed(span) + const processedSpan: TraceSpan = { ...span } + + const directChildren = Array.isArray(span.children) ? span.children : [] + const outputChildren = + span.output && + typeof span.output === 'object' && + Array.isArray((span.output as { childTraceSpans?: TraceSpan[] }).childTraceSpans) + ? ((span.output as { childTraceSpans?: TraceSpan[] }).childTraceSpans as TraceSpan[]) + : [] + + const allChildren = [...directChildren, ...outputChildren] + if (allChildren.length > 0) { + processedSpan.children = flattenWorkflowChildren(allChildren) + } + + if (outputChildren.length > 0 && processedSpan.output) { + const { childTraceSpans: _, ...cleanOutput } = processedSpan.output as { + childTraceSpans?: TraceSpan[] + } & Record + processedSpan.output = cleanOutput + } + flattened.push(processedSpan) }) return flattened } -function getTraceSpanKey(span: TraceSpan): string { - if (span.id) { - return span.id - } - - const name = span.name || 'span' - const start = span.startTime || 'unknown-start' - const end = span.endTime || 'unknown-end' - - return `${name}|${start}|${end}` -} - -function mergeTraceSpanChildren(...childGroups: TraceSpan[][]): TraceSpan[] { - const merged: TraceSpan[] = [] - const seen = new Set() - - childGroups.forEach((group) => { - group.forEach((child) => { - const key = getTraceSpanKey(child) - if (seen.has(key)) { - return - } - seen.add(key) - merged.push(child) - }) - }) - - return merged -} - export function buildTraceSpans(result: ExecutionResult): { traceSpans: TraceSpan[] totalDuration: number @@ -318,19 +342,24 @@ export function buildTraceSpans(result: ExecutionResult): { } } - if ( - isWorkflowBlockType(log.blockType) && - log.output?.childTraceSpans && - Array.isArray(log.output.childTraceSpans) - ) { - const childTraceSpans = log.output.childTraceSpans as TraceSpan[] - const flattenedChildren = flattenWorkflowChildren(childTraceSpans) - span.children = mergeTraceSpanChildren(span.children || [], flattenedChildren) + if (isWorkflowBlockType(log.blockType)) { + const childTraceSpans = Array.isArray(log.childTraceSpans) + ? log.childTraceSpans + : Array.isArray(log.output?.childTraceSpans) + ? (log.output.childTraceSpans as TraceSpan[]) + : null - const { childTraceSpans: _, ...cleanOutput } = span.output as { - childTraceSpans?: TraceSpan[] - } & Record - span.output = cleanOutput + if (childTraceSpans) { + const flattenedChildren = flattenWorkflowChildren(childTraceSpans) + span.children = flattenedChildren + + if (span.output && typeof span.output === 'object' && 'childTraceSpans' in span.output) { + const { childTraceSpans: _, ...cleanOutput } = span.output as { + childTraceSpans?: TraceSpan[] + } & Record + span.output = cleanOutput + } + } } spanMap.set(spanId, span) @@ -730,47 +759,3 @@ function groupIterationBlocks(spans: TraceSpan[]): TraceSpan[] { return result } - -function ensureNestedWorkflowsProcessed(span: TraceSpan): TraceSpan { - const processedSpan: TraceSpan = { ...span } - - if (processedSpan.output && typeof processedSpan.output === 'object') { - processedSpan.output = { ...processedSpan.output } - } - - const normalizedChildren = Array.isArray(span.children) - ? span.children.map((child) => ensureNestedWorkflowsProcessed(child)) - : [] - - const outputChildSpans = (() => { - if (!processedSpan.output || typeof processedSpan.output !== 'object') { - return [] as TraceSpan[] - } - - const maybeChildSpans = (processedSpan.output as { childTraceSpans?: TraceSpan[] }) - .childTraceSpans - if (!Array.isArray(maybeChildSpans) || maybeChildSpans.length === 0) { - return [] as TraceSpan[] - } - - return flattenWorkflowChildren(maybeChildSpans) - })() - - const mergedChildren = mergeTraceSpanChildren(normalizedChildren, outputChildSpans) - - if ( - processedSpan.output && - typeof processedSpan.output === 'object' && - processedSpan.output !== null && - 'childTraceSpans' in processedSpan.output - ) { - const { childTraceSpans, ...cleanOutput } = processedSpan.output as { - childTraceSpans?: TraceSpan[] - } & Record - processedSpan.output = cleanOutput - } - - processedSpan.children = mergedChildren.length > 0 ? mergedChildren : undefined - - return processedSpan -}