From 8cba1004888f60ca653ee069501c13b3cadcc561 Mon Sep 17 00:00:00 2001 From: Ivan Atanasov Date: Thu, 7 Nov 2024 10:14:49 +0100 Subject: [PATCH] fix(editor): Show only error title and 'Open errored node' button; hide 'Ask Assistant' in root for sub-node errors (#11573) --- .../components/Error/NodeErrorView.test.ts | 154 ++++++++++++++---- .../src/components/Error/NodeErrorView.vue | 55 ++++--- .../src/plugins/i18n/locales/en.json | 2 +- 3 files changed, 155 insertions(+), 56 deletions(-) diff --git a/packages/editor-ui/src/components/Error/NodeErrorView.test.ts b/packages/editor-ui/src/components/Error/NodeErrorView.test.ts index 0813b2a781..af9960bc62 100644 --- a/packages/editor-ui/src/components/Error/NodeErrorView.test.ts +++ b/packages/editor-ui/src/components/Error/NodeErrorView.test.ts @@ -1,38 +1,61 @@ import { createComponentRenderer } from '@/__tests__/render'; -import { SETTINGS_STORE_DEFAULT_STATE } from '@/__tests__/utils'; + import NodeErrorView from '@/components/Error/NodeErrorView.vue'; -import { STORES } from '@/constants'; + import { createTestingPinia } from '@pinia/testing'; -import { type INode } from 'n8n-workflow'; +import type { NodeError } from 'n8n-workflow'; import { useAssistantStore } from '@/stores/assistant.store'; import { useNodeTypesStore } from '@/stores/nodeTypes.store'; +import { mockedStore } from '@/__tests__/utils'; +import userEvent from '@testing-library/user-event'; +import { useNDVStore } from '@/stores/ndv.store'; -const DEFAULT_SETUP = { - pinia: createTestingPinia({ - initialState: { - [STORES.SETTINGS]: SETTINGS_STORE_DEFAULT_STATE, - }, - }), -}; +const renderComponent = createComponentRenderer(NodeErrorView); -const renderComponent = createComponentRenderer(NodeErrorView, DEFAULT_SETUP); +let mockAiAssistantStore: ReturnType>; +let mockNodeTypeStore: ReturnType>; +let mockNdvStore: ReturnType>; describe('NodeErrorView.vue', () => { - let mockNode: INode; - afterEach(() => { - mockNode = { - parameters: { - mode: 'runOnceForAllItems', - language: 'javaScript', - jsCode: 'cons error = 9;', - notice: '', + let error: NodeError; + + beforeEach(() => { + createTestingPinia(); + + mockAiAssistantStore = mockedStore(useAssistantStore); + mockNodeTypeStore = mockedStore(useNodeTypesStore); + mockNdvStore = mockedStore(useNDVStore); + //@ts-expect-error + error = { + name: 'NodeOperationError', + message: 'Test error message', + description: 'Test error description', + context: { + descriptionKey: 'noInputConnection', + nodeCause: 'Test node cause', + runIndex: '1', + itemIndex: '2', + parameter: 'testParameter', + data: { key: 'value' }, + causeDetailed: 'Detailed cause', }, - id: 'd1ce5dc9-f9ae-4ac6-84e5-0696ba175dd9', - name: 'Code', - type: 'n8n-nodes-base.code', - typeVersion: 2, - position: [940, 240], + node: { + parameters: { + mode: 'runOnceForAllItems', + language: 'javaScript', + jsCode: 'cons error = 9;', + notice: '', + }, + id: 'd1ce5dc9-f9ae-4ac6-84e5-0696ba175dd9', + name: 'ErrorCode', + type: 'n8n-nodes-base.code', + typeVersion: 2, + position: [940, 240], + }, + stack: 'Test stack trace', }; + }); + afterEach(() => { vi.clearAllMocks(); }); @@ -40,7 +63,7 @@ describe('NodeErrorView.vue', () => { const { getByTestId } = renderComponent({ props: { error: { - node: mockNode, + node: error.node, messages: ['Unexpected identifier [line 1]'], }, }, @@ -55,7 +78,7 @@ describe('NodeErrorView.vue', () => { const { getByTestId } = renderComponent({ props: { error: { - node: mockNode, + node: error.node, message: 'Unexpected identifier [line 1]', }, }, @@ -67,24 +90,20 @@ describe('NodeErrorView.vue', () => { }); it('should not render AI assistant button when error happens in deprecated function node', async () => { - const aiAssistantStore = useAssistantStore(DEFAULT_SETUP.pinia); - const nodeTypeStore = useNodeTypesStore(DEFAULT_SETUP.pinia); - //@ts-expect-error - nodeTypeStore.getNodeType = vi.fn(() => ({ + mockNodeTypeStore.getNodeType = vi.fn(() => ({ type: 'n8n-nodes-base.function', typeVersion: 1, hidden: true, })); - //@ts-expect-error - aiAssistantStore.canShowAssistantButtonsOnCanvas = true; + mockAiAssistantStore.canShowAssistantButtonsOnCanvas = true; const { queryByTestId } = renderComponent({ props: { error: { node: { - ...mockNode, + ...error.node, type: 'n8n-nodes-base.function', typeVersion: 1, }, @@ -96,4 +115,73 @@ describe('NodeErrorView.vue', () => { expect(aiAssistantButton).toBeNull(); }); + + it('renders error message', () => { + const { getByTestId } = renderComponent({ + props: { error }, + }); + expect(getByTestId('node-error-message').textContent).toContain('Test error message'); + }); + + it('renders error description', () => { + const { getByTestId } = renderComponent({ + props: { error }, + }); + expect(getByTestId('node-error-description').innerHTML).toContain( + 'This node has no input data. Please make sure this node is connected to another node.', + ); + }); + + it('renders stack trace', () => { + const { getByText } = renderComponent({ + props: { error }, + }); + expect(getByText('Test stack trace')).toBeTruthy(); + }); + + it('renders open node button when the error is in sub node', () => { + const { getByTestId, queryByTestId } = renderComponent({ + props: { + error: { + ...error, + name: 'NodeOperationError', + functionality: 'configuration-node', + }, + }, + }); + + expect(getByTestId('node-error-view-open-node-button')).toHaveTextContent('Open errored node'); + + expect(queryByTestId('ask-assistant-button')).not.toBeInTheDocument(); + }); + + it('does not renders open node button when the error is in sub node', () => { + mockAiAssistantStore.canShowAssistantButtonsOnCanvas = true; + const { getByTestId, queryByTestId } = renderComponent({ + props: { + error, + }, + }); + + expect(queryByTestId('node-error-view-open-node-button')).not.toBeInTheDocument(); + + expect(getByTestId('ask-assistant-button')).toBeInTheDocument(); + }); + + it('open error node details when open error node is clicked', async () => { + const { getByTestId, emitted } = renderComponent({ + props: { + error: { + ...error, + name: 'NodeOperationError', + functionality: 'configuration-node', + }, + }, + }); + + await userEvent.click(getByTestId('node-error-view-open-node-button')); + + expect(emitted().click).toHaveLength(1); + expect(mockNdvStore.activeNodeName).toBe(error.node.name); + }); }); diff --git a/packages/editor-ui/src/components/Error/NodeErrorView.vue b/packages/editor-ui/src/components/Error/NodeErrorView.vue index 76965f0632..f1b8a1f7c6 100644 --- a/packages/editor-ui/src/components/Error/NodeErrorView.vue +++ b/packages/editor-ui/src/components/Error/NodeErrorView.vue @@ -117,7 +117,7 @@ const prepareRawMessages = computed(() => { }); const isAskAssistantAvailable = computed(() => { - if (!node.value) { + if (!node.value || isSubNodeError.value) { return false; } const isCustomNode = node.value.type === undefined || isCommunityPackageName(node.value.type); @@ -132,6 +132,13 @@ const assistantAlreadyAsked = computed(() => { }); }); +const isSubNodeError = computed(() => { + return ( + props.error.name === 'NodeOperationError' && + (props.error as NodeOperationError).functionality === 'configuration-node' + ); +}); + function nodeVersionTag(nodeType: NodeError['node']): string { if (!nodeType || ('hidden' in nodeType && nodeType.hidden)) { return i18n.baseText('nodeSettings.deprecated'); @@ -153,19 +160,6 @@ function prepareDescription(description: string): string { } function getErrorDescription(): string { - const isSubNodeError = - props.error.name === 'NodeOperationError' && - (props.error as NodeOperationError).functionality === 'configuration-node'; - - if (isSubNodeError) { - return prepareDescription( - props.error.description + - i18n.baseText('pushConnection.executionError.openNode', { - interpolate: { node: props.error.node.name }, - }), - ); - } - if (props.error.context?.descriptionKey) { const interpolate = { nodeCause: props.error.context.nodeCause as string, @@ -205,13 +199,10 @@ function addItemIndexSuffix(message: string): string { function getErrorMessage(): string { let message = ''; - const isSubNodeError = - props.error.name === 'NodeOperationError' && - (props.error as NodeOperationError).functionality === 'configuration-node'; const isNonEmptyString = (value?: unknown): value is string => !!value && typeof value === 'string'; - if (isSubNodeError) { + if (isSubNodeError.value) { message = i18n.baseText('nodeErrorView.errorSubNode', { interpolate: { node: props.error.node.name }, }); @@ -390,6 +381,10 @@ function nodeIsHidden() { return nodeType?.hidden ?? false; } +const onOpenErrorNodeDetailClick = () => { + ndvStore.activeNodeName = props.error.node.name; +}; + async function onAskAssistantClick() { const { message, lineNumber, description } = props.error; const sessionInProgress = !assistantStore.isSessionEnded; @@ -428,14 +423,25 @@ async function onAskAssistantClick() {
+ +
+ +
@@ -696,9 +702,14 @@ async function onAskAssistantClick() { } } - &__assistant-button { + &__button { margin-left: var(--spacing-s); margin-bottom: var(--spacing-xs); + flex-direction: row-reverse; + span { + margin-right: var(--spacing-5xs); + margin-left: var(--spacing-5xs); + } } &__debugging { @@ -831,7 +842,7 @@ async function onAskAssistantClick() { } } -.node-error-view__assistant-button { +.node-error-view__button { margin-top: var(--spacing-xs); } diff --git a/packages/editor-ui/src/plugins/i18n/locales/en.json b/packages/editor-ui/src/plugins/i18n/locales/en.json index d3d5f4d9bf..bec1cd473b 100644 --- a/packages/editor-ui/src/plugins/i18n/locales/en.json +++ b/packages/editor-ui/src/plugins/i18n/locales/en.json @@ -1498,7 +1498,7 @@ "pushConnection.executionFailed": "Execution failed", "pushConnection.executionFailed.message": "There might not be enough memory to finish the execution. Tips for avoiding this here", "pushConnection.executionError": "There was a problem executing the workflow{error}", - "pushConnection.executionError.openNode": " Open node", + "pushConnection.executionError.openNode": "Open errored node", "pushConnection.executionError.details": "
{details}", "prompts.productTeamMessage": "Our product team will get in touch personally", "prompts.npsSurvey.recommendationQuestion": "How likely are you to recommend n8n to a friend or colleague?",