Skip to content

Commit

Permalink
fix: Overhaul expression error messages related to paired item (#8765)
Browse files Browse the repository at this point in the history
  • Loading branch information
elsmr authored Mar 21, 2024
1 parent 079a114 commit 45461c8
Show file tree
Hide file tree
Showing 6 changed files with 135 additions and 102 deletions.
31 changes: 30 additions & 1 deletion packages/editor-ui/src/components/Error/NodeErrorView.vue
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
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
Original file line number Diff line number Diff line change
Expand Up @@ -1033,6 +1033,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
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,
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

0 comments on commit 45461c8

Please sign in to comment.