Skip to content

Commit

Permalink
fix(core): Improve error message when resolving itemMatching with pin…
Browse files Browse the repository at this point in the history
…ned data (no-changelog) (#12641)
  • Loading branch information
tomi authored Jan 17, 2025
1 parent 05858c2 commit 395f2ad
Show file tree
Hide file tree
Showing 5 changed files with 406 additions and 14 deletions.
5 changes: 4 additions & 1 deletion cypress/e2e/5-ndv.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,10 @@ describe('NDV', () => {
ndv.actions.execute();
ndv.getters
.nodeRunErrorMessage()
.should('have.text', 'Info for expression missing from previous node');
.should(
'have.text',
"Using the item method doesn't work with pinned data in this scenario. Please unpin 'Break pairedItem chain' and try again.",
);
ndv.getters
.nodeRunErrorDescription()
.should(
Expand Down
28 changes: 16 additions & 12 deletions packages/workflow/src/WorkflowDataProxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -703,17 +703,19 @@ export class WorkflowDataProxy {
});
};

const createMissingPairedItemError = (nodeCause: string) => {
return createExpressionError("Can't get data for expression", {
messageTemplate: 'Info for expression missing from previous node',
const createMissingPairedItemError = (
nodeCause: string,
usedMethodName: 'itemMatching' | 'pairedItem' | 'item' | '$getPairedItem' = 'pairedItem',
) => {
const message = `Using the ${usedMethodName} method doesn't work with pinned data in this scenario. Please unpin '${nodeCause}' and try again.`;
return new ExpressionError(message, {
runIndex: that.runIndex,
itemIndex: that.itemIndex,
functionality: 'pairedItem',
functionOverrides: {
message: "Can't get data",
},
nodeCause,
descriptionKey: isScriptingNode(nodeCause, that.workflow)
? 'pairedItemNoInfoCodeNode'
: 'pairedItemNoInfo',
nodeCause,
causeDetailed: `Missing pairedItem data (node '${nodeCause}' probably didn't supply it)`,
type: 'paired_item_no_info',
});
Expand All @@ -737,6 +739,7 @@ export class WorkflowDataProxy {
destinationNodeName: string,
incomingSourceData: ISourceData | null,
pairedItem: IPairedItemData,
usedMethodName: 'pairedItem' | 'itemMatching' | 'item' | '$getPairedItem' = '$getPairedItem',
): INodeExecutionData | null => {
let taskData: ITaskData | undefined;

Expand Down Expand Up @@ -790,7 +793,7 @@ export class WorkflowDataProxy {
const itemPreviousNode: INodeExecutionData = previousNodeOutputData[pairedItem.item];

if (itemPreviousNode.pairedItem === undefined) {
throw createMissingPairedItemError(sourceData.previousNode);
throw createMissingPairedItemError(sourceData.previousNode, usedMethodName);
}

if (Array.isArray(itemPreviousNode.pairedItem)) {
Expand All @@ -806,7 +809,7 @@ export class WorkflowDataProxy {
throw new ApplicationError('Not found');
}

return getPairedItem(destinationNodeName, source[itemInput], item);
return getPairedItem(destinationNodeName, source[itemInput], item, usedMethodName);
} catch (error) {
// Means pairedItem could not be found
return null;
Expand Down Expand Up @@ -858,6 +861,7 @@ export class WorkflowDataProxy {
// A trigger node got reached, so looks like that that item can not be resolved
throw createNoConnectionError(destinationNodeName);
}

throw createExpressionError('Can’t get data for expression', {
messageTemplate: 'Can’t get data for expression under ‘%%PARAMETER%%’ field',
functionality: 'pairedItem',
Expand Down Expand Up @@ -1039,7 +1043,7 @@ export class WorkflowDataProxy {
);
}

if (['pairedItem', 'itemMatching', 'item'].includes(property as string)) {
if (property === 'pairedItem' || property === 'itemMatching' || property === 'item') {
// Before resolving the pairedItem make sure that the requested node comes in the
// graph before the current one
const activeNode = that.workflow.getNode(that.activeNodeName);
Expand Down Expand Up @@ -1101,7 +1105,7 @@ export class WorkflowDataProxy {
const pairedItem = input.pairedItem as IPairedItemData;

if (pairedItem === undefined) {
throw createMissingPairedItemError(that.activeNodeName);
throw createMissingPairedItemError(that.activeNodeName, property);
}

if (!that.executeData?.source) {
Expand All @@ -1122,7 +1126,7 @@ export class WorkflowDataProxy {
that.executeData.source.main[pairedItem.input || 0] ??
that.executeData.source.main[0];

return getPairedItem(nodeName, sourceData, pairedItem);
return getPairedItem(nodeName, sourceData, pairedItem, property);
};

if (property === 'item') {
Expand Down
65 changes: 64 additions & 1 deletion packages/workflow/test/WorkflowDataProxy.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { ensureError } from '@/errors/ensure-error';
import { ExpressionError } from '@/errors/expression.error';
import {
NodeConnectionType,
Expand Down Expand Up @@ -334,7 +335,9 @@ describe('WorkflowDataProxy', () => {
} catch (error) {
expect(error).toBeInstanceOf(ExpressionError);
const exprError = error as ExpressionError;
expect(exprError.message).toEqual("Can't get data for expression");
expect(exprError.message).toEqual(
"Using the item method doesn't work with pinned data in this scenario. Please unpin 'Break pairedItem chain' and try again.",
);
expect(exprError.context.type).toEqual('paired_item_no_info');
done();
}
Expand Down Expand Up @@ -412,6 +415,66 @@ describe('WorkflowDataProxy', () => {
});
});

describe('Pinned data with paired items', () => {
const fixture = loadFixture('pindata_paireditem');
const proxy = getProxyFromFixture(fixture.workflow, fixture.run, 'Set', 'manual', {
runIndex: 0,
throwOnMissingExecutionData: false,
});

test.each([{ methodName: 'itemMatching' }, { methodName: 'pairedItem' }])(
'$methodName should throw when it cannot find a paired item',
async ({ methodName }) => {
try {
proxy.$('DebugHelper')[methodName](0);
fail('should throw');
} catch (e) {
const error = ensureError(e);
expect(error.message).toEqual(
`Using the ${methodName} method doesn't work with pinned data in this scenario. Please unpin 'Edit Fields' and try again.`,
);

expect(error).toMatchObject({
functionality: 'pairedItem',
context: {
runIndex: 0,
itemIndex: 0,
type: 'paired_item_no_info',
descriptionKey: 'pairedItemNoInfo',
nodeCause: 'Edit Fields',
causeDetailed:
"Missing pairedItem data (node 'Edit Fields' probably didn't supply it)",
},
});
}
},
);

test('item should throw when it cannot find a paired item', async () => {
try {
proxy.$('DebugHelper').item;
fail('should throw');
} catch (e) {
const error = ensureError(e);
expect(error.message).toEqual(
"Using the item method doesn't work with pinned data in this scenario. Please unpin 'Edit Fields' and try again.",
);

expect(error).toMatchObject({
functionality: 'pairedItem',
context: {
runIndex: 0,
itemIndex: 0,
type: 'paired_item_no_info',
descriptionKey: 'pairedItemNoInfo',
nodeCause: 'Edit Fields',
causeDetailed: "Missing pairedItem data (node 'Edit Fields' probably didn't supply it)",
},
});
}
});
});

describe('Partial data', () => {
const fixture = loadFixture('partial_data');

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,216 @@
{
"data": {
"startData": {},
"resultData": {
"runData": {
"When clicking ‘Test workflow’": [
{
"hints": [],
"startTime": 1737031584297,
"executionTime": 1,
"source": [],
"executionStatus": "success",
"data": { "main": [[{ "json": {}, "pairedItem": { "item": 0 } }]] }
}
],
"DebugHelper": [
{
"hints": [],
"startTime": 1737031584299,
"executionTime": 1,
"source": [{ "previousNode": "When clicking ‘Test workflow’" }],
"executionStatus": "success",
"data": {
"main": [
[
{
"json": {
"uid": "3c54ac5d-5c75-409d-9975-76ee151e5fc9",
"email": "[email protected]",
"firstname": "Betty",
"lastname": "Wolf",
"password": "c~837Nv"
},
"pairedItem": { "item": 0 }
},
{
"json": {
"uid": "5b0d9bd7-2ecf-47fb-b484-3eb1e76fa901",
"email": "[email protected]",
"firstname": "Shannon",
"lastname": "Champlin",
"password": "48FF,6dnx"
},
"pairedItem": { "item": 0 }
},
{
"json": {
"uid": "76437ebe-d406-447a-ab89-3b10f5183480",
"email": "[email protected]",
"firstname": "Alma",
"lastname": "Conn",
"password": "6$G2R3nT"
},
"pairedItem": { "item": 0 }
}
]
]
}
}
],
"Edit Fields": [
{
"hints": [],
"startTime": 1737031584301,
"executionTime": 0,
"source": [{ "previousNode": "DebugHelper" }],
"executionStatus": "success",
"data": {
"main": [
[
{
"json": {
"uid": "d42c6385-12f2-4486-92b5-eebd2e95d161",
"email": "[email protected]",
"firstname": "Laurie",
"lastname": "Krajcik",
"password": "k%Y2I9oq",
"test": "1"
}
},
{
"json": {
"uid": "53fc09df-5463-4f48-9fda-6500b1b77c82",
"email": "[email protected]",
"firstname": "Tracy",
"lastname": "Mraz",
"password": "t48s3-r",
"test": "1"
}
}
]
]
}
}
],
"Set": [
{
"hints": [],
"startTime": 1737031584301,
"executionTime": 0,
"source": [{ "previousNode": "Edit Fields" }],
"data": {
"main": [
[
{
"json": {
"uid": "d42c6385-12f2-4486-92b5-eebd2e95d161",
"email": "[email protected]",
"firstname": "Laurie",
"lastname": "Krajcik",
"password": "k%Y2I9oq",
"test": "1"
},
"pairedItem": {
"item": 0
}
},
{
"json": {
"uid": "53fc09df-5463-4f48-9fda-6500b1b77c82",
"email": "[email protected]",
"firstname": "Tracy",
"lastname": "Mraz",
"password": "t48s3-r",
"test": "1"
},
"pairedItem": {
"item": 1
}
}
]
]
}
}
]
},
"pinData": {
"Edit Fields": [
{
"json": {
"uid": "d42c6385-12f2-4486-92b5-eebd2e95d161",
"email": "[email protected]",
"firstname": "Laurie",
"lastname": "Krajcik",
"password": "k%Y2I9oq",
"test": "1"
}
},
{
"json": {
"uid": "53fc09df-5463-4f48-9fda-6500b1b77c82",
"email": "[email protected]",
"firstname": "Tracy",
"lastname": "Mraz",
"password": "t48s3-r",
"test": "1"
}
}
]
},
"lastNodeExecuted": "Set"
},
"executionData": {
"contextData": {},
"nodeExecutionStack": [
{
"node": {
"parameters": {
"mode": "runOnceForAllItems",
"language": "javaScript",
"jsCode": "for (let i = 0; i < $input.all().length; i++) { $(\"DebugHelper\").itemMatching(i) } return []",
"notice": ""
},
"type": "n8n-nodes-base.code",
"typeVersion": 2,
"position": [720, 0],
"id": "d4e8b6a2-cd73-452d-b5f0-986753f5dc4a",
"name": "Set"
},
"data": {
"main": [
[
{
"json": {
"uid": "d42c6385-12f2-4486-92b5-eebd2e95d161",
"email": "[email protected]",
"firstname": "Laurie",
"lastname": "Krajcik",
"password": "k%Y2I9oq",
"test": "1"
},
"pairedItem": { "item": 0 }
},
{
"json": {
"uid": "53fc09df-5463-4f48-9fda-6500b1b77c82",
"email": "[email protected]",
"firstname": "Tracy",
"lastname": "Mraz",
"password": "t48s3-r",
"test": "1"
},
"pairedItem": { "item": 1 }
}
]
]
},
"source": { "main": [{ "previousNode": "Edit Fields" }] }
}
],
"metadata": {},
"waitingExecution": {},
"waitingExecutionSource": {}
}
}
}
Loading

0 comments on commit 395f2ad

Please sign in to comment.