Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(editor): Fix node execution errors showing undefined #9487

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 20 additions & 7 deletions cypress/e2e/5-ndv.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
162 changes: 162 additions & 0 deletions cypress/fixtures/Test_workflow_ndv_run_error.json
Original file line number Diff line number Diff line change
@@ -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": []
}
2 changes: 2 additions & 0 deletions cypress/pages/ndv.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
39 changes: 20 additions & 19 deletions packages/editor-ui/src/components/Error/NodeErrorView.vue
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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();
Expand All @@ -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[]>(() => {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
@@ -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]');
});
});
7 changes: 2 additions & 5 deletions packages/editor-ui/src/components/RunData.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Loading