fix(core): Align VM expression engine error handler with legacy engine (#28166)

Co-authored-by: Danny Martini <danny@n8n.io>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

packages/@n8n/expression-runtime/src/__tests__/integration.test.ts

@@ -377,58 +377,34 @@ describe('Integration: ExpressionEvaluator + IsolatedVmBridge', () => {
 
 	it('should handle throw null without crashing', () => {
 		const data = { $json: {} };
-		let error: Error | undefined;
-		try {
-			evaluator.evaluate('{{ (() => { throw null })() }}', data, caller);
-		} catch (e) {
-			error = e as Error;
-		}
-		expect(error).toBeDefined();
-		expect(error?.message).not.toContain('Cannot read properties');
+		expect(evaluator.evaluate('{{ (() => { throw null })() }}', data, caller)).toBeUndefined();
 	});
 
 	it('should handle throw undefined without crashing', () => {
 		const data = { $json: {} };
-		let error: Error | undefined;
-		try {
-			evaluator.evaluate('{{ (() => { throw undefined })() }}', data, caller);
-		} catch (e) {
-			error = e as Error;
-		}
-		expect(error).toBeDefined();
-		expect(error?.message).not.toContain('Cannot read properties');
+		expect(evaluator.evaluate('{{ (() => { throw undefined })() }}', data, caller)).toBeUndefined();
 	});
 
 	it('should handle throw of null-prototype object with properties without crashing', () => {
 		const data = { $json: {} };
-		let error: Error | undefined;
-		try {
+		expect(
 			evaluator.evaluate(
 				'{{ (() => { var e = Object.create(null); e.foo = "bar"; throw e; })() }}',
 				data,
 				caller,
-			);
-		} catch (e) {
-			error = e as Error;
-		}
-		expect(error).toBeDefined();
-		expect(error?.message).not.toContain('hasOwnProperty is not a function');
+			),
+		).toBeUndefined();
 	});
 
 	it('should handle throw of object with hasOwnProperty shadowed by null without crashing', () => {
 		const data = { $json: {} };
-		let error: Error | undefined;
-		try {
+		expect(
 			evaluator.evaluate(
 				'{{ (() => { throw { hasOwnProperty: null, foo: "bar" }; })() }}',
 				data,
 				caller,
-			);
-		} catch (e) {
-			error = e as Error;
-		}
-		expect(error).toBeDefined();
-		expect(error?.message).not.toContain('hasOwnProperty is not a function');
+			),
+		).toBeUndefined();
 	});
 
 	it('should swallow TypeError and return undefined', () => {
@@ -445,19 +421,48 @@ describe('Integration: ExpressionEvaluator + IsolatedVmBridge', () => {
 		expect(result).toBeUndefined();
 	});
 
-	it('should propagate errors thrown when reading a property across the isolate boundary', () => {
+	it('should re-throw ExpressionError from host-side callbacks', () => {
+		const json = {
+			get brokenProp() {
+				const err = new Error('paired item failed');
+				err.name = 'ExpressionError';
+				throw err;
+			},
+		};
+
+		expect(() => evaluator.evaluate('{{ $json.brokenProp }}', { $json: json }, caller)).toThrow(
+			expect.objectContaining({ name: 'ExpressionError', message: 'paired item failed' }),
+		);
+	});
+
+	it('should re-throw ExpressionExtensionError from host-side callbacks', () => {
+		const json = {
+			get brokenProp() {
+				const err = new Error('extension failed');
+				err.name = 'ExpressionExtensionError';
+				throw err;
+			},
+		};
+
+		expect(() => evaluator.evaluate('{{ $json.brokenProp }}', { $json: json }, caller)).toThrow(
+			expect.objectContaining({
+				name: 'ExpressionExtensionError',
+				message: 'extension failed',
+			}),
+		);
+	});
+
+	it('should swallow generic errors thrown when reading a property across the isolate boundary', () => {
 		const json = {
 			get brokenProp() {
 				throw new Error('property access failed');
 			},
 		};
 
-		expect(() => evaluator.evaluate('{{ $json.brokenProp }}', { $json: json }, caller)).toThrow(
-			'property access failed',
-		);
+		expect(evaluator.evaluate('{{ $json.brokenProp }}', { $json: json }, caller)).toBeUndefined();
 	});
 
-	it('should propagate errors thrown by functions accessed via the lazy proxy', () => {
+	it('should swallow generic errors thrown by functions accessed via the lazy proxy', () => {
 		const data = {
 			$json: {
 				myFn() {
@@ -466,22 +471,20 @@ describe('Integration: ExpressionEvaluator + IsolatedVmBridge', () => {
 			},
 		};
 
-		expect(() => evaluator.evaluate('{{ $json.myFn() }}', data, caller)).toThrow('function threw');
+		expect(evaluator.evaluate('{{ $json.myFn() }}', data, caller)).toBeUndefined();
 	});
 
-	it('should propagate errors from $items() when result properties are accessed', () => {
+	it('should swallow generic errors from $items() when result properties are accessed', () => {
 		const data = {
 			$items() {
 				throw new Error('items failed');
 			},
 		};
 
-		// Without throwIfErrorSentinel in the $items wrapper, the sentinel is
-		// returned as a value and .length reads undefined on it — silently swallowed
-		expect(() => evaluator.evaluate('{{ $items().length }}', data, caller)).toThrow('items failed');
+		expect(evaluator.evaluate('{{ $items().length }}', data, caller)).toBeUndefined();
 	});
 
-	it('should propagate errors thrown during array element access across the isolate boundary', () => {
+	it('should swallow generic errors thrown during array element access across the isolate boundary', () => {
 		const items = [1, 2, 3];
 		Object.defineProperty(items, '0', {
 			get() {
@@ -493,12 +496,10 @@ describe('Integration: ExpressionEvaluator + IsolatedVmBridge', () => {
 
 		const data = { $json: { items } };
 
-		expect(() => evaluator.evaluate('{{ $json.items[0] }}', data, caller)).toThrow(
-			'element access failed',
-		);
+		expect(evaluator.evaluate('{{ $json.items[0] }}', data, caller)).toBeUndefined();
 	});
 
-	it('should propagate errors thrown during an "in" operator check across the isolate boundary', () => {
+	it('should swallow generic errors thrown during an "in" operator check across the isolate boundary', () => {
 		const json = {
 			get brokenProp() {
 				throw new Error('in-check access failed');
@@ -507,11 +508,9 @@ describe('Integration: ExpressionEvaluator + IsolatedVmBridge', () => {
 
 		// The 'in' operator triggers the has trap on $json proxy.
 		// The bridge calls __getValueAtPath(['$json', 'brokenProp']) which throws.
-		// Without throwIfErrorSentinel in the has trap, the sentinel is returned
-		// as a non-undefined value so 'brokenProp' in $json incorrectly returns true.
-		expect(() =>
+		expect(
 			evaluator.evaluate('{{ "brokenProp" in $json }}', { $json: json }, caller),
-		).toThrow('in-check access failed');
+		).toBeUndefined();
 	});
 });
 

packages/@n8n/expression-runtime/src/bridge/isolated-vm-bridge.ts

@@ -252,11 +252,22 @@ export class IsolatedVmBridge implements RuntimeBridge {
 	/**
 	 * Inject the E() error handler into the isolate context.
 	 *
-	 * Tournament wraps expressions with try-catch that calls E(error, this).
-	 * This handler:
-	 * - Re-throws security violations from __sanitize
-	 * - Swallows TypeErrors (failed attack attempts return undefined)
-	 * - Re-throws all other errors
+	 * There are two exception-handling layers inside the isolate:
+	 *
+	 * 1. **Inner layer (this handler, `E()`)** — Tournament wraps each
+	 *    expression with try-catch that calls `E(error, this)`. This handler
+	 *    must match the legacy engine's behavior (set in expression.ts via
+	 *    setErrorHandler):
+	 *    - Re-throw ExpressionError / ExpressionExtensionError
+	 *    - Swallow everything else (TypeErrors, generic Errors, etc.)
+	 *
+	 * 2. **Outer layer (`wrappedCode` try-catch in `execute()`)** — Catches
+	 *    anything that escaped `E()` (e.g. re-thrown ExpressionErrors) and
+	 *    serializes it into a sentinel object so the host can reconstruct it.
+	 *
+	 * Inside the isolate, errors from host callbacks arrive as sentinel
+	 * objects ({ __isError, name, message, ... }) rather than class instances,
+	 * so we match by name instead of instanceof.
 	 *
 	 * @private
 	 * @throws {Error} If context not initialized
@@ -269,15 +280,15 @@ export class IsolatedVmBridge implements RuntimeBridge {
 		await this.context.eval(`
 			if (typeof E === 'undefined') {
 				globalThis.E = function(error, _context) {
-					// Re-throw security violations from __sanitize
-					if (error && error.message && error.message.includes('due to security concerns')) {
+					// Re-throw ExpressionError / ExpressionExtensionError to match
+					// the legacy handler in expression.ts. Errors from host callbacks
+					// arrive as sentinels (not class instances), so check by name.
+					const name = error?.name;
+					if (name === 'ExpressionError' || name === 'ExpressionExtensionError') {
 						throw error;
 					}
-					// Swallow TypeErrors (failed attack attempts return undefined)
-					if (error instanceof TypeError) {
-						return undefined;
-					}
-					throw error;
+					// Swallow everything else (TypeErrors, generic Errors, etc.)
+					return undefined;
 				};
 			}
 		`);

packages/workflow/test/expression.test.ts

@@ -10,6 +10,7 @@ import { ExpressionReservedVariableError } from '../src/errors/expression-reserv
 import { ExpressionError } from '../src/errors/expression.error';
 import { Expression } from '../src/expression';
 import { extendSyntax } from '../src/extensions/expression-extension';
+import { createRunExecutionData } from '../src';
 import type { INodeExecutionData } from '../src/interfaces';
 import { Workflow } from '../src/workflow';
 import { WorkflowDataProxy } from '../src/workflow-data-proxy';
@@ -924,6 +925,109 @@ describe('Expression', () => {
 		});
 	});
 
+	describe('$() node reference through expression engine', () => {
+		const nodeTypes = Helpers.NodeTypes();
+
+		function createTestWorkflow(connected: boolean) {
+			return new Workflow({
+				id: 'test-dollar-ref',
+				name: 'Test',
+				nodes: [
+					{
+						id: 'source-id',
+						name: 'source',
+						type: 'n8n-nodes-base.set',
+						typeVersion: 1,
+						position: [0, 0],
+						parameters: {},
+					},
+					{
+						id: 'consumer-id',
+						name: 'consumer',
+						type: 'n8n-nodes-base.set',
+						typeVersion: 1,
+						position: [200, 0],
+						parameters: {},
+					},
+				],
+				connections: connected
+					? { source: { main: [[{ node: 'consumer', type: 'main', index: 0 }]] } }
+					: {},
+				active: false,
+				nodeTypes,
+			});
+		}
+
+		const runExecutionData = createRunExecutionData({
+			resultData: {
+				runData: {
+					source: [
+						{
+							startTime: 1,
+							executionTime: 1,
+							executionIndex: 0,
+							source: [],
+							data: {
+								main: [[{ json: { city: 'Prague' }, pairedItem: { item: 0 } }]],
+							},
+						},
+					],
+				},
+			},
+		});
+
+		it("should resolve $('source').item.json.city", async () => {
+			const testWorkflow = createTestWorkflow(true);
+
+			await testWorkflow.expression.acquireIsolate();
+			try {
+				const result = testWorkflow.expression.getParameterValue(
+					"={{ $('source').item.json.city }}",
+					runExecutionData,
+					0,
+					0,
+					'consumer',
+					[{ json: { city: 'Prague' }, pairedItem: { item: 0 } }],
+					'manual',
+					{},
+					{
+						node: testWorkflow.getNode('consumer')!,
+						data: {},
+						source: {
+							main: [{ previousNode: 'source', previousNodeOutput: 0, previousNodeRun: 0 }],
+						},
+					},
+				);
+
+				expect(result).toBe('Prague');
+			} finally {
+				await testWorkflow.expression.releaseIsolate();
+			}
+		});
+
+		it('should throw ExpressionError when nodes are not connected', async () => {
+			const testWorkflow = createTestWorkflow(false);
+
+			await testWorkflow.expression.acquireIsolate();
+			try {
+				expect(() =>
+					testWorkflow.expression.getParameterValue(
+						"={{ $('source').item.json.city }}",
+						runExecutionData,
+						0,
+						0,
+						'consumer',
+						[{ json: {} }],
+						'manual',
+						{},
+					),
+				).toThrow(ExpressionError);
+			} finally {
+				await testWorkflow.expression.releaseIsolate();
+			}
+		});
+	});
+
 	describe('getParameterValue with IWorkflowDataProxyData', () => {
 		it('should evaluate simple expression with provided IWorkflowDataProxyData', async () => {
 			const nodeTypes = Helpers.NodeTypes();