From 62ee79689569b5d2c9823afac238e66e4c645d9b Mon Sep 17 00:00:00 2001
From: Elias Meire <elias@meire.dev>
Date: Wed, 22 May 2024 15:54:25 +0200
Subject: [PATCH] fix(editor): Fix node execution errors showing undefined
 (#9487)

---
 cypress/e2e/5-ndv.cy.ts                       |  27 ++-
 .../fixtures/Test_workflow_ndv_run_error.json | 162 ++++++++++++++++++
 cypress/pages/ndv.ts                          |   2 +
 .../src/components/Error/NodeErrorView.vue    |  39 +++--
 .../Error/__tests__/NodeErrorView.test.ts     |  66 +++++++
 packages/editor-ui/src/components/RunData.vue |   7 +-
 6 files changed, 272 insertions(+), 31 deletions(-)
 create mode 100644 cypress/fixtures/Test_workflow_ndv_run_error.json
 create mode 100644 packages/editor-ui/src/components/Error/__tests__/NodeErrorView.test.ts

diff --git a/cypress/e2e/5-ndv.cy.ts b/cypress/e2e/5-ndv.cy.ts
index 04da53af23..76efdb32cc 100644
--- a/cypress/e2e/5-ndv.cy.ts
+++ b/cypress/e2e/5-ndv.cy.ts
@@ -105,13 +105,26 @@ describe('NDV', () => {
 	});
 
 	it('should show all validation errors when opening pasted node', () => {
-		cy.fixture('Test_workflow_ndv_errors.json').then((data) => {
-			cy.get('body').paste(JSON.stringify(data));
-			workflowPage.getters.canvasNodes().should('have.have.length', 1);
-			workflowPage.actions.openNode('Airtable');
-			cy.get('.has-issues').should('have.length', 3);
-			cy.get('[class*=hasIssues]').should('have.length', 1);
-		});
+		cy.createFixtureWorkflow('Test_workflow_ndv_errors.json', 'Validation errors');
+		workflowPage.getters.canvasNodes().should('have.have.length', 1);
+		workflowPage.actions.openNode('Airtable');
+		cy.get('.has-issues').should('have.length', 3);
+		cy.get('[class*=hasIssues]').should('have.length', 1);
+	});
+
+	it('should render run errors correctly', () => {
+		cy.createFixtureWorkflow('Test_workflow_ndv_run_error.json', 'Run error');
+		workflowPage.actions.openNode('Error');
+		ndv.actions.execute();
+		ndv.getters
+			.nodeRunErrorMessage()
+			.should('have.text', 'Info for expression missing from previous node');
+		ndv.getters
+			.nodeRunErrorDescription()
+			.should(
+				'contains.text',
+				"An expression here won't work because it uses .item and n8n can't figure out the matching item.",
+			);
 	});
 
 	it('should save workflow using keyboard shortcut from NDV', () => {
diff --git a/cypress/fixtures/Test_workflow_ndv_run_error.json b/cypress/fixtures/Test_workflow_ndv_run_error.json
new file mode 100644
index 0000000000..45a045851d
--- /dev/null
+++ b/cypress/fixtures/Test_workflow_ndv_run_error.json
@@ -0,0 +1,162 @@
+{
+    "name": "My workflow 52",
+    "nodes": [
+      {
+        "parameters": {
+          "jsCode": "\nreturn [\n  {\n    \"field\": \"the same\"\n  }\n];"
+        },
+        "id": "38c14c4a-7af1-4b04-be76-f8e474c95569",
+        "name": "Break pairedItem chain",
+        "type": "n8n-nodes-base.code",
+        "typeVersion": 2,
+        "position": [
+          240,
+          1020
+        ]
+      },
+      {
+        "parameters": {
+          "options": {}
+        },
+        "id": "78c4964a-c4e8-47e5-81f3-89ba778feb8b",
+        "name": "Edit Fields",
+        "type": "n8n-nodes-base.set",
+        "typeVersion": 3.2,
+        "position": [
+          40,
+          1020
+        ]
+      },
+      {
+        "parameters": {},
+        "id": "4f4c6527-d565-448a-96bd-8f5414caf8cc",
+        "name": "When clicking \"Test workflow\"",
+        "type": "n8n-nodes-base.manualTrigger",
+        "typeVersion": 1,
+        "position": [
+          -180,
+          1020
+        ]
+      },
+      {
+        "parameters": {
+          "fields": {
+            "values": [
+              {
+                "stringValue": "={{ $('Edit Fields').item.json.name }}"
+              }
+            ]
+          },
+          "options": {}
+        },
+        "id": "44f4e5da-bfe9-4dc3-8d1f-f38e9f364754",
+        "name": "Error",
+        "type": "n8n-nodes-base.set",
+        "typeVersion": 3.2,
+        "position": [
+          460,
+          1020
+        ]
+      }
+    ],
+    "pinData": {
+      "Edit Fields": [
+        {
+          "json": {
+            "id": "23423532",
+            "name": "Jay Gatsby",
+            "email": "gatsby@west-egg.com",
+            "notes": "Keeps asking about a green light??",
+            "country": "US",
+            "created": "1925-04-10"
+          }
+        },
+        {
+          "json": {
+            "id": "23423533",
+            "name": "José Arcadio Buendía",
+            "email": "jab@macondo.co",
+            "notes": "Lots of people named after him. Very confusing",
+            "country": "CO",
+            "created": "1967-05-05"
+          }
+        },
+        {
+          "json": {
+            "id": "23423534",
+            "name": "Max Sendak",
+            "email": "info@in-and-out-of-weeks.org",
+            "notes": "Keeps rolling his terrible eyes",
+            "country": "US",
+            "created": "1963-04-09"
+          }
+        },
+        {
+          "json": {
+            "id": "23423535",
+            "name": "Zaphod Beeblebrox",
+            "email": "captain@heartofgold.com",
+            "notes": "Felt like I was talking to more than one person",
+            "country": null,
+            "created": "1979-10-12"
+          }
+        },
+        {
+          "json": {
+            "id": "23423536",
+            "name": "Edmund Pevensie",
+            "email": "edmund@narnia.gov",
+            "notes": "Passionate sailor",
+            "country": "UK",
+            "created": "1950-10-16"
+          }
+        }
+      ]
+    },
+    "connections": {
+      "Break pairedItem chain": {
+        "main": [
+          [
+            {
+              "node": "Error",
+              "type": "main",
+              "index": 0
+            }
+          ]
+        ]
+      },
+      "Edit Fields": {
+        "main": [
+          [
+            {
+              "node": "Break pairedItem chain",
+              "type": "main",
+              "index": 0
+            }
+          ]
+        ]
+      },
+      "When clicking \"Test workflow\"": {
+        "main": [
+          [
+            {
+              "node": "Edit Fields",
+              "type": "main",
+              "index": 0
+            }
+          ]
+        ]
+      }
+    },
+    "active": false,
+    "settings": {
+      "executionOrder": "v1"
+    },
+    "versionId": "ca53267f-4eb4-481d-9e09-ecb97f6b09e2",
+    "meta": {
+      "templateCredsSetupCompleted": true,
+      "instanceId": "27cc9b56542ad45b38725555722c50a1c3fee1670bbb67980558314ee08517c4"
+    },
+    "id": "6fr8GiRyMlZCiDQW",
+    "tags": []
+  }
diff --git a/cypress/pages/ndv.ts b/cypress/pages/ndv.ts
index d9e40e4291..32cc4329b3 100644
--- a/cypress/pages/ndv.ts
+++ b/cypress/pages/ndv.ts
@@ -124,6 +124,8 @@ export class NDV extends BasePage {
 		codeEditorFullscreen: () => this.getters.codeEditorDialog().find('.cm-content'),
 		nodeRunSuccessIndicator: () => cy.getByTestId('node-run-info-success'),
 		nodeRunErrorIndicator: () => cy.getByTestId('node-run-info-danger'),
+		nodeRunErrorMessage: () => cy.getByTestId('node-error-message'),
+		nodeRunErrorDescription: () => cy.getByTestId('node-error-description'),
 	};
 
 	actions = {
diff --git a/packages/editor-ui/src/components/Error/NodeErrorView.vue b/packages/editor-ui/src/components/Error/NodeErrorView.vue
index 7574d17912..1ebfc1ebee 100644
--- a/packages/editor-ui/src/components/Error/NodeErrorView.vue
+++ b/packages/editor-ui/src/components/Error/NodeErrorView.vue
@@ -1,6 +1,5 @@
 <script lang="ts" setup>
 import { useI18n } from '@/composables/useI18n';
-import type { PropType } from 'vue';
 import { computed } from 'vue';
 import { useClipboard } from '@/composables/useClipboard';
 import { useToast } from '@/composables/useToast';
@@ -20,13 +19,11 @@ import { sanitizeHtml } from '@/utils/htmlUtils';
 import { MAX_DISPLAY_DATA_SIZE } from '@/constants';
 import type { BaseTextKey } from '@/plugins/i18n';
 
-const props = defineProps({
-	error: {
-		type: Object as PropType<NodeError | NodeApiError | NodeOperationError>,
-		required: true,
-	},
-});
+type Props = {
+	error: NodeError | NodeApiError | NodeOperationError;
+};
 
+const props = defineProps<Props>();
 const clipboard = useClipboard();
 const toast = useToast();
 const i18n = useI18n();
@@ -36,7 +33,7 @@ const ndvStore = useNDVStore();
 const rootStore = useRootStore();
 
 const displayCause = computed(() => {
-	return JSON.stringify(props.error.cause).length < MAX_DISPLAY_DATA_SIZE;
+	return JSON.stringify(props.error.cause ?? '').length < MAX_DISPLAY_DATA_SIZE;
 });
 
 const parameters = computed<INodeProperties[]>(() => {
@@ -181,28 +178,31 @@ function addItemIndexSuffix(message: string): string {
 }
 
 function getErrorMessage(): string {
-	const baseErrorMessage = '';
 	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) {
 		message = i18n.baseText('nodeErrorView.errorSubNode', {
 			interpolate: { node: props.error.node.name },
 		});
 	} else if (
-		props.error.message === props.error.description ||
-		!props.error.context?.messageTemplate
+		isNonEmptyString(props.error.message) &&
+		(props.error.message === props.error.description || !props.error.context?.messageTemplate)
 	) {
-		message = baseErrorMessage + props.error.message;
-	} else {
-		const parameterName = parameterDisplayName(props.error.context.parameter as string);
-
-		message =
-			baseErrorMessage +
-			(props.error.context.messageTemplate as string).replace(/%%PARAMETER%%/g, parameterName);
+		message = props.error.message;
+	} else if (
+		isNonEmptyString(props.error.context?.messageTemplate) &&
+		isNonEmptyString(props.error.context?.parameter)
+	) {
+		const parameterName = parameterDisplayName(props.error.context.parameter);
+		message = props.error.context.messageTemplate.replace(/%%PARAMETER%%/g, parameterName);
+	} else if (Array.isArray(props.error.messages) && props.error.messages.length > 0) {
+		message = props.error.messages[0];
 	}
 
 	return addItemIndexSuffix(message);
@@ -364,13 +364,14 @@ function copySuccess() {
 <template>
 	<div class="node-error-view">
 		<div class="node-error-view__header">
-			<div class="node-error-view__header-message">
+			<div class="node-error-view__header-message" data-test-id="node-error-message">
 				<div>
 					{{ getErrorMessage() }}
 				</div>
 			</div>
 			<div
 				v-if="error.description || error.context?.descriptionKey"
+				data-test-id="node-error-description"
 				class="node-error-view__header-description"
 				v-html="getErrorDescription()"
 			></div>
diff --git a/packages/editor-ui/src/components/Error/__tests__/NodeErrorView.test.ts b/packages/editor-ui/src/components/Error/__tests__/NodeErrorView.test.ts
new file mode 100644
index 0000000000..9d33bc3414
--- /dev/null
+++ b/packages/editor-ui/src/components/Error/__tests__/NodeErrorView.test.ts
@@ -0,0 +1,66 @@
+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';
+
+const DEFAULT_SETUP = {
+	pinia: createTestingPinia({
+		initialState: {
+			[STORES.SETTINGS]: SETTINGS_STORE_DEFAULT_STATE,
+		},
+	}),
+};
+
+const renderComponent = createComponentRenderer(NodeErrorView, DEFAULT_SETUP);
+
+describe('NodeErrorView.vue', () => {
+	let mockNode: INode;
+	afterEach(() => {
+		mockNode = {
+			parameters: {
+				mode: 'runOnceForAllItems',
+				language: 'javaScript',
+				jsCode: 'cons error = 9;',
+				notice: '',
+			},
+			id: 'd1ce5dc9-f9ae-4ac6-84e5-0696ba175dd9',
+			name: 'Code',
+			type: 'n8n-nodes-base.code',
+			typeVersion: 2,
+			position: [940, 240],
+		};
+		vi.clearAllMocks();
+	});
+
+	it('renders an Error with a messages array', async () => {
+		const { getByTestId } = renderComponent({
+			props: {
+				error: {
+					node: mockNode,
+					messages: ['Unexpected identifier [line 1]'],
+				},
+			},
+		});
+
+		const errorMessage = getByTestId('node-error-message');
+
+		expect(errorMessage).toHaveTextContent('Unexpected identifier [line 1]');
+	});
+
+	it('renders an Error with a message string', async () => {
+		const { getByTestId } = renderComponent({
+			props: {
+				error: {
+					node: mockNode,
+					message: 'Unexpected identifier [line 1]',
+				},
+			},
+		});
+
+		const errorMessage = getByTestId('node-error-message');
+
+		expect(errorMessage).toHaveTextContent('Unexpected identifier [line 1]');
+	});
+});
diff --git a/packages/editor-ui/src/components/RunData.vue b/packages/editor-ui/src/components/RunData.vue
index 48afcd4b93..6b28cd1a7c 100644
--- a/packages/editor-ui/src/components/RunData.vue
+++ b/packages/editor-ui/src/components/RunData.vue
@@ -837,13 +837,10 @@ export default defineComponent({
 			return Boolean(this.workflowsStore.subWorkflowExecutionError);
 		},
 		workflowRunErrorAsNodeError(): NodeError {
-			return {
-				node: this.node,
-				messages: [this.workflowRunData?.[this.node?.name]?.[this.runIndex]?.error?.message ?? ''],
-			} as NodeError;
+			return this.workflowRunData?.[this.node?.name]?.[this.runIndex]?.error as NodeError;
 		},
 		hasRunError(): boolean {
-			return Boolean(this.node && this.workflowRunData?.[this.node.name]?.[this.runIndex]?.error);
+			return Boolean(this.node && this.workflowRunErrorAsNodeError);
 		},
 		executionHints(): NodeHint[] {
 			if (this.hasNodeRun) {