fix(core)!: Allow syntax errors and expression errors to fail executions (#6352)

* fix: Unify expression error behavior for v1

* fix: Add `package.json` to `tsconfig.build.json`

* fix: Make `isFrontend` a constant

* fix: Use CommonJS require to read version

* fix: Use `JSON.parse()` and `fs.readFileSync()`

* feat(editor): Make WF name a link on /executions (#6354)

* make wf name a link in exec view

* link color

* make wf name a link in exec view

* link color

---------

Co-authored-by: Alex Grozav <alex@grozav.com>

* fix: Try restoring inclusions in tsconfig files

* fix: Try with copy

* refactor: Switch base branch and remove global toggle

* chore: Remove unrelated changes

* chore: Restore lockfile

* fix: Ensure all expression errors fail executions

* uncaught ExpressionErrors should not fail e2e tests

---------

Co-authored-by: romainminaud <romain.minaud@gmail.com>
Co-authored-by: Alex Grozav <alex@grozav.com>
Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <aditya@netroy.in>

cypress/e2e/11-inline-expression-editor.cy.ts

@@ -21,6 +21,8 @@ describe('Inline expression editor', () => {
 		WorkflowPage.actions.addNodeToCanvas('Hacker News');
 		WorkflowPage.actions.openNode('Hacker News');
 		WorkflowPage.actions.openInlineExpressionEditor();
+
+		cy.on('uncaught:exception', (err) => err.name !== 'ExpressionError');
 	});
 
 	it('should resolve primitive resolvables', () => {

cypress/e2e/9-expression-editor-modal.cy.ts

@@ -21,6 +21,8 @@ describe('Expression editor modal', () => {
 		WorkflowPage.actions.addNodeToCanvas('Hacker News');
 		WorkflowPage.actions.openNode('Hacker News');
 		WorkflowPage.actions.openExpressionEditorModal();
+
+		cy.on('uncaught:exception', (err) => err.name !== 'ExpressionError');
 	});
 
 	it('should resolve primitive resolvables', () => {

packages/core/src/NodeExecuteFunctions.ts

@@ -1982,7 +1982,6 @@ const validateValueAgainstSchema = (
 			}' [item ${itemIndex}]`,
 			{
 				description: validationResult.errorMessage,
-				failExecution: true,
 				runIndex,
 				itemIndex,
 				nodeCause: node.name,

packages/workflow/src/Expression.ts

@@ -22,21 +22,14 @@ import type { Workflow } from './Workflow';
 import { extend, extendOptional } from './Extensions';
 import { extendedFunctions } from './Extensions/ExtendedFunctions';
 import { extendSyntax } from './Extensions/ExpressionExtension';
+import { isExpressionError, IS_FRONTEND, isSyntaxError, isTypeError } from './utils';
 
 // Set it to use double curly brackets instead of single ones
 tmpl.brackets.set('{{ }}');
 
 // Make sure that error get forwarded
 tmpl.tmpl.errorHandler = (error: Error) => {
-	if (error instanceof ExpressionError) {
-		if (error.context.failExecution) {
-			throw error;
-		}
-
-		if (typeof process === 'undefined' && error.clientOnly) {
-			throw error;
-		}
-	}
+	if (isExpressionError(error)) throw error;
 };
 
 // eslint-disable-next-line @typescript-eslint/naming-convention
@@ -332,35 +325,11 @@ export class Expression {
 			);
 			return tmpl.tmpl(expression, data);
 		} catch (error) {
-			if (error instanceof ExpressionError) {
-				// Ignore all errors except if they are ExpressionErrors and they are supposed
-				// to fail the execution
-				if (error.context.failExecution) {
-					throw error;
-				}
+			if (isExpressionError(error)) throw error;
 
-				if (typeof process === 'undefined' && error.clientOnly) {
-					throw error;
-				}
-			}
+			if (isSyntaxError(error)) throw new Error('invalid syntax');
 
-			// Syntax errors resolve to `Error` on the frontend and `null` on the backend.
-			// This is a temporary divergence in evaluation behavior until we make the
-			// breaking change to allow syntax errors to fail executions.
-			if (
-				typeof process === 'undefined' &&
-				error instanceof Error &&
-				error.name === 'SyntaxError'
-			) {
-				throw new Error('invalid syntax');
-			}
-
-			if (
-				typeof process === 'undefined' &&
-				error instanceof Error &&
-				error.name === 'TypeError' &&
-				error.message.endsWith('is not a function')
-			) {
+			if (isTypeError(error) && IS_FRONTEND && error.message.endsWith('is not a function')) {
 				const match = error.message.match(/(?<msg>[^.]+is not a function)/);
 
 				if (!match?.groups?.msg) return null;
@@ -373,6 +342,7 @@ export class Expression {
 				value: fnConstructors.async,
 			});
 		}
+
 		return null;
 	}
 

packages/workflow/src/ExpressionError.ts

@@ -5,8 +5,6 @@ import { ExecutionBaseError } from './NodeErrors';
  * Class for instantiating an expression error
  */
 export class ExpressionError extends ExecutionBaseError {
-	clientOnly = false;
-
 	constructor(
 		message: string,
 		options?: {
@@ -14,8 +12,6 @@ export class ExpressionError extends ExecutionBaseError {
 			causeDetailed?: string;
 			description?: string;
 			descriptionTemplate?: string;
-			failExecution?: boolean;
-			clientOnly?: boolean; // whether to throw error only on frontend
 			functionality?: 'pairedItem';
 			itemIndex?: number;
 			messageTemplate?: string;
@@ -31,12 +27,6 @@ export class ExpressionError extends ExecutionBaseError {
 			this.description = options.description;
 		}
 
-		if (options?.clientOnly) {
-			this.clientOnly = options.clientOnly;
-		}
-
-		this.context.failExecution = !!options?.failExecution;
-
 		const allowedKeys = [
 			'causeDetailed',
 			'descriptionTemplate',
@@ -58,9 +48,4 @@ export class ExpressionError extends ExecutionBaseError {
 	}
 }
 
-export class ExpressionExtensionError extends ExpressionError {
-	constructor(message: string) {
-		super(message);
-		this.context.failExecution = true;
-	}
-}
+export class ExpressionExtensionError extends ExpressionError {}

packages/workflow/src/WorkflowDataProxy.ts

@@ -468,7 +468,6 @@ export class WorkflowDataProxy {
 						throw new ExpressionError('not accessible via UI, please run node', {
 							runIndex: that.runIndex,
 							itemIndex: that.itemIndex,
-							failExecution: true,
 						});
 					}
 					if (process.env.N8N_BLOCK_ENV_ACCESS_IN_NODE === 'true') {
@@ -477,7 +476,6 @@ export class WorkflowDataProxy {
 								'If you need access please contact the administrator to remove the environment variable ‘N8N_BLOCK_ENV_ACCESS_IN_NODE‘',
 							runIndex: that.runIndex,
 							itemIndex: that.itemIndex,
-							failExecution: true,
 						});
 					}
 					return process.env[name.toString()];
@@ -560,7 +558,6 @@ export class WorkflowDataProxy {
 								description: 'Please save the workflow first to use $workflow',
 								runIndex: that.runIndex,
 								itemIndex: that.itemIndex,
-								failExecution: true,
 							});
 						}
 
@@ -592,8 +589,6 @@ export class WorkflowDataProxy {
 						throw new ExpressionError(`"${nodeName}" node doesn't exist`, {
 							runIndex: that.runIndex,
 							itemIndex: that.itemIndex,
-							// TODO: re-enable this for v1.0.0 release
-							// failExecution: true,
 						});
 					}
 
@@ -630,7 +625,6 @@ export class WorkflowDataProxy {
 				throw new ExpressionError('expected two arguments (Object, string) for this function', {
 					runIndex: that.runIndex,
 					itemIndex: that.itemIndex,
-					clientOnly: true,
 				});
 			}
 
@@ -696,7 +690,6 @@ export class WorkflowDataProxy {
 			return new ExpressionError(message, {
 				runIndex: that.runIndex,
 				itemIndex: that.itemIndex,
-				failExecution: true,
 				...context,
 			});
 		};

packages/workflow/src/utils.ts

@@ -1,3 +1,4 @@
+import { ExpressionError, ExpressionExtensionError } from './ExpressionError';
 import type { BinaryFileType } from './Interfaces';
 
 const readStreamClasses = new Set(['ReadStream', 'Readable', 'ReadableStream']);
@@ -127,3 +128,20 @@ export function assert<T>(condition: T, msg?: string): asserts condition {
 		throw error;
 	}
 }
+
+const IS_FRONTEND_IN_DEV_MODE =
+	typeof process === 'object' &&
+	Object.keys(process).length === 1 &&
+	'env' in process &&
+	Object.keys(process.env).length === 0;
+
+export const IS_FRONTEND = typeof process === 'undefined' || IS_FRONTEND_IN_DEV_MODE;
+
+export const isSyntaxError = (error: unknown): error is SyntaxError =>
+	error instanceof SyntaxError || (error instanceof Error && error.name === 'SyntaxError');
+
+export const isExpressionError = (error: unknown): error is ExpressionError =>
+	error instanceof ExpressionError || error instanceof ExpressionExtensionError;
+
+export const isTypeError = (error: unknown): error is TypeError =>
+	error instanceof TypeError || (error instanceof Error && error.name === 'TypeError');

packages/workflow/test/Expression.test.ts

@@ -10,6 +10,7 @@ import type { ExpressionTestEvaluation, ExpressionTestTransform } from './Expres
 import { baseFixtures } from './ExpressionFixtures/base';
 import type { INodeExecutionData } from '@/Interfaces';
 import { extendSyntax } from '@/Extensions/ExpressionExtension';
+import { ExpressionError } from '@/ExpressionError';
 
 describe('Expression', () => {
 	describe('getParameterValue()', () => {
@@ -154,7 +155,9 @@ describe('Expression', () => {
 		it('should not able to do arbitrary code execution', () => {
 			const testFn = jest.fn();
 			Object.assign(global, { testFn });
-			evaluate("={{ Date['constructor']('testFn()')()}}");
+			expect(() => evaluate("={{ Date['constructor']('testFn()')()}}")).toThrowError(
+				new ExpressionError('Arbitrary code execution detected'),
+			);
 			expect(testFn).not.toHaveBeenCalled();
 		});
 	});
@@ -180,7 +183,18 @@ describe('Expression', () => {
 		const expression = new Expression(workflow);
 
 		const evaluate = (value: string, data: INodeExecutionData[]) => {
-			return expression.getParameterValue(value, null, 0, 0, 'node', data, 'manual', '', {});
+			const itemIndex = data.length === 0 ? -1 : 0;
+			return expression.getParameterValue(
+				value,
+				null,
+				0,
+				itemIndex,
+				'node',
+				data,
+				'manual',
+				'',
+				{},
+			);
 		};
 
 		for (const t of baseFixtures) {