From 11cf1cd23a181714e445ef58e97fdd7dca870dd7 Mon Sep 17 00:00:00 2001 From: Charlie Kolb Date: Thu, 13 Feb 2025 09:44:52 +0100 Subject: [PATCH] feat(editor): Expose `View Execution` links for erroneous sub-executions (#13185) --- .../ChainRetrievalQA/ChainRetrievalQa.node.ts | 8 +- .../v2/utils/WorkflowToolService.ts | 5 +- .../@n8n/nodes-langchain/utils/logWrapper.ts | 4 +- .../src/workflow-execute-additional-data.ts | 4 + .../editor-ui/src/components/RunData.test.ts | 51 +++++++++++++ packages/editor-ui/src/components/RunData.vue | 32 ++++++-- .../ExecuteWorkflow/ExecuteWorkflow.node.ts | 19 ++++- packages/workflow/src/MetadataUtils.ts | 31 ++++++++ .../workflow/src/errors/node-api.error.ts | 5 ++ .../src/errors/node-operation.error.ts | 1 + packages/workflow/src/index.ts | 1 + packages/workflow/src/utils.ts | 7 ++ packages/workflow/test/MetadataUtils.test.ts | 30 ++++++++ packages/workflow/test/utils.test.ts | 76 +++++++++++++++++++ 14 files changed, 261 insertions(+), 13 deletions(-) create mode 100644 packages/workflow/src/MetadataUtils.ts create mode 100644 packages/workflow/test/MetadataUtils.test.ts diff --git a/packages/@n8n/nodes-langchain/nodes/chains/ChainRetrievalQA/ChainRetrievalQa.node.ts b/packages/@n8n/nodes-langchain/nodes/chains/ChainRetrievalQA/ChainRetrievalQa.node.ts index 7829bc7813..e20128a702 100644 --- a/packages/@n8n/nodes-langchain/nodes/chains/ChainRetrievalQA/ChainRetrievalQa.node.ts +++ b/packages/@n8n/nodes-langchain/nodes/chains/ChainRetrievalQA/ChainRetrievalQa.node.ts @@ -14,6 +14,7 @@ import { type INodeType, type INodeTypeDescription, NodeOperationError, + parseErrorMetadata, } from 'n8n-workflow'; import { promptTypeOptions, textFromPreviousNode } from '@utils/descriptions'; @@ -229,7 +230,12 @@ export class ChainRetrievalQa implements INodeType { returnData.push({ json: { response } }); } catch (error) { if (this.continueOnFail()) { - returnData.push({ json: { error: error.message }, pairedItem: { item: itemIndex } }); + const metadata = parseErrorMetadata(error); + returnData.push({ + json: { error: error.message }, + pairedItem: { item: itemIndex }, + metadata, + }); continue; } diff --git a/packages/@n8n/nodes-langchain/nodes/tools/ToolWorkflow/v2/utils/WorkflowToolService.ts b/packages/@n8n/nodes-langchain/nodes/tools/ToolWorkflow/v2/utils/WorkflowToolService.ts index 3930938370..f3089239fe 100644 --- a/packages/@n8n/nodes-langchain/nodes/tools/ToolWorkflow/v2/utils/WorkflowToolService.ts +++ b/packages/@n8n/nodes-langchain/nodes/tools/ToolWorkflow/v2/utils/WorkflowToolService.ts @@ -24,6 +24,7 @@ import { jsonParse, NodeConnectionType, NodeOperationError, + parseErrorMetadata, traverseNodeParameters, } from 'n8n-workflow'; import { z } from 'zod'; @@ -92,7 +93,9 @@ export class WorkflowToolService { } catch (error) { const executionError = error as ExecutionError; const errorResponse = `There was an error: "${executionError.message}"`; - void this.context.addOutputData(NodeConnectionType.AiTool, index, executionError); + + const metadata = parseErrorMetadata(error); + void this.context.addOutputData(NodeConnectionType.AiTool, index, executionError, metadata); return errorResponse; } finally { // @ts-expect-error this accesses a private member on the actual implementation to fix https://linear.app/n8n/issue/ADO-3186/bug-workflowtool-v2-always-uses-first-row-of-input-data diff --git a/packages/@n8n/nodes-langchain/utils/logWrapper.ts b/packages/@n8n/nodes-langchain/utils/logWrapper.ts index 073b005162..699bf39395 100644 --- a/packages/@n8n/nodes-langchain/utils/logWrapper.ts +++ b/packages/@n8n/nodes-langchain/utils/logWrapper.ts @@ -16,7 +16,7 @@ import type { ISupplyDataFunctions, ITaskMetadata, } from 'n8n-workflow'; -import { NodeOperationError, NodeConnectionType } from 'n8n-workflow'; +import { NodeOperationError, NodeConnectionType, parseErrorMetadata } from 'n8n-workflow'; import { logAiEvent, isToolsInstance, isBaseChatMemory, isBaseChatMessageHistory } from './helpers'; import { N8nBinaryLoader } from './N8nBinaryLoader'; @@ -41,10 +41,12 @@ export async function callMethodAsync( functionality: 'configuration-node', }); + const metadata = parseErrorMetadata(error); parameters.executeFunctions.addOutputData( parameters.connectionType, parameters.currentNodeRunIndex, error, + metadata, ); if (error.message) { diff --git a/packages/cli/src/workflow-execute-additional-data.ts b/packages/cli/src/workflow-execute-additional-data.ts index 1c5dfcbf67..a83ee5b263 100644 --- a/packages/cli/src/workflow-execute-additional-data.ts +++ b/packages/cli/src/workflow-execute-additional-data.ts @@ -295,6 +295,8 @@ async function startExecution( throw objectToError( { ...executionError, + executionId, + workflowId: workflowData.id, stack: executionError?.stack, message: executionError?.message, }, @@ -322,6 +324,8 @@ async function startExecution( throw objectToError( { ...error, + executionId, + workflowId: workflowData.id, stack: error?.stack, }, workflow, diff --git a/packages/editor-ui/src/components/RunData.test.ts b/packages/editor-ui/src/components/RunData.test.ts index fb6dcbcdb3..b47f1eede9 100644 --- a/packages/editor-ui/src/components/RunData.test.ts +++ b/packages/editor-ui/src/components/RunData.test.ts @@ -387,6 +387,57 @@ describe('RunData', () => { expect(trackOpeningRelatedExecution).toHaveBeenCalledWith(metadata, 'json'); }); + it('should render sub-execution link in header with sub-node error', async () => { + const metadata = { + subExecution: { + workflowId: 'xyz', + executionId: '123', + }, + subExecutionsCount: 1, + }; + + const { getByTestId } = render({ + defaultRunItems: [ + { + json: {}, + }, + ], + displayMode: 'table', + paneType: 'output', + runs: [ + { + hints: [], + startTime: 1737643696893, + executionTime: 2, + source: [ + { + previousNode: 'When clicking ‘Test workflow’', + }, + ], + executionStatus: 'error', + error: { + level: 'error', + errorResponse: { + ...metadata.subExecution, + }, + } as never, + }, + ], + }); + + expect(getByTestId('related-execution-link')).toBeInTheDocument(); + expect(getByTestId('related-execution-link')).toHaveTextContent('View sub-execution'); + expect(resolveRelatedExecutionUrl).toHaveBeenCalledWith(metadata); + expect(getByTestId('related-execution-link')).toHaveAttribute('href', MOCK_EXECUTION_URL); + + expect(getByTestId('ndv-items-count')).toHaveTextContent( + '1 item, 1 sub-execution View sub-execution', + ); + + getByTestId('related-execution-link').click(); + expect(trackOpeningRelatedExecution).toHaveBeenCalledWith(metadata, 'table'); + }); + it('should render input selector when input node has error', async () => { const testNodes = [ { diff --git a/packages/editor-ui/src/components/RunData.vue b/packages/editor-ui/src/components/RunData.vue index bf7175e68d..2ff86f2374 100644 --- a/packages/editor-ui/src/components/RunData.vue +++ b/packages/editor-ui/src/components/RunData.vue @@ -12,8 +12,9 @@ import { type ITaskMetadata, type NodeError, type NodeHint, - type Workflow, TRIMMED_TASK_DATA_CONNECTIONS_KEY, + type Workflow, + parseErrorMetadata, } from 'n8n-workflow'; import { NodeConnectionType, NodeHelpers } from 'n8n-workflow'; import { computed, defineAsyncComponent, onBeforeUnmount, onMounted, ref, toRef, watch } from 'vue'; @@ -300,6 +301,12 @@ const subworkflowExecutionError = computed(() => { const hasSubworkflowExecutionError = computed(() => Boolean(workflowsStore.subWorkflowExecutionError), ); + +// Sub-nodes may wish to display the parent node error as it can contain additional metadata +const parentNodeError = computed(() => { + const parentNode = props.workflow.getChildNodes(node.value?.name ?? '', 'ALL_NON_MAIN')[0]; + return workflowRunData.value?.[parentNode]?.[props.runIndex]?.error as NodeError; +}); const workflowRunErrorAsNodeError = computed(() => { if (!node.value) { return null; @@ -307,8 +314,7 @@ const workflowRunErrorAsNodeError = computed(() => { // If the node is a sub-node, we need to get the parent node error to check for input errors if (isSubNodeType.value && props.paneType === 'input') { - const parentNode = props.workflow.getChildNodes(node.value?.name ?? '', 'ALL_NON_MAIN')[0]; - return workflowRunData.value?.[parentNode]?.[props.runIndex]?.error as NodeError; + return parentNodeError.value; } return workflowRunData.value?.[node.value?.name]?.[props.runIndex]?.error as NodeError; }); @@ -533,13 +539,25 @@ const activeTaskMetadata = computed((): ITaskMetadata | null => { if (!node.value) { return null; } + const errorMetadata = parseErrorMetadata(workflowRunErrorAsNodeError.value); + if (errorMetadata !== undefined) { + return errorMetadata; + } + + // This is needed for the WorkflowRetriever to display the associated execution + if (parentNodeError.value) { + const subNodeMetadata = parseErrorMetadata(parentNodeError.value); + if (subNodeMetadata !== undefined) { + return subNodeMetadata; + } + } return workflowRunData.value?.[node.value.name]?.[props.runIndex]?.metadata ?? null; }); -const hasReleatedExectuion = computed((): boolean => { +const hasRelatedExecution = computed(() => { return Boolean( - activeTaskMetadata.value?.subExecution || activeTaskMetadata.value?.parentExecution, + activeTaskMetadata.value?.subExecution ?? activeTaskMetadata.value?.parentExecution, ); }); @@ -1483,7 +1501,7 @@ defineExpose({ enterEditMode }); hasKey(response, x) && typeof response[x] === 'string', + ); +} + +type ISubWorkflowMetadata = Required>; + +function parseErrorResponseWorkflowMetadata(response: unknown): ISubWorkflowMetadata | undefined { + if (!responseHasSubworkflowData(response)) return undefined; + + return { + subExecution: { + executionId: response.executionId, + workflowId: response.workflowId, + }, + subExecutionsCount: 1, + }; +} + +export function parseErrorMetadata(error: unknown): ISubWorkflowMetadata | undefined { + if (hasKey(error, 'errorResponse')) { + return parseErrorResponseWorkflowMetadata(error.errorResponse); + } + return undefined; +} diff --git a/packages/workflow/src/errors/node-api.error.ts b/packages/workflow/src/errors/node-api.error.ts index aeae1c754e..b631fe933b 100644 --- a/packages/workflow/src/errors/node-api.error.ts +++ b/packages/workflow/src/errors/node-api.error.ts @@ -18,6 +18,7 @@ import type { IDataObject, IStatusCodeMessages, Functionality, + RelatedExecution, } from '../Interfaces'; import { removeCircularRefs } from '../utils'; @@ -30,6 +31,10 @@ export interface NodeOperationErrorOptions { messageMapping?: { [key: string]: string }; // allows to pass custom mapping for error messages scoped to a node functionality?: Functionality; type?: string; + metadata?: { + subExecution?: RelatedExecution; + parentExecution?: RelatedExecution; + }; } interface NodeApiErrorOptions extends NodeOperationErrorOptions { diff --git a/packages/workflow/src/errors/node-operation.error.ts b/packages/workflow/src/errors/node-operation.error.ts index 9203a7ecda..04944a4f29 100644 --- a/packages/workflow/src/errors/node-operation.error.ts +++ b/packages/workflow/src/errors/node-operation.error.ts @@ -35,6 +35,7 @@ export class NodeOperationError extends NodeError { this.description = options.description; this.context.runIndex = options.runIndex; this.context.itemIndex = options.itemIndex; + this.context.metadata = options.metadata; if (this.message === this.description) { this.description = undefined; diff --git a/packages/workflow/src/index.ts b/packages/workflow/src/index.ts index 111bc16a32..7e01f7200f 100644 --- a/packages/workflow/src/index.ts +++ b/packages/workflow/src/index.ts @@ -15,6 +15,7 @@ export * from './ExecutionStatus'; export * from './Expression'; export * from './FromAIParseUtils'; export * from './NodeHelpers'; +export * from './MetadataUtils'; export * from './Workflow'; export * from './WorkflowDataProxy'; export * from './WorkflowDataProxyEnvProvider'; diff --git a/packages/workflow/src/utils.ts b/packages/workflow/src/utils.ts index 4637b4b47c..d758e17324 100644 --- a/packages/workflow/src/utils.ts +++ b/packages/workflow/src/utils.ts @@ -276,3 +276,10 @@ export function randomString(minLength: number, maxLength?: number): string { .map((byte) => ALPHABET[byte % ALPHABET.length]) .join(''); } + +/** + * Checks if a value is an object with a specific key and provides a type guard for the key. + */ +export function hasKey(value: unknown, key: T): value is Record { + return value !== null && typeof value === 'object' && value.hasOwnProperty(key); +} diff --git a/packages/workflow/test/MetadataUtils.test.ts b/packages/workflow/test/MetadataUtils.test.ts new file mode 100644 index 0000000000..22b2fd4916 --- /dev/null +++ b/packages/workflow/test/MetadataUtils.test.ts @@ -0,0 +1,30 @@ +import { parseErrorMetadata } from '@/MetadataUtils'; + +describe('MetadataUtils', () => { + describe('parseMetadataFromError', () => { + it('should return undefined if error does not have response', () => { + const error = { message: 'An error occurred' }; + const result = parseErrorMetadata(error); + expect(result).toBeUndefined(); + }); + + it('should return undefined if error response does not have subworkflow data', () => { + const error = { errorResponse: { someKey: 'someValue' } }; + const result = parseErrorMetadata(error); + expect(result).toBeUndefined(); + }); + + it('should return metadata if error response has subworkflow data', () => { + const error = { errorResponse: { executionId: '123', workflowId: '456' } }; + const expectedMetadata = { + subExecution: { + executionId: '123', + workflowId: '456', + }, + subExecutionsCount: 1, + }; + const result = parseErrorMetadata(error); + expect(result).toEqual(expectedMetadata); + }); + }); +}); diff --git a/packages/workflow/test/utils.test.ts b/packages/workflow/test/utils.test.ts index fe6bccd201..3723f846b0 100644 --- a/packages/workflow/test/utils.test.ts +++ b/packages/workflow/test/utils.test.ts @@ -8,6 +8,7 @@ import { fileTypeFromMimeType, randomInt, randomString, + hasKey, } from '@/utils'; describe('isObjectEmpty', () => { @@ -290,3 +291,78 @@ describe('randomString', () => { }); }); }); + +type Expect = T; +type Equal = (() => T extends X ? 1 : 2) extends () => T extends Y ? 1 : 2 + ? true + : false; + +describe('hasKey', () => { + it('should return false if the input is null', () => { + const x = null; + const result = hasKey(x, 'key'); + + expect(result).toEqual(false); + }); + it('should return false if the input is undefined', () => { + const x = undefined; + const result = hasKey(x, 'key'); + + expect(result).toEqual(false); + }); + it('should return false if the input is a number', () => { + const x = 1; + const result = hasKey(x, 'key'); + + expect(result).toEqual(false); + }); + it('should return false if the input is an array out of bounds', () => { + const x = [1, 2]; + const result = hasKey(x, 5); + + expect(result).toEqual(false); + }); + + it('should return true if the input is an array within bounds', () => { + const x = [1, 2]; + const result = hasKey(x, 1); + + expect(result).toEqual(true); + }); + it('should return true if the input is an array with the key `length`', () => { + const x = [1, 2]; + const result = hasKey(x, 'length'); + + expect(result).toEqual(true); + }); + it('should return false if the input is an array with the key `toString`', () => { + const x = [1, 2]; + const result = hasKey(x, 'toString'); + + expect(result).toEqual(false); + }); + it('should return false if the input is an object without the key', () => { + const x = { a: 3 }; + const result = hasKey(x, 'a'); + + expect(result).toEqual(true); + }); + + it('should return true if the input is an object with the key', () => { + const x = { a: 3 }; + const result = hasKey(x, 'b'); + + expect(result).toEqual(false); + }); + + it('should provide a type guard', () => { + const x: unknown = { a: 3 }; + if (hasKey(x, '0')) { + const y: Expect>> = true; + y; + } else { + const z: Expect> = true; + z; + } + }); +});