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.

Sign up for GitHub

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: Overhaul expression error messages related to paired item #8765

Merged
merged 8 commits into from
Mar 21, 2024
Merged

fix: Overhaul expression error messages related to paired item #8765

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
43e8776
Overhaul expression error messages
elsmr Feb 28, 2024
e818630
Show correct error when previous node is code node
elsmr Feb 29, 2024
c9cf83c
Update tests with new error messages
elsmr Feb 29, 2024
ee6749d
Merge branch 'master' into node-1133-paired-item-error-improvements
elsmr Mar 8, 2024
9d7c1d5
Add descriptionKey support to error view
elsmr Mar 15, 2024
9d2d98a
Merge branch 'master' into node-1133-paired-item-error-improvements
elsmr Mar 15, 2024
580d3a3
Migrate final error descriptions to i18n
elsmr Mar 15, 2024
77d109d
Fix type import
elsmr Mar 15, 2024
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
31 changes: 30 additions & 1 deletion packages/editor-ui/src/components/Error/NodeErrorView.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { sanitizeHtml } from '@/utils/htmlUtils';
import { useAIStore } from '@/stores/ai.store';
import { MAX_DISPLAY_DATA_SIZE } from '@/constants';
import VueMarkdown from 'vue-markdown-render';
import type { BaseTextKey } from '@/plugins/i18n';

const props = defineProps({
error: {
Expand Down Expand Up @@ -188,6 +189,21 @@ function getErrorDescription(): string {
}),
);
}

if (props.error.context?.descriptionKey) {
const interpolate = {
nodeCause: props.error.context.nodeCause as string,
runIndex: (props.error.context.runIndex as string) ?? '0',
itemIndex: (props.error.context.itemIndex as string) ?? '0',
};
return sanitizeHtml(
i18n.baseText(
`nodeErrorView.description.${props.error.context.descriptionKey as string}` as BaseTextKey,
{ interpolate },
),
);
}

if (!props.error.context?.descriptionTemplate) {
return sanitizeHtml(props.error.description ?? '');
}
Expand Down Expand Up @@ -397,7 +413,7 @@ function copySuccess() {
</N8nButton>
</div>
<div
v-if="error.description"
v-if="error.description || error.context?.descriptionKey"
class="node-error-view__header-description"
v-html="getErrorDescription()"
></div>
Expand Down Expand Up @@ -661,6 +677,19 @@ function copySuccess() {
&__header-description {
padding: 0 var(--spacing-s) var(--spacing-3xs) var(--spacing-s);
font-size: var(--font-size-xs);

ul {
padding: var(--spacing-s) 0;
padding-left: var(--spacing-l);
}

code {
font-size: var(--font-size-xs);
color: var(--color-text-base);
background: var(--color-background-base);
padding: var(--spacing-5xs);
border-radius: var(--border-radius-base);
}
}

&__debugging {
Expand Down
2 changes: 2 additions & 0 deletions packages/editor-ui/src/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -664,6 +664,8 @@ export const ALLOWED_HTML_TAGS = [
'a',
'br',
'i',
'ul',
'li',
'em',
'small',
'details',
Expand Down
11 changes: 11 additions & 0 deletions packages/editor-ui/src/plugins/i18n/locales/en.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1026,6 +1026,17 @@
"nodeErrorView.theErrorCauseIsTooLargeToBeDisplayed": "The error cause is too large to be displayed",
"nodeErrorView.time": "Time",
"nodeErrorView.inputPanel.previousNodeError.title": "Error running node '{nodeName}'",
"nodeErrorView.description.pairedItemInvalidInfo": "An expression here won't work because it uses <code>.item</code> and n8n can't figure out the <a href=”https://docs.n8n.io/data/data-mapping/data-item-linking/item-linking-errors/”>matching item</a>. This is because the node <strong>'{nodeCause}'</strong> returned incorrect matching information (for item {itemIndex} of run {runIndex}). <br/><br/>Try using <code>.first()</code>, <code>.last()</code> or <code>.all()[index]</code> instead of <code>.item</code>.",
"nodeErrorView.description.pairedItemNoInfo": "An expression here won't work because it uses <code>.item</code> and n8n can't figure out the <a href=”https://docs.n8n.io/data/data-mapping/data-item-linking/item-linking-errors/”>matching item</a>. The node <strong>'{nodeCause}'</strong> didn't return enough information.",
"nodeErrorView.description.pairedItemNoInfoCodeNode": "An expression here won't work because it uses <code>.item</code> and n8n can't figure out the <a href=\"https://docs.n8n.io/data/data-mapping/data-item-linking/item-linking-errors/\">matching item</a>. You can either: <ul><li>Add the <a href=\"https://docs.n8n.io/data/data-mapping/data-item-linking/item-linking-code-node/\">missing information</a> to the node <strong>'{nodeCause}'</strong></li><li>Or use <code>.first()</code>, <code>.last()</code> or <code>.all()[index]</code> instead of <code>.item</code></li></ul>",
"nodeErrorView.description.pairedItemNoConnection": "There is no connection back to the node <strong>'{nodeCause}'</strong>, but it's used in an expression here.<br/><br/>Please wire up the node (there can be other nodes in between).",
"nodeErrorView.description.pairedItemNoConnectionCodeNode": "There is no connection back to the node <strong>'{nodeCause}'</strong>, but it's used in code here.<br/><br/>Please wire up the node (there can be other nodes in between).",
"nodeErrorView.description.noNodeExecutionData": "An expression references the node <strong>'{nodeCause}'</strong>, but it hasn't been executed yet. Either change the expression, or re-wire your workflow to make sure that node executes first.",
"nodeErrorView.description.nodeNotFound": "The node <strong>'{nodeCause}'</strong> doesn't exist, but it's used in an expression here.",
"nodeErrorView.description.noInputConnection": "This node has no input data. Please make sure this node is connected to another node.",
"nodeErrorView.description.pairedItemMultipleMatches": "An expression here won't work because it uses <code>.item</code> and n8n can't figure out the <a href=”https://docs.n8n.io/data/data-mapping/data-item-linking/item-linking-errors/”>matching item</a>. (There are multiple possible matches) <br/><br/>Try using <code>.first()</code>, <code>.last()</code> or <code>.all()[index]</code> instead of <code>.item</code> or <a href=”https://docs.n8n.io/data/data-mapping/data-item-linking/item-linking-code-node/”>reference a different node</a>.",
"nodeErrorView.description.pairedItemMultipleMatchesCodeNode": "The code here won't work because it uses <code>.item</code> and n8n can't figure out the <a href=”https://docs.n8n.io/data/data-mapping/data-item-linking/item-linking-errors/”>matching item</a>. (There are multiple possible matches) <br/><br/>Try using <code>.first()</code>, <code>.last()</code> or <code>.all()[index]</code> instead of <code>.item</code> or <a href=”https://docs.n8n.io/data/data-mapping/data-item-linking/item-linking-code-node/”>reference a different node</a>.",
"nodeErrorView.description.pairedItemPinned": "The <a href=”https://docs.n8n.io/data/data-mapping/data-item-linking/item-linking-errors/”>item-matching</a> data in that node may be stale. It is needed by an expression in this node that uses <code>.item</code>.",
"nodeErrorView.debugError.button": "Ask AI ✨",
"nodeErrorView.debugError.loading": "Asking AI.. ✨",
"nodeErrorView.debugError.feedback.reload": "Regenerate answer",
Expand Down
181 changes: 85 additions & 96 deletions packages/workflow/src/WorkflowDataProxy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,20 +271,23 @@ export class WorkflowDataProxy {
}

if (!that.workflow.getNode(nodeName)) {
throw new ExpressionError(`"${nodeName}" node doesn't exist`, {
throw new ExpressionError("Referenced node doesn't exist", {
runIndex: that.runIndex,
itemIndex: that.itemIndex,
nodeCause: nodeName,
elsmr marked this conversation as resolved.
Show resolved Hide resolved
descriptionKey: 'nodeNotFound',
});
}

if (
!that.runExecutionData.resultData.runData.hasOwnProperty(nodeName) &&
!that.workflow.getPinDataOfNode(nodeName)
) {
throw new ExpressionError(`no data, execute "${nodeName}" node first`, {
throw new ExpressionError('Referenced node is unexecuted', {
runIndex: that.runIndex,
itemIndex: that.itemIndex,
type: 'no_node_execution_data',
descriptionKey: 'noNodeExecutionData',
nodeCause: nodeName,
});
}
Expand Down Expand Up @@ -369,7 +372,12 @@ export class WorkflowDataProxy {
name = name.toString();

if (!node) {
throw new ExpressionError(`"${nodeName}" node doesn't exist`);
throw new ExpressionError("Referenced node doesn't exist", {
runIndex: that.runIndex,
itemIndex: that.itemIndex,
nodeCause: nodeName,
descriptionKey: 'nodeNotFound',
});
}

if (['binary', 'data', 'json'].includes(name)) {
Expand All @@ -380,8 +388,7 @@ export class WorkflowDataProxy {
throw new ExpressionError('No execution data available', {
messageTemplate:
'No execution data available to expression under ‘%%PARAMETER%%’',
description:
'This node has no input data. Please make sure this node is connected to another node.',
descriptionKey: 'noInputConnection',
nodeCause: nodeName,
runIndex: that.runIndex,
itemIndex: that.itemIndex,
Expand Down Expand Up @@ -589,9 +596,11 @@ export class WorkflowDataProxy {
const nodeName = name.toString();

if (that.workflow.getNode(nodeName) === null) {
throw new ExpressionError(`"${nodeName}" node doesn't exist`, {
throw new ExpressionError("Referenced node doesn't exist", {
runIndex: that.runIndex,
itemIndex: that.itemIndex,
nodeCause: nodeName,
descriptionKey: 'nodeNotFound',
});
}

Expand Down Expand Up @@ -666,10 +675,9 @@ export class WorkflowDataProxy {
if (!context) {
context = {};
}
message = `‘Node ${nodeName}‘ must be unpinned to execute`;
message = `Unpin '${nodeName}' to execute`;
context.messageTemplate = undefined;
context.description = `To fetch the data for the expression, you must unpin the node <strong>'${nodeName}'</strong> and execute the workflow again.`;
context.descriptionTemplate = `To fetch the data for the expression under '%%PARAMETER%%', you must unpin the node <strong>'${nodeName}'</strong> and execute the workflow again.`;
context.descriptionKey = 'pairedItemPinned';
}

if (context.moreInfoLink && (pinData || isScriptingNode(nodeName, that.workflow))) {
Expand All @@ -688,6 +696,48 @@ export class WorkflowDataProxy {
});
};

const createInvalidPairedItemError = ({ nodeName }: { nodeName: string }) => {
return createExpressionError("Can't get data for expression", {
messageTemplate: 'Expression info invalid',
functionality: 'pairedItem',
functionOverrides: {
message: "Can't get data",
},
nodeCause: nodeName,
descriptionKey: 'pairedItemInvalidInfo',
type: 'paired_item_invalid_info',
});
};

const createMissingPairedItemError = (nodeCause: string) => {
return createExpressionError("Can't get data for expression", {
messageTemplate: 'Info for expression missing from previous node',
functionality: 'pairedItem',
functionOverrides: {
message: "Can't get data",
},
nodeCause,
descriptionKey: isScriptingNode(nodeCause, that.workflow)
? 'pairedItemNoInfoCodeNode'
: 'pairedItemNoInfo',
causeDetailed: `Missing pairedItem data (node '${nodeCause}' probably didn't supply it)`,
type: 'paired_item_no_info',
});
};

const createNoConnectionError = (nodeCause: string) => {
return createExpressionError('Invalid expression', {
messageTemplate: 'No path back to referenced node',
functionality: 'pairedItem',
descriptionKey: isScriptingNode(nodeCause, that.workflow)
? 'pairedItemNoConnectionCodeNode'
: 'pairedItemNoConnection',
type: 'paired_item_no_connection',
moreInfoLink: true,
nodeCause,
});
};

const getPairedItem = (
destinationNodeName: string,
incomingSourceData: ISourceData | null,
Expand Down Expand Up @@ -736,42 +786,15 @@ export class WorkflowDataProxy {
const source = taskData?.source ?? [];

if (pairedItem.item >= previousNodeOutputData.length) {
throw createExpressionError('Can’t get data for expression', {
messageTemplate: 'Can’t get data for expression under ‘%%PARAMETER%%’ field',
functionality: 'pairedItem',
functionOverrides: {
message: 'Can’t get data',
},
nodeCause: nodeBeforeLast,
description: `In node ‘<strong>${nodeBeforeLast!}</strong>’, output item ${
currentPairedItem.item || 0
} ${
sourceData.previousNodeRun
? `of run ${(sourceData.previousNodeRun || 0).toString()} `
: ''
}points to an input item on node ‘<strong>${
sourceData.previousNode
}</strong>‘ that doesn’t exist.`,
type: 'paired_item_invalid_info',
moreInfoLink: true,
throw createInvalidPairedItemError({
nodeName: sourceData.previousNode,
});
}

const itemPreviousNode: INodeExecutionData = previousNodeOutputData[pairedItem.item];

if (itemPreviousNode.pairedItem === undefined) {
throw createExpressionError('Can’t get data for expression', {
messageTemplate: 'Can’t get data for expression under ‘%%PARAMETER%%’ field',
functionality: 'pairedItem',
functionOverrides: {
message: 'Can’t get data',
},
nodeCause: sourceData.previousNode,
description: `To fetch the data from other nodes that this expression needs, more information is needed from the node ‘<strong>${sourceData.previousNode}</strong>’`,
causeDetailed: `Missing pairedItem data (node ‘${sourceData.previousNode}’ probably didn’t supply it)`,
type: 'paired_item_no_info',
moreInfoLink: true,
});
throw createMissingPairedItemError(sourceData.previousNode);
}

if (Array.isArray(itemPreviousNode.pairedItem)) {
Expand Down Expand Up @@ -804,13 +827,17 @@ export class WorkflowDataProxy {
}

throw createExpressionError('Invalid expression', {
messageTemplate: 'Invalid expression under ‘%%PARAMETER%%’',
messageTemplate: `Multiple matching items for expression [item ${
currentPairedItem.item || 0
}]`,
functionality: 'pairedItem',
functionOverrides: {
description: `The code uses data in the node ‘<strong>${destinationNodeName}</strong>’ but there is more than one matching item in that node`,
message: 'Invalid code',
message: `Multiple matching items for code [item ${currentPairedItem.item || 0}]`,
},
description: `The expression uses data in the node ‘<strong>${destinationNodeName}</strong>’ but there is more than one matching item in that node`,
nodeCause: destinationNodeName,
descriptionKey: isScriptingNode(destinationNodeName, that.workflow)
? 'pairedItemMultipleMatchesCodeNode'
: 'pairedItemMultipleMatches',
type: 'paired_item_multiple_matches',
});
}
Expand All @@ -833,17 +860,7 @@ export class WorkflowDataProxy {
if (itemInput >= source.length) {
if (source.length === 0) {
// A trigger node got reached, so looks like that that item can not be resolved
throw createExpressionError('Invalid expression', {
messageTemplate: 'Invalid expression under ‘%%PARAMETER%%’',
functionality: 'pairedItem',
functionOverrides: {
description: `The code uses data in the node ‘<strong>${destinationNodeName}</strong>’ but there is no path back to it. Please check this node is connected to it (there can be other nodes in between).`,
message: 'Invalid code',
},
description: `The expression uses data in the node ‘<strong>${destinationNodeName}</strong>’ but there is no path back to it. Please check this node is connected to it (there can be other nodes in between).`,
type: 'paired_item_no_connection',
moreInfoLink: true,
});
throw createNoConnectionError(destinationNodeName);
}
throw createExpressionError('Can’t get data for expression', {
messageTemplate: 'Can’t get data for expression under ‘%%PARAMETER%%’ field',
Expand Down Expand Up @@ -905,24 +922,8 @@ export class WorkflowDataProxy {
}

if (pairedItem.item >= taskData.data!.main[previousNodeOutput]!.length) {
throw createExpressionError('Can’t get data for expression', {
messageTemplate: 'Can’t get data for expression under ‘%%PARAMETER%%’ field',
functionality: 'pairedItem',
functionOverrides: {
message: 'Can’t get data',
},
nodeCause: nodeBeforeLast,
description: `In node ‘<strong>${nodeBeforeLast!}</strong>’, output item ${
currentPairedItem.item || 0
} ${
sourceData.previousNodeRun
? `of run ${(sourceData.previousNodeRun || 0).toString()} `
: ''
}points to an input item on node ‘<strong>${
sourceData.previousNode
}</strong>‘ that doesn’t exist.`,
type: 'paired_item_invalid_info',
moreInfoLink: true,
throw createInvalidPairedItemError({
nodeName: sourceData.previousNode,
});
}

Expand All @@ -937,16 +938,24 @@ export class WorkflowDataProxy {

const referencedNode = that.workflow.getNode(nodeName);
if (referencedNode === null) {
throw createExpressionError(`"${nodeName}" node doesn't exist`);
throw createExpressionError("Referenced node doesn't exist", {
runIndex: that.runIndex,
itemIndex: that.itemIndex,
nodeCause: nodeName,
descriptionKey: 'nodeNotFound',
});
}

const ensureNodeExecutionData = () => {
if (
!that?.runExecutionData?.resultData?.runData.hasOwnProperty(nodeName) &&
!that.workflow.getPinDataOfNode(nodeName)
) {
throw createExpressionError(`no data, execute "${nodeName}" node first`, {
throw createExpressionError('Referenced node is unexecuted', {
runIndex: that.runIndex,
itemIndex: that.itemIndex,
type: 'no_node_execution_data',
descriptionKey: 'noNodeExecutionData',
nodeCause: nodeName,
});
}
Expand Down Expand Up @@ -989,17 +998,7 @@ export class WorkflowDataProxy {
}
const parentNodes = that.workflow.getParentNodes(contextNode);
if (!parentNodes.includes(nodeName)) {
throw createExpressionError('Invalid expression', {
messageTemplate: 'Invalid expression under ‘%%PARAMETER%%’',
functionality: 'pairedItem',
functionOverrides: {
description: `The code uses data in the node <strong>‘${nodeName}’</strong> but there is no path back to it. Please check this node is connected to it (there can be other nodes in between).`,
message: `No path back to node ‘${nodeName}’`,
},
description: `The expression uses data in the node <strong>‘${nodeName}’</strong> but there is no path back to it. Please check this node is connected to it (there can be other nodes in between).`,
nodeCause: nodeName,
type: 'paired_item_no_connection',
});
throw createNoConnectionError(nodeName);
}

ensureNodeExecutionData();
Expand Down Expand Up @@ -1037,17 +1036,7 @@ export class WorkflowDataProxy {
const pairedItem = input.pairedItem as IPairedItemData;

if (pairedItem === undefined) {
throw createExpressionError('Can’t get data for expression', {
messageTemplate: 'Can’t get data for expression under ‘%%PARAMETER%%’ field',
functionality: 'pairedItem',
functionOverrides: {
description: `To fetch the data from other nodes that this code needs, more information is needed from the node ‘<strong>${that.activeNodeName}</strong>‘`,
message: 'Can’t get data',
},
description: `To fetch the data from other nodes that this expression needs, more information is needed from the node ‘<strong>${that.activeNodeName}</strong>‘`,
causeDetailed: `Missing pairedItem data (node ‘${that.activeNodeName}‘ probably didn’t supply it)`,
itemIndex,
});
throw createMissingPairedItemError(that.activeNodeName);
}

if (!that.executeData?.source) {
Expand Down
Loading
Loading