Skip to content

Commit 62ee796

Browse files
elsmrelsmr
authored
fix(editor): Fix node execution errors showing undefined (#9487)
1 parent a217866 commit 62ee796

File tree

6 files changed

+272
-31
lines changed

6 files changed

+272
-31
lines changed

‎cypress/e2e/5-ndv.cy.ts

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -105,13 +105,26 @@ describe('NDV', () => {
105105
});
106106

107107
it('should show all validation errors when opening pasted node', () => {
108-
cy.fixture('Test_workflow_ndv_errors.json').then((data) => {
109-
cy.get('body').paste(JSON.stringify(data));
110-
workflowPage.getters.canvasNodes().should('have.have.length', 1);
111-
workflowPage.actions.openNode('Airtable');
112-
cy.get('.has-issues').should('have.length', 3);
113-
cy.get('[class*=hasIssues]').should('have.length', 1);
114-
});
108+
cy.createFixtureWorkflow('Test_workflow_ndv_errors.json', 'Validation errors');
109+
workflowPage.getters.canvasNodes().should('have.have.length', 1);
110+
workflowPage.actions.openNode('Airtable');
111+
cy.get('.has-issues').should('have.length', 3);
112+
cy.get('[class*=hasIssues]').should('have.length', 1);
113+
});
114+
115+
it('should render run errors correctly', () => {
116+
cy.createFixtureWorkflow('Test_workflow_ndv_run_error.json', 'Run error');
117+
workflowPage.actions.openNode('Error');
118+
ndv.actions.execute();
119+
ndv.getters
120+
.nodeRunErrorMessage()
121+
.should('have.text', 'Info for expression missing from previous node');
122+
ndv.getters
123+
.nodeRunErrorDescription()
124+
.should(
125+
'contains.text',
126+
"An expression here won't work because it uses .item and n8n can't figure out the matching item.",
127+
);
115128
});
116129

117130
it('should save workflow using keyboard shortcut from NDV', () => {

‎cypress/fixtures/Test_workflow_ndv_run_error.json

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
{
2+
"name": "My workflow 52",
3+
"nodes": [
4+
{
5+
"parameters": {
6+
"jsCode": "\nreturn [\n {\n \"field\": \"the same\"\n }\n];"
7+
},
8+
"id": "38c14c4a-7af1-4b04-be76-f8e474c95569",
9+
"name": "Break pairedItem chain",
10+
"type": "n8n-nodes-base.code",
11+
"typeVersion": 2,
12+
"position": [
13+
240,
14+
1020
15+
]
16+
},
17+
{
18+
"parameters": {
19+
"options": {}
20+
},
21+
"id": "78c4964a-c4e8-47e5-81f3-89ba778feb8b",
22+
"name": "Edit Fields",
23+
"type": "n8n-nodes-base.set",
24+
"typeVersion": 3.2,
25+
"position": [
26+
40,
27+
1020
28+
]
29+
},
30+
{
31+
"parameters": {},
32+
"id": "4f4c6527-d565-448a-96bd-8f5414caf8cc",
33+
"name": "When clicking \"Test workflow\"",
34+
"type": "n8n-nodes-base.manualTrigger",
35+
"typeVersion": 1,
36+
"position": [
37+
-180,
38+
1020
39+
]
40+
},
41+
{
42+
"parameters": {
43+
"fields": {
44+
"values": [
45+
{
46+
"stringValue": "={{ $('Edit Fields').item.json.name }}"
47+
}
48+
]
49+
},
50+
"options": {}
51+
},
52+
"id": "44f4e5da-bfe9-4dc3-8d1f-f38e9f364754",
53+
"name": "Error",
54+
"type": "n8n-nodes-base.set",
55+
"typeVersion": 3.2,
56+
"position": [
57+
460,
58+
1020
59+
]
60+
}
61+
],
62+
"pinData": {
63+
"Edit Fields": [
64+
{
65+
"json": {
66+
"id": "23423532",
67+
"name": "Jay Gatsby",
68+
"email": "[email protected]",
69+
"notes": "Keeps asking about a green light??",
70+
"country": "US",
71+
"created": "1925-04-10"
72+
}
73+
},
74+
{
75+
"json": {
76+
"id": "23423533",
77+
"name": "José Arcadio Buendía",
78+
"email": "[email protected]",
79+
"notes": "Lots of people named after him. Very confusing",
80+
"country": "CO",
81+
"created": "1967-05-05"
82+
}
83+
},
84+
{
85+
"json": {
86+
"id": "23423534",
87+
"name": "Max Sendak",
88+
"email": "[email protected]",
89+
"notes": "Keeps rolling his terrible eyes",
90+
"country": "US",
91+
"created": "1963-04-09"
92+
}
93+
},
94+
{
95+
"json": {
96+
"id": "23423535",
97+
"name": "Zaphod Beeblebrox",
98+
"email": "[email protected]",
99+
"notes": "Felt like I was talking to more than one person",
100+
"country": null,
101+
"created": "1979-10-12"
102+
}
103+
},
104+
{
105+
"json": {
106+
"id": "23423536",
107+
"name": "Edmund Pevensie",
108+
"email": "[email protected]",
109+
"notes": "Passionate sailor",
110+
"country": "UK",
111+
"created": "1950-10-16"
112+
}
113+
}
114+
]
115+
},
116+
"connections": {
117+
"Break pairedItem chain": {
118+
"main": [
119+
[
120+
{
121+
"node": "Error",
122+
"type": "main",
123+
"index": 0
124+
}
125+
]
126+
]
127+
},
128+
"Edit Fields": {
129+
"main": [
130+
[
131+
{
132+
"node": "Break pairedItem chain",
133+
"type": "main",
134+
"index": 0
135+
}
136+
]
137+
]
138+
},
139+
"When clicking \"Test workflow\"": {
140+
"main": [
141+
[
142+
{
143+
"node": "Edit Fields",
144+
"type": "main",
145+
"index": 0
146+
}
147+
]
148+
]
149+
}
150+
},
151+
"active": false,
152+
"settings": {
153+
"executionOrder": "v1"
154+
},
155+
"versionId": "ca53267f-4eb4-481d-9e09-ecb97f6b09e2",
156+
"meta": {
157+
"templateCredsSetupCompleted": true,
158+
"instanceId": "27cc9b56542ad45b38725555722c50a1c3fee1670bbb67980558314ee08517c4"
159+
},
160+
"id": "6fr8GiRyMlZCiDQW",
161+
"tags": []
162+
}

‎cypress/pages/ndv.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,8 @@ export class NDV extends BasePage {
124124
codeEditorFullscreen: () => this.getters.codeEditorDialog().find('.cm-content'),
125125
nodeRunSuccessIndicator: () => cy.getByTestId('node-run-info-success'),
126126
nodeRunErrorIndicator: () => cy.getByTestId('node-run-info-danger'),
127+
nodeRunErrorMessage: () => cy.getByTestId('node-error-message'),
128+
nodeRunErrorDescription: () => cy.getByTestId('node-error-description'),
127129
};
128130

129131
actions = {

‎packages/editor-ui/src/components/Error/NodeErrorView.vue

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
<script lang="ts" setup>
22
import { useI18n } from '@/composables/useI18n';
3-
import type { PropType } from 'vue';
43
import { computed } from 'vue';
54
import { useClipboard } from '@/composables/useClipboard';
65
import { useToast } from '@/composables/useToast';
@@ -20,13 +19,11 @@ import { sanitizeHtml } from '@/utils/htmlUtils';
2019
import { MAX_DISPLAY_DATA_SIZE } from '@/constants';
2120
import type { BaseTextKey } from '@/plugins/i18n';
2221
23-
const props = defineProps({
24-
error: {
25-
type: Object as PropType<NodeError | NodeApiError | NodeOperationError>,
26-
required: true,
27-
},
28-
});
22+
type Props = {
23+
error: NodeError | NodeApiError | NodeOperationError;
24+
};
2925
26+
const props = defineProps<Props>();
3027
const clipboard = useClipboard();
3128
const toast = useToast();
3229
const i18n = useI18n();
@@ -36,7 +33,7 @@ const ndvStore = useNDVStore();
3633
const rootStore = useRootStore();
3734
3835
const displayCause = computed(() => {
39-
return JSON.stringify(props.error.cause).length < MAX_DISPLAY_DATA_SIZE;
36+
return JSON.stringify(props.error.cause ?? '').length < MAX_DISPLAY_DATA_SIZE;
4037
});
4138
4239
const parameters = computed<INodeProperties[]>(() => {
@@ -181,28 +178,31 @@ function addItemIndexSuffix(message: string): string {
181178
}
182179
183180
function getErrorMessage(): string {
184-
const baseErrorMessage = '';
185181
let message = '';
186182
187183
const isSubNodeError =
188184
props.error.name === 'NodeOperationError' &&
189185
(props.error as NodeOperationError).functionality === 'configuration-node';
186+
const isNonEmptyString = (value?: unknown): value is string =>
187+
!!value && typeof value === 'string';
190188
191189
if (isSubNodeError) {
192190
message = i18n.baseText('nodeErrorView.errorSubNode', {
193191
interpolate: { node: props.error.node.name },
194192
});
195193
} else if (
196-
props.error.message === props.error.description ||
197-
!props.error.context?.messageTemplate
194+
isNonEmptyString(props.error.message) &&
195+
(props.error.message === props.error.description || !props.error.context?.messageTemplate)
198196
) {
199-
message = baseErrorMessage + props.error.message;
200-
} else {
201-
const parameterName = parameterDisplayName(props.error.context.parameter as string);
202-
203-
message =
204-
baseErrorMessage +
205-
(props.error.context.messageTemplate as string).replace(/%%PARAMETER%%/g, parameterName);
197+
message = props.error.message;
198+
} else if (
199+
isNonEmptyString(props.error.context?.messageTemplate) &&
200+
isNonEmptyString(props.error.context?.parameter)
201+
) {
202+
const parameterName = parameterDisplayName(props.error.context.parameter);
203+
message = props.error.context.messageTemplate.replace(/%%PARAMETER%%/g, parameterName);
204+
} else if (Array.isArray(props.error.messages) && props.error.messages.length > 0) {
205+
message = props.error.messages[0];
206206
}
207207
208208
return addItemIndexSuffix(message);
@@ -364,13 +364,14 @@ function copySuccess() {
364364
<template>
365365
<div class="node-error-view">
366366
<div class="node-error-view__header">
367-
<div class="node-error-view__header-message">
367+
<div class="node-error-view__header-message" data-test-id="node-error-message">
368368
<div>
369369
{{ getErrorMessage() }}
370370
</div>
371371
</div>
372372
<div
373373
v-if="error.description || error.context?.descriptionKey"
374+
data-test-id="node-error-description"
374375
class="node-error-view__header-description"
375376
v-html="getErrorDescription()"
376377
></div>

‎packages/editor-ui/src/components/Error/__tests__/NodeErrorView.test.ts

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
import { createComponentRenderer } from '@/__tests__/render';
2+
import { SETTINGS_STORE_DEFAULT_STATE } from '@/__tests__/utils';
3+
import NodeErrorView from '@/components/Error/NodeErrorView.vue';
4+
import { STORES } from '@/constants';
5+
import { createTestingPinia } from '@pinia/testing';
6+
import { type INode } from 'n8n-workflow';
7+
8+
const DEFAULT_SETUP = {
9+
pinia: createTestingPinia({
10+
initialState: {
11+
[STORES.SETTINGS]: SETTINGS_STORE_DEFAULT_STATE,
12+
},
13+
}),
14+
};
15+
16+
const renderComponent = createComponentRenderer(NodeErrorView, DEFAULT_SETUP);
17+
18+
describe('NodeErrorView.vue', () => {
19+
let mockNode: INode;
20+
afterEach(() => {
21+
mockNode = {
22+
parameters: {
23+
mode: 'runOnceForAllItems',
24+
language: 'javaScript',
25+
jsCode: 'cons error = 9;',
26+
notice: '',
27+
},
28+
id: 'd1ce5dc9-f9ae-4ac6-84e5-0696ba175dd9',
29+
name: 'Code',
30+
type: 'n8n-nodes-base.code',
31+
typeVersion: 2,
32+
position: [940, 240],
33+
};
34+
vi.clearAllMocks();
35+
});
36+
37+
it('renders an Error with a messages array', async () => {
38+
const { getByTestId } = renderComponent({
39+
props: {
40+
error: {
41+
node: mockNode,
42+
messages: ['Unexpected identifier [line 1]'],
43+
},
44+
},
45+
});
46+
47+
const errorMessage = getByTestId('node-error-message');
48+
49+
expect(errorMessage).toHaveTextContent('Unexpected identifier [line 1]');
50+
});
51+
52+
it('renders an Error with a message string', async () => {
53+
const { getByTestId } = renderComponent({
54+
props: {
55+
error: {
56+
node: mockNode,
57+
message: 'Unexpected identifier [line 1]',
58+
},
59+
},
60+
});
61+
62+
const errorMessage = getByTestId('node-error-message');
63+
64+
expect(errorMessage).toHaveTextContent('Unexpected identifier [line 1]');
65+
});
66+
});

‎packages/editor-ui/src/components/RunData.vue

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -837,13 +837,10 @@ export default defineComponent({
837837
return Boolean(this.workflowsStore.subWorkflowExecutionError);
838838
},
839839
workflowRunErrorAsNodeError(): NodeError {
840-
return {
841-
node: this.node,
842-
messages: [this.workflowRunData?.[this.node?.name]?.[this.runIndex]?.error?.message ?? ''],
843-
} as NodeError;
840+
return this.workflowRunData?.[this.node?.name]?.[this.runIndex]?.error as NodeError;
844841
},
845842
hasRunError(): boolean {
846-
return Boolean(this.node && this.workflowRunData?.[this.node.name]?.[this.runIndex]?.error);
843+
return Boolean(this.node && this.workflowRunErrorAsNodeError);
847844
},
848845
executionHints(): NodeHint[] {
849846
if (this.hasNodeRun) {

0 commit comments

Comments
 (0)