refactor(core): Consolidate task result validation in Code node (#18928)

packages/nodes-base/nodes/Code/JsTaskRunnerSandbox.ts

@@ -6,8 +6,14 @@ import {
 } from 'n8n-workflow';
 
 import { validateNoDisallowedMethodsInRunForEach } from './JsCodeValidator';
+import type { TextKeys } from './result-validation';
+import { validateRunCodeAllItems, validateRunCodeEachItem } from './result-validation';
 import { throwExecutionError } from './throw-execution-error';
 
+const JS_TEXT_KEYS: TextKeys = {
+	object: { singular: 'object', plural: 'objects' },
+};
+
 /**
  * JS Code execution sandbox that executes the JS code using task runner.
  */
@@ -34,9 +40,15 @@ export class JsTaskRunnerSandbox {
 			itemIndex,
 		);
 
-		return executionResult.ok
-			? executionResult.result
-			: throwExecutionError('error' in executionResult ? executionResult.error : {});
+		if (!executionResult.ok) {
+			throwExecutionError('error' in executionResult ? executionResult.error : {});
+		}
+
+		return validateRunCodeAllItems(
+			executionResult.result,
+			JS_TEXT_KEYS,
+			this.executeFunctions.helpers.normalizeItems.bind(this.executeFunctions.helpers),
+		);
 	}
 
 	async runCodeForEachItem(numInputItems: number): Promise<INodeExecutionData[]> {
@@ -66,6 +78,17 @@ export class JsTaskRunnerSandbox {
 				return throwExecutionError('error' in executionResult ? executionResult.error : {});
 			}
 
+			for (let i = 0; i < executionResult.result.length; i++) {
+				const actualItemIndex = chunk.startIdx + i;
+				const validatedItem = validateRunCodeEachItem(
+					executionResult.result[i],
+					actualItemIndex,
+					JS_TEXT_KEYS,
+					this.executeFunctions.helpers.normalizeItems.bind(this.executeFunctions.helpers),
+				);
+				executionResult.result[i] = validatedItem;
+			}
+
 			executionResults = executionResults.concat(executionResult.result);
 		}
 

packages/nodes-base/nodes/Code/Sandbox.ts

@@ -6,8 +6,7 @@ import type {
 	IWorkflowDataProxyData,
 } from 'n8n-workflow';
 
-import { isObject } from './utils';
-import { ValidationError } from './ValidationError';
+import { validateRunCodeAllItems, validateRunCodeEachItem } from './result-validation';
 
 interface SandboxTextKeys {
 	object: {
@@ -22,20 +21,6 @@ export interface SandboxContext extends IWorkflowDataProxyData {
 	helpers: IExecuteFunctions['helpers'];
 }
 
-export const REQUIRED_N8N_ITEM_KEYS = new Set([
-	'json',
-	'binary',
-	'pairedItem',
-	'error',
-
-	/**
-	 * The `index` key was added accidentally to Function, FunctionItem, Gong,
-	 * Execute Workflow, and ToolWorkflowV2, so we need to allow it temporarily.
-	 * Once we stop using it in all nodes, we can stop allowing the `index` key.
-	 */
-	'index',
-]);
-
 export function getSandboxContext(
 	this: IExecuteFunctions | ISupplyDataFunctions,
 	index: number,
@@ -75,147 +60,21 @@ export abstract class Sandbox extends EventEmitter {
 		executionResult: INodeExecutionData | undefined,
 		itemIndex: number,
 	): INodeExecutionData {
-		if (typeof executionResult !== 'object') {
-			throw new ValidationError({
-				message: `Code doesn't return ${this.getTextKey('object', { includeArticle: true })}`,
-				description: `Please return ${this.getTextKey('object', {
-					includeArticle: true,
-				})} representing the output item. ('${executionResult}' was returned instead.)`,
-				itemIndex,
-			});
-		}
-
-		if (Array.isArray(executionResult)) {
-			const firstSentence =
-				executionResult.length > 0
-					? `An array of ${typeof executionResult[0]}s was returned.`
-					: 'An empty array was returned.';
-			throw new ValidationError({
-				message: `Code doesn't return a single ${this.getTextKey('object')}`,
-				description: `${firstSentence} If you need to output multiple items, please use the 'Run Once for All Items' mode instead.`,
-				itemIndex,
-			});
-		}
-
-		const [returnData] = this.helpers.normalizeItems([executionResult]);
-
-		this.validateItem(returnData, itemIndex);
-
-		// If at least one top-level key is a supported item key (`json`, `binary`, etc.),
-		// and another top-level key is unrecognized, then the user mis-added a property
-		// directly on the item, when they intended to add it on the `json` property
-		this.validateTopLevelKeys(returnData, itemIndex);
-
-		return returnData;
+		return validateRunCodeEachItem(
+			executionResult,
+			itemIndex,
+			this.textKeys,
+			this.helpers.normalizeItems.bind(this.helpers),
+		);
 	}
 
 	validateRunCodeAllItems(
 		executionResult: INodeExecutionData | INodeExecutionData[] | undefined,
 	): INodeExecutionData[] {
-		if (typeof executionResult !== 'object') {
-			throw new ValidationError({
-				message: "Code doesn't return items properly",
-				description: `Please return an array of ${this.getTextKey('object', {
-					plural: true,
-				})}, one for each item you would like to output.`,
-			});
-		}
-
-		if (Array.isArray(executionResult)) {
-			/**
-			 * If at least one top-level key is an n8n item key (`json`, `binary`, etc.),
-			 * then require all item keys to be an n8n item key.
-			 *
-			 * If no top-level key is an n8n key, then skip this check, allowing non-n8n
-			 * item keys to be wrapped in `json` when normalizing items below.
-			 */
-			const mustHaveTopLevelN8nKey = executionResult.some((item) =>
-				Object.keys(item).find((key) => REQUIRED_N8N_ITEM_KEYS.has(key)),
-			);
-
-			if (mustHaveTopLevelN8nKey) {
-				for (let index = 0; index < executionResult.length; index++) {
-					const item = executionResult[index];
-					this.validateTopLevelKeys(item, index);
-				}
-			}
-		}
-
-		const returnData = this.helpers.normalizeItems(executionResult);
-		returnData.forEach((item, index) => this.validateItem(item, index));
-		return returnData;
-	}
-
-	private getTextKey(
-		key: keyof SandboxTextKeys,
-		options?: { includeArticle?: boolean; plural?: boolean },
-	) {
-		const response = this.textKeys[key][options?.plural ? 'plural' : 'singular'];
-		if (!options?.includeArticle) {
-			return response;
-		}
-		if (['a', 'e', 'i', 'o', 'u'].some((value) => response.startsWith(value))) {
-			return `an ${response}`;
-		}
-		return `a ${response}`;
-	}
-
-	private validateItem({ json, binary }: INodeExecutionData, itemIndex: number) {
-		if (json === undefined || !isObject(json)) {
-			throw new ValidationError({
-				message: `A 'json' property isn't ${this.getTextKey('object', { includeArticle: true })}`,
-				description: `In the returned data, every key named 'json' must point to ${this.getTextKey(
-					'object',
-					{ includeArticle: true },
-				)}.`,
-				itemIndex,
-			});
-		}
-
-		if (binary !== undefined && !isObject(binary)) {
-			throw new ValidationError({
-				message: `A 'binary' property isn't ${this.getTextKey('object', { includeArticle: true })}`,
-				description: `In the returned data, every key named 'binary’ must point to ${this.getTextKey(
-					'object',
-					{ includeArticle: true },
-				)}.`,
-				itemIndex,
-			});
-		}
-	}
-
-	private validateTopLevelKeys(item: INodeExecutionData, itemIndex: number) {
-		let foundReservedKey: string | null = null;
-		const unknownKeys: string[] = [];
-
-		for (const key in item) {
-			if (!Object.prototype.hasOwnProperty.call(item, key)) continue;
-
-			if (REQUIRED_N8N_ITEM_KEYS.has(key)) {
-				foundReservedKey ??= key;
-			} else {
-				unknownKeys.push(key);
-			}
-		}
-
-		if (unknownKeys.length > 0) {
-			if (foundReservedKey) throw new ReservedKeyFoundError(foundReservedKey, itemIndex);
-
-			throw new ValidationError({
-				message: `Unknown top-level item key: ${unknownKeys[0]}`,
-				description: 'Access the properties of an item under `.json`, e.g. `item.json`',
-				itemIndex,
-			});
-		}
-	}
-}
-
-class ReservedKeyFoundError extends ValidationError {
-	constructor(reservedKey: string, itemIndex: number) {
-		super({
-			message: 'Invalid output format',
-			description: `An output item contains the reserved key <code>${reservedKey}</code>. To get around this, please wrap each item in an object, under a key called <code>json</code>. <a href="https://docs.n8n.io/data/data-structure/#data-structure" target="_blank">Example</a>`,
-			itemIndex,
-		});
+		return validateRunCodeAllItems(
+			executionResult,
+			this.textKeys,
+			this.helpers.normalizeItems.bind(this.helpers),
+		);
 	}
 }

packages/nodes-base/nodes/Code/reserved-key-found-error.ts

@@ -0,0 +1,11 @@
+import { ValidationError } from './ValidationError';
+
+export class ReservedKeyFoundError extends ValidationError {
+	constructor(reservedKey: string, itemIndex: number) {
+		super({
+			message: 'Invalid output format',
+			description: `An output item contains the reserved key <code>${reservedKey}</code>. To get around this, please wrap each item in an object, under a key called <code>json</code>. <a href="https://docs.n8n.io/data/data-structure/#data-structure" target="_blank">Example</a>`,
+			itemIndex,
+		});
+	}
+}

packages/nodes-base/nodes/Code/result-validation.ts

@@ -0,0 +1,174 @@
+import type { INodeExecutionData } from 'n8n-workflow';
+
+import { ReservedKeyFoundError } from './reserved-key-found-error';
+import { isObject } from './utils';
+import { ValidationError } from './ValidationError';
+
+export interface TextKeys {
+	object: {
+		singular: string;
+		plural: string;
+	};
+}
+
+export const REQUIRED_N8N_ITEM_KEYS = new Set(['json', 'binary', 'pairedItem', 'error', 'index']);
+
+export function getTextKey(
+	textKeys: TextKeys,
+	key: keyof TextKeys,
+	options?: { includeArticle?: boolean; plural?: boolean },
+) {
+	const response = textKeys[key][options?.plural ? 'plural' : 'singular'];
+	if (!options?.includeArticle) {
+		return response;
+	}
+	if (['a', 'e', 'i', 'o', 'u'].some((value) => response.startsWith(value))) {
+		return `an ${response}`;
+	}
+	return `a ${response}`;
+}
+
+export function validateItem(
+	{ json, binary }: INodeExecutionData,
+	itemIndex: number,
+	textKeys: TextKeys,
+) {
+	if (json === undefined || !isObject(json)) {
+		throw new ValidationError({
+			message: `A 'json' property isn't ${getTextKey(textKeys, 'object', { includeArticle: true })}`,
+			description: `In the returned data, every key named 'json' must point to ${getTextKey(
+				textKeys,
+				'object',
+				{ includeArticle: true },
+			)}.`,
+			itemIndex,
+		});
+	}
+
+	if (binary !== undefined && !isObject(binary)) {
+		throw new ValidationError({
+			message: `A 'binary' property isn't ${getTextKey(textKeys, 'object', { includeArticle: true })}`,
+			description: `In the returned data, every key named 'binary' must point to ${getTextKey(
+				textKeys,
+				'object',
+				{ includeArticle: true },
+			)}.`,
+			itemIndex,
+		});
+	}
+}
+
+export function validateTopLevelKeys(item: INodeExecutionData, itemIndex: number) {
+	let foundReservedKey: string | null = null;
+	const unknownKeys: string[] = [];
+
+	for (const key in item) {
+		if (!Object.prototype.hasOwnProperty.call(item, key)) continue;
+
+		if (REQUIRED_N8N_ITEM_KEYS.has(key)) {
+			foundReservedKey ??= key;
+		} else {
+			unknownKeys.push(key);
+		}
+	}
+
+	if (unknownKeys.length > 0) {
+		if (foundReservedKey) throw new ReservedKeyFoundError(foundReservedKey, itemIndex);
+
+		throw new ValidationError({
+			message: `Unknown top-level item key: ${unknownKeys[0]}`,
+			description: 'Access the properties of an item under `.json`, e.g. `item.json`',
+			itemIndex,
+		});
+	}
+}
+
+export function validateRunCodeEachItem(
+	executionResult: INodeExecutionData | undefined,
+	itemIndex: number,
+	textKeys: TextKeys,
+	normalizeItems: (items: INodeExecutionData[]) => INodeExecutionData[],
+): INodeExecutionData {
+	if (typeof executionResult !== 'object') {
+		throw new ValidationError({
+			message: `Code doesn't return ${getTextKey(textKeys, 'object', { includeArticle: true })}`,
+			description: `Please return ${getTextKey(textKeys, 'object', {
+				includeArticle: true,
+			})} representing the output item. ('${executionResult}' was returned instead.)`,
+			itemIndex,
+		});
+	}
+
+	if (Array.isArray(executionResult)) {
+		const firstSentence =
+			executionResult.length > 0
+				? `An array of ${typeof executionResult[0]}s was returned.`
+				: 'An empty array was returned.';
+		throw new ValidationError({
+			message: `Code doesn't return a single ${getTextKey(textKeys, 'object')}`,
+			description: `${firstSentence} If you need to output multiple items, please use the 'Run Once for All Items' mode instead.`,
+			itemIndex,
+		});
+	}
+
+	const [returnData] = normalizeItems([executionResult]);
+
+	validateItem(returnData, itemIndex, textKeys);
+
+	// If at least one top-level key is a supported item key (`json`, `binary`, etc.),
+	// and another top-level key is unrecognized, then the user mis-added a property
+	// directly on the item, when they intended to add it on the `json` property
+	validateTopLevelKeys(returnData, itemIndex);
+
+	return returnData;
+}
+
+export function validateRunCodeAllItems(
+	executionResult: INodeExecutionData | INodeExecutionData[] | undefined,
+	textKeys: TextKeys,
+	normalizeItems: (items: INodeExecutionData | INodeExecutionData[]) => INodeExecutionData[],
+): INodeExecutionData[] {
+	if (typeof executionResult !== 'object') {
+		throw new ValidationError({
+			message: "Code doesn't return items properly",
+			description: `Please return an array of ${getTextKey(textKeys, 'object', {
+				plural: true,
+			})}, one for each item you would like to output.`,
+		});
+	}
+
+	if (Array.isArray(executionResult)) {
+		/**
+		 * If at least one top-level key is an n8n item key (`json`, `binary`, etc.),
+		 * then require all item keys to be an n8n item key.
+		 *
+		 * If no top-level key is an n8n key, then skip this check, allowing non-n8n
+		 * item keys to be wrapped in `json` when normalizing items below.
+		 */
+		for (const item of executionResult) {
+			if (!isObject(item)) {
+				throw new ValidationError({
+					message: "Code doesn't return items properly",
+					description: `Please return an array of ${getTextKey(textKeys, 'object', {
+						plural: true,
+					})}, one for each item you would like to output.`,
+				});
+			}
+		}
+
+		const mustHaveTopLevelN8nKey = executionResult.some((item) =>
+			Object.keys(item).find((key) => REQUIRED_N8N_ITEM_KEYS.has(key)),
+		);
+
+		if (mustHaveTopLevelN8nKey) {
+			for (let index = 0; index < executionResult.length; index++) {
+				const item = executionResult[index];
+				validateTopLevelKeys(item, index);
+			}
+		}
+	}
+
+	const returnData = normalizeItems(executionResult);
+	returnData.forEach((item, index) => validateItem(item, index, textKeys));
+	return returnData;
+}

packages/nodes-base/nodes/Code/test/JsTaskRunnerSandbox.test.ts

@@ -11,6 +11,13 @@ describe('JsTaskRunnerSandbox', () => {
 			const nodeMode = 'runOnceForEachItem';
 			const workflowMode = 'manual';
 			const executeFunctions = mock<IExecuteFunctions>();
+			executeFunctions.helpers = {
+				...executeFunctions.helpers,
+				normalizeItems: jest
+					.fn()
+					.mockImplementation((items) => (Array.isArray(items) ? items : [items])),
+			};
+
 			const sandbox = new JsTaskRunnerSandbox(jsCode, nodeMode, workflowMode, executeFunctions, 2);
 			let i = 1;
 			executeFunctions.startJob.mockResolvedValue(createResultOk([{ json: { item: i++ } }]));