From 75ba58f0b7ed0b101a90f31f6defdcafce28ce11 Mon Sep 17 00:00:00 2001 From: Valya Bullions Date: Wed, 21 Jun 2023 13:52:05 +0100 Subject: [PATCH 01/14] add new expression evaluator and setting to toggle it --- packages/cli/src/ExpressionEvalator.ts | 6 + packages/cli/src/Server.ts | 3 + packages/cli/src/commands/BaseCommand.ts | 2 + packages/cli/src/config/schema.ts | 9 + .../editor-ui/src/mixins/workflowHelpers.ts | 9 +- packages/workflow/src/Expression.ts | 38 +- .../workflow/src/ExpressionEvaluatorProxy.ts | 54 +++ packages/workflow/src/Interfaces.ts | 5 + packages/workflow/src/index.ts | 1 + packages/workflow/test/Expression.test.ts | 370 +++++++++--------- 10 files changed, 302 insertions(+), 195 deletions(-) create mode 100644 packages/cli/src/ExpressionEvalator.ts create mode 100644 packages/workflow/src/ExpressionEvaluatorProxy.ts diff --git a/packages/cli/src/ExpressionEvalator.ts b/packages/cli/src/ExpressionEvalator.ts new file mode 100644 index 0000000000000..4f1f5149a2d7b --- /dev/null +++ b/packages/cli/src/ExpressionEvalator.ts @@ -0,0 +1,6 @@ +import config from '@/config'; +import { ExpressionEvaluatorProxy } from 'n8n-workflow'; + +export const initExpressionEvaluator = () => { + ExpressionEvaluatorProxy.setEvaluator(config.getEnv('expression.evaluator')); +}; diff --git a/packages/cli/src/Server.ts b/packages/cli/src/Server.ts index 545c6449b20f2..0401dccc38ba1 100644 --- a/packages/cli/src/Server.ts +++ b/packages/cli/src/Server.ts @@ -322,6 +322,9 @@ export class Server extends AbstractServer { variables: { limit: 0, }, + expressions: { + evaluator: config.get('expression.evaluator') as 'tmpl' | 'tournament', + }, }; } diff --git a/packages/cli/src/commands/BaseCommand.ts b/packages/cli/src/commands/BaseCommand.ts index 9b190542d5a87..40988d9abc34c 100644 --- a/packages/cli/src/commands/BaseCommand.ts +++ b/packages/cli/src/commands/BaseCommand.ts @@ -20,6 +20,7 @@ import type { IExternalHooksClass } from '@/Interfaces'; import { InternalHooks } from '@/InternalHooks'; import { PostHogClient } from '@/posthog'; import { License } from '@/License'; +import { initExpressionEvaluator } from '@/ExpressionEvalator'; export const UM_FIX_INSTRUCTION = 'Please fix the database by running ./packages/cli/bin/n8n user-management:reset'; @@ -41,6 +42,7 @@ export abstract class BaseCommand extends Command { async init(): Promise { await initErrorHandling(); + initExpressionEvaluator(); process.once('SIGTERM', async () => this.stopProcess()); process.once('SIGINT', async () => this.stopProcess()); diff --git a/packages/cli/src/config/schema.ts b/packages/cli/src/config/schema.ts index c25db2b255f65..1bc50c9d36226 100644 --- a/packages/cli/src/config/schema.ts +++ b/packages/cli/src/config/schema.ts @@ -1185,4 +1185,13 @@ export const schema = { }, }, }, + + expression: { + evaluator: { + doc: 'Expression evaluator to use', + format: ['tmpl', 'tournament'] as const, + default: 'tmpl', + env: 'N8N_EXPRESSION_EVALUATOR', + }, + }, }; diff --git a/packages/editor-ui/src/mixins/workflowHelpers.ts b/packages/editor-ui/src/mixins/workflowHelpers.ts index 93fbefec7895c..53960b87b3412 100644 --- a/packages/editor-ui/src/mixins/workflowHelpers.ts +++ b/packages/editor-ui/src/mixins/workflowHelpers.ts @@ -9,7 +9,7 @@ import { MODAL_CONFIRM, } from '@/constants'; -import type { +import { IConnections, IDataObject, INode, @@ -27,6 +27,7 @@ import type { IExecuteData, INodeConnection, IWebhookDescription, + ExpressionEvaluatorProxy, } from 'n8n-workflow'; import { NodeHelpers } from 'n8n-workflow'; @@ -61,7 +62,7 @@ import { useUsersStore } from '@/stores/users.store'; import type { IPermissions } from '@/permissions'; import { getWorkflowPermissions } from '@/permissions'; import type { ICredentialsResponse } from '@/Interface'; -import { useEnvironmentsStore } from '@/stores'; +import { useEnvironmentsStore, useSettingsStore } from '@/stores'; export function resolveParameter( parameter: NodeParameterValue | INodeParameters | NodeParameterValue[] | INodeParameters[], @@ -157,6 +158,10 @@ export function resolveParameter( } const _executeData = executeData(parentNode, activeNode!.name, inputName, runIndexCurrent); + ExpressionEvaluatorProxy.setEvaluator( + useSettingsStore().settings.expressions?.evaluator ?? 'tmpl', + ); + return workflow.expression.getParameterValue( parameter, runExecutionData, diff --git a/packages/workflow/src/Expression.ts b/packages/workflow/src/Expression.ts index 2d6aa4e14fb26..a18d12b5d00b5 100644 --- a/packages/workflow/src/Expression.ts +++ b/packages/workflow/src/Expression.ts @@ -1,5 +1,5 @@ -import * as tmpl from '@n8n_io/riot-tmpl'; import { DateTime, Duration, Interval } from 'luxon'; +import type * as tmpl from '@n8n_io/riot-tmpl'; import type { IExecuteData, @@ -22,12 +22,29 @@ import type { Workflow } from './Workflow'; import { extend, extendOptional } from './Extensions'; import { extendedFunctions } from './Extensions/ExtendedFunctions'; import { extendSyntax } from './Extensions/ExpressionExtension'; - -// 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) => { +import { + checkEvaluatorDifferences, + evaluateExpression, + setErrorHandler, +} from './ExpressionEvaluatorProxy'; + +// // 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; +// } +// } +// }; + +setErrorHandler((error: Error) => { if (error instanceof ExpressionError) { if (error.context.failExecution) { throw error; @@ -37,7 +54,7 @@ tmpl.tmpl.errorHandler = (error: Error) => { throw error; } } -}; +}); // eslint-disable-next-line @typescript-eslint/naming-convention const AsyncFunction = (async () => {}).constructor as FunctionConstructor; @@ -59,7 +76,7 @@ export class Expression { } static resolveWithoutWorkflow(expression: string) { - return tmpl.tmpl(expression, {}); + return evaluateExpression(expression, {}); } /** @@ -330,7 +347,8 @@ export class Expression { [Function, AsyncFunction].forEach(({ prototype }) => Object.defineProperty(prototype, 'constructor', { value: fnConstructors.mock }), ); - return tmpl.tmpl(expression, data); + checkEvaluatorDifferences(expression); + return evaluateExpression(expression, data); } catch (error) { if (error instanceof ExpressionError) { // Ignore all errors except if they are ExpressionErrors and they are supposed diff --git a/packages/workflow/src/ExpressionEvaluatorProxy.ts b/packages/workflow/src/ExpressionEvaluatorProxy.ts new file mode 100644 index 0000000000000..30ca62190ac0c --- /dev/null +++ b/packages/workflow/src/ExpressionEvaluatorProxy.ts @@ -0,0 +1,54 @@ +import * as tmpl from '@n8n_io/riot-tmpl'; +import type { TmplDifference } from '@n8n/tournament'; +import { Tournament } from '@n8n/tournament'; +import type { ExpressionEvaluatorType } from './Interfaces'; + +type Evaluator = (expr: string, data: unknown) => tmpl.ReturnValue; +type ErrorHandler = (error: Error) => void; +type DifferenceHandler = (expr: string) => void; + +// Set it to use double curly brackets instead of single ones +tmpl.brackets.set('{{ }}'); + +let errorHandler: ErrorHandler = () => {}; +let differenceHandler: DifferenceHandler = () => {}; +const differenceChecker = (diff: TmplDifference) => { + if (diff.same) { + return; + } + if (diff.has?.function || diff.has?.templateString) { + return; + } + differenceHandler(diff.expression as string); +}; +const tournamentEvaluator = new Tournament(errorHandler, undefined, differenceChecker); +let evaluator: Evaluator = tmpl.tmpl; + +export const setErrorHandler = (handler: ErrorHandler) => { + errorHandler = handler; + tmpl.tmpl.errorHandler = handler; + tournamentEvaluator.errorHandler = handler; +}; + +export const setEvaluator = (evalType: ExpressionEvaluatorType) => { + console.log('setEvaluator', evalType); + if (evalType === 'tmpl') { + evaluator = tmpl.tmpl; + } else if (evalType === 'tournament') { + evaluator = tournamentEvaluator.execute.bind(tournamentEvaluator); + } +}; + +export const setDiffReporter = (reporter: (expr: string) => void) => { + differenceHandler = reporter; +}; + +export const checkEvaluatorDifferences = (expr: string) => { + tournamentEvaluator.tmplDiff(expr); +}; + +export const getEvaluator = () => { + return evaluator; +}; + +export const evaluateExpression: Evaluator = (expr, data) => evaluator(expr, data); diff --git a/packages/workflow/src/Interfaces.ts b/packages/workflow/src/Interfaces.ts index 790b704bd2102..216568622c760 100644 --- a/packages/workflow/src/Interfaces.ts +++ b/packages/workflow/src/Interfaces.ts @@ -2039,6 +2039,8 @@ export interface IPublicApiSettings { export type ILogLevel = 'info' | 'debug' | 'warn' | 'error' | 'verbose' | 'silent'; +export type ExpressionEvaluatorType = 'tmpl' | 'tournament'; + export interface IN8nUISettings { endpointWebhook: string; endpointWebhookTest: string; @@ -2121,4 +2123,7 @@ export interface IN8nUISettings { variables: { limit: number; }; + expressions: { + evaluator: ExpressionEvaluatorType; + }; } diff --git a/packages/workflow/src/index.ts b/packages/workflow/src/index.ts index 6ab146926cf0c..1b59f34a67e27 100644 --- a/packages/workflow/src/index.ts +++ b/packages/workflow/src/index.ts @@ -1,5 +1,6 @@ import * as LoggerProxy from './LoggerProxy'; export * as ErrorReporterProxy from './ErrorReporterProxy'; +export * as ExpressionEvaluatorProxy from './ExpressionEvaluatorProxy'; import * as NodeHelpers from './NodeHelpers'; import * as ObservableObject from './ObservableObject'; import * as TelemetryHelpers from './TelemetryHelpers'; diff --git a/packages/workflow/test/Expression.test.ts b/packages/workflow/test/Expression.test.ts index 5229361440a11..ac1ab5bd156fe 100644 --- a/packages/workflow/test/Expression.test.ts +++ b/packages/workflow/test/Expression.test.ts @@ -10,208 +10,212 @@ import type { ExpressionTestEvaluation, ExpressionTestTransform } from './Expres import { baseFixtures } from './ExpressionFixtures/base'; import type { INodeExecutionData } from '@/Interfaces'; import { extendSyntax } from '@/Extensions/ExpressionExtension'; +import { setEvaluator } from '@/ExpressionEvaluatorProxy'; + +for (const evaluator of ['tmpl', 'tournament'] as const) { + setEvaluator(evaluator); + describe(`Expression (with ${evaluator})`, () => { + describe('getParameterValue()', () => { + const nodeTypes = Helpers.NodeTypes(); + const workflow = new Workflow({ + nodes: [ + { + name: 'node', + typeVersion: 1, + type: 'test.set', + id: 'uuid-1234', + position: [0, 0], + parameters: {}, + }, + ], + connections: {}, + active: false, + nodeTypes, + }); + const expression = new Expression(workflow); -describe('Expression', () => { - describe('getParameterValue()', () => { - const nodeTypes = Helpers.NodeTypes(); - const workflow = new Workflow({ - nodes: [ - { - name: 'node', - typeVersion: 1, - type: 'test.set', - id: 'uuid-1234', - position: [0, 0], - parameters: {}, - }, - ], - connections: {}, - active: false, - nodeTypes, - }); - const expression = new Expression(workflow); - - const evaluate = (value: string) => - expression.getParameterValue(value, null, 0, 0, 'node', [], 'manual', '', {}); + const evaluate = (value: string) => + expression.getParameterValue(value, null, 0, 0, 'node', [], 'manual', '', {}); - it('should not be able to use global built-ins from denylist', () => { - expect(evaluate('={{document}}')).toEqual({}); - expect(evaluate('={{window}}')).toEqual({}); + it('should not be able to use global built-ins from denylist', () => { + expect(evaluate('={{document}}')).toEqual({}); + expect(evaluate('={{window}}')).toEqual({}); - expect(evaluate('={{Window}}')).toEqual({}); - expect(evaluate('={{globalThis}}')).toEqual({}); - expect(evaluate('={{self}}')).toEqual({}); + expect(evaluate('={{Window}}')).toEqual({}); + expect(evaluate('={{globalThis}}')).toEqual({}); + expect(evaluate('={{self}}')).toEqual({}); - expect(evaluate('={{alert}}')).toEqual({}); - expect(evaluate('={{prompt}}')).toEqual({}); - expect(evaluate('={{confirm}}')).toEqual({}); + expect(evaluate('={{alert}}')).toEqual({}); + expect(evaluate('={{prompt}}')).toEqual({}); + expect(evaluate('={{confirm}}')).toEqual({}); - expect(evaluate('={{eval}}')).toEqual({}); - expect(evaluate('={{uneval}}')).toEqual({}); - expect(evaluate('={{setTimeout}}')).toEqual({}); - expect(evaluate('={{setInterval}}')).toEqual({}); - expect(evaluate('={{Function}}')).toEqual({}); + expect(evaluate('={{eval}}')).toEqual({}); + expect(evaluate('={{uneval}}')).toEqual({}); + expect(evaluate('={{setTimeout}}')).toEqual({}); + expect(evaluate('={{setInterval}}')).toEqual({}); + expect(evaluate('={{Function}}')).toEqual({}); - expect(evaluate('={{fetch}}')).toEqual({}); - expect(evaluate('={{XMLHttpRequest}}')).toEqual({}); + expect(evaluate('={{fetch}}')).toEqual({}); + expect(evaluate('={{XMLHttpRequest}}')).toEqual({}); - expect(evaluate('={{Promise}}')).toEqual({}); - expect(evaluate('={{Generator}}')).toEqual({}); - expect(evaluate('={{GeneratorFunction}}')).toEqual({}); - expect(evaluate('={{AsyncFunction}}')).toEqual({}); - expect(evaluate('={{AsyncGenerator}}')).toEqual({}); - expect(evaluate('={{AsyncGeneratorFunction}}')).toEqual({}); + expect(evaluate('={{Promise}}')).toEqual({}); + expect(evaluate('={{Generator}}')).toEqual({}); + expect(evaluate('={{GeneratorFunction}}')).toEqual({}); + expect(evaluate('={{AsyncFunction}}')).toEqual({}); + expect(evaluate('={{AsyncGenerator}}')).toEqual({}); + expect(evaluate('={{AsyncGeneratorFunction}}')).toEqual({}); - expect(evaluate('={{WebAssembly}}')).toEqual({}); + expect(evaluate('={{WebAssembly}}')).toEqual({}); - expect(evaluate('={{Reflect}}')).toEqual({}); - expect(evaluate('={{Proxy}}')).toEqual({}); + expect(evaluate('={{Reflect}}')).toEqual({}); + expect(evaluate('={{Proxy}}')).toEqual({}); - expect(evaluate('={{constructor}}')).toEqual({}); + expect(evaluate('={{constructor}}')).toEqual({}); - expect(evaluate('={{escape}}')).toEqual({}); - expect(evaluate('={{unescape}}')).toEqual({}); - }); + expect(evaluate('={{escape}}')).toEqual({}); + expect(evaluate('={{unescape}}')).toEqual({}); + }); - it('should be able to use global built-ins from allowlist', () => { - expect(evaluate('={{new Date()}}')).toBeInstanceOf(Date); - expect(evaluate('={{DateTime.now().toLocaleString()}}')).toEqual( - DateTime.now().toLocaleString(), - ); - expect(evaluate('={{Interval.after(new Date(), 100)}}')).toEqual( - Interval.after(new Date(), 100), - ); - expect(evaluate('={{Duration.fromMillis(100)}}')).toEqual(Duration.fromMillis(100)); - - expect(evaluate('={{new Object()}}')).toEqual(new Object()); - - expect(evaluate('={{new Array()}}')).toEqual([]); - expect(evaluate('={{new Int8Array()}}')).toEqual(new Int8Array()); - expect(evaluate('={{new Uint8Array()}}')).toEqual(new Uint8Array()); - expect(evaluate('={{new Uint8ClampedArray()}}')).toEqual(new Uint8ClampedArray()); - expect(evaluate('={{new Int16Array()}}')).toEqual(new Int16Array()); - expect(evaluate('={{new Uint16Array()}}')).toEqual(new Uint16Array()); - expect(evaluate('={{new Int32Array()}}')).toEqual(new Int32Array()); - expect(evaluate('={{new Uint32Array()}}')).toEqual(new Uint32Array()); - expect(evaluate('={{new Float32Array()}}')).toEqual(new Float32Array()); - expect(evaluate('={{new Float64Array()}}')).toEqual(new Float64Array()); - expect(evaluate('={{new BigInt64Array()}}')).toEqual(new BigInt64Array()); - expect(evaluate('={{new BigUint64Array()}}')).toEqual(new BigUint64Array()); - - expect(evaluate('={{new Map()}}')).toEqual(new Map()); - expect(evaluate('={{new WeakMap()}}')).toEqual(new WeakMap()); - expect(evaluate('={{new Set()}}')).toEqual(new Set()); - expect(evaluate('={{new WeakSet()}}')).toEqual(new WeakSet()); - - expect(evaluate('={{new Error()}}')).toEqual(new Error()); - expect(evaluate('={{new TypeError()}}')).toEqual(new TypeError()); - expect(evaluate('={{new SyntaxError()}}')).toEqual(new SyntaxError()); - expect(evaluate('={{new EvalError()}}')).toEqual(new EvalError()); - expect(evaluate('={{new RangeError()}}')).toEqual(new RangeError()); - expect(evaluate('={{new ReferenceError()}}')).toEqual(new ReferenceError()); - expect(evaluate('={{new URIError()}}')).toEqual(new URIError()); - - expect(evaluate('={{Intl}}')).toEqual(Intl); - - expect(evaluate('={{new String()}}')).toEqual(new String()); - expect(evaluate("={{new RegExp('')}}")).toEqual(new RegExp('')); - - expect(evaluate('={{Math}}')).toEqual(Math); - expect(evaluate('={{new Number()}}')).toEqual(new Number()); - expect(evaluate("={{BigInt('1')}}")).toEqual(BigInt('1')); - expect(evaluate('={{Infinity}}')).toEqual(Infinity); - expect(evaluate('={{NaN}}')).toEqual(NaN); - expect(evaluate('={{isFinite(1)}}')).toEqual(isFinite(1)); - expect(evaluate('={{isNaN(1)}}')).toEqual(isNaN(1)); - expect(evaluate("={{parseFloat('1')}}")).toEqual(parseFloat('1')); - expect(evaluate("={{parseInt('1', 10)}}")).toEqual(parseInt('1', 10)); - - expect(evaluate('={{JSON.stringify({})}}')).toEqual(JSON.stringify({})); - expect(evaluate('={{new ArrayBuffer(10)}}')).toEqual(new ArrayBuffer(10)); - expect(evaluate('={{new SharedArrayBuffer(10)}}')).toEqual(new SharedArrayBuffer(10)); - expect(evaluate('={{Atomics}}')).toEqual(Atomics); - expect(evaluate('={{new DataView(new ArrayBuffer(1))}}')).toEqual( - new DataView(new ArrayBuffer(1)), - ); - - expect(evaluate("={{encodeURI('https://google.com')}}")).toEqual( - encodeURI('https://google.com'), - ); - expect(evaluate("={{encodeURIComponent('https://google.com')}}")).toEqual( - encodeURIComponent('https://google.com'), - ); - expect(evaluate("={{decodeURI('https://google.com')}}")).toEqual( - decodeURI('https://google.com'), - ); - expect(evaluate("={{decodeURIComponent('https://google.com')}}")).toEqual( - decodeURIComponent('https://google.com'), - ); - - expect(evaluate('={{Boolean(1)}}')).toEqual(Boolean(1)); - expect(evaluate('={{Symbol(1).toString()}}')).toEqual(Symbol(1).toString()); - }); + it('should be able to use global built-ins from allowlist', () => { + expect(evaluate('={{new Date()}}')).toBeInstanceOf(Date); + expect(evaluate('={{DateTime.now().toLocaleString()}}')).toEqual( + DateTime.now().toLocaleString(), + ); + expect(evaluate('={{Interval.after(new Date(), 100)}}')).toEqual( + Interval.after(new Date(), 100), + ); + expect(evaluate('={{Duration.fromMillis(100)}}')).toEqual(Duration.fromMillis(100)); + + expect(evaluate('={{new Object()}}')).toEqual(new Object()); + + expect(evaluate('={{new Array()}}')).toEqual([]); + expect(evaluate('={{new Int8Array()}}')).toEqual(new Int8Array()); + expect(evaluate('={{new Uint8Array()}}')).toEqual(new Uint8Array()); + expect(evaluate('={{new Uint8ClampedArray()}}')).toEqual(new Uint8ClampedArray()); + expect(evaluate('={{new Int16Array()}}')).toEqual(new Int16Array()); + expect(evaluate('={{new Uint16Array()}}')).toEqual(new Uint16Array()); + expect(evaluate('={{new Int32Array()}}')).toEqual(new Int32Array()); + expect(evaluate('={{new Uint32Array()}}')).toEqual(new Uint32Array()); + expect(evaluate('={{new Float32Array()}}')).toEqual(new Float32Array()); + expect(evaluate('={{new Float64Array()}}')).toEqual(new Float64Array()); + expect(evaluate('={{new BigInt64Array()}}')).toEqual(new BigInt64Array()); + expect(evaluate('={{new BigUint64Array()}}')).toEqual(new BigUint64Array()); + + expect(evaluate('={{new Map()}}')).toEqual(new Map()); + expect(evaluate('={{new WeakMap()}}')).toEqual(new WeakMap()); + expect(evaluate('={{new Set()}}')).toEqual(new Set()); + expect(evaluate('={{new WeakSet()}}')).toEqual(new WeakSet()); + + expect(evaluate('={{new Error()}}')).toEqual(new Error()); + expect(evaluate('={{new TypeError()}}')).toEqual(new TypeError()); + expect(evaluate('={{new SyntaxError()}}')).toEqual(new SyntaxError()); + expect(evaluate('={{new EvalError()}}')).toEqual(new EvalError()); + expect(evaluate('={{new RangeError()}}')).toEqual(new RangeError()); + expect(evaluate('={{new ReferenceError()}}')).toEqual(new ReferenceError()); + expect(evaluate('={{new URIError()}}')).toEqual(new URIError()); + + expect(evaluate('={{Intl}}')).toEqual(Intl); + + expect(evaluate('={{new String()}}')).toEqual(new String()); + expect(evaluate("={{new RegExp('')}}")).toEqual(new RegExp('')); + + expect(evaluate('={{Math}}')).toEqual(Math); + expect(evaluate('={{new Number()}}')).toEqual(new Number()); + expect(evaluate("={{BigInt('1')}}")).toEqual(BigInt('1')); + expect(evaluate('={{Infinity}}')).toEqual(Infinity); + expect(evaluate('={{NaN}}')).toEqual(NaN); + expect(evaluate('={{isFinite(1)}}')).toEqual(isFinite(1)); + expect(evaluate('={{isNaN(1)}}')).toEqual(isNaN(1)); + expect(evaluate("={{parseFloat('1')}}")).toEqual(parseFloat('1')); + expect(evaluate("={{parseInt('1', 10)}}")).toEqual(parseInt('1', 10)); + + expect(evaluate('={{JSON.stringify({})}}')).toEqual(JSON.stringify({})); + expect(evaluate('={{new ArrayBuffer(10)}}')).toEqual(new ArrayBuffer(10)); + expect(evaluate('={{new SharedArrayBuffer(10)}}')).toEqual(new SharedArrayBuffer(10)); + expect(evaluate('={{Atomics}}')).toEqual(Atomics); + expect(evaluate('={{new DataView(new ArrayBuffer(1))}}')).toEqual( + new DataView(new ArrayBuffer(1)), + ); + + expect(evaluate("={{encodeURI('https://google.com')}}")).toEqual( + encodeURI('https://google.com'), + ); + expect(evaluate("={{encodeURIComponent('https://google.com')}}")).toEqual( + encodeURIComponent('https://google.com'), + ); + expect(evaluate("={{decodeURI('https://google.com')}}")).toEqual( + decodeURI('https://google.com'), + ); + expect(evaluate("={{decodeURIComponent('https://google.com')}}")).toEqual( + decodeURIComponent('https://google.com'), + ); + + expect(evaluate('={{Boolean(1)}}')).toEqual(Boolean(1)); + expect(evaluate('={{Symbol(1).toString()}}')).toEqual(Symbol(1).toString()); + }); - it('should not able to do arbitrary code execution', () => { - const testFn = jest.fn(); - Object.assign(global, { testFn }); - evaluate("={{ Date['constructor']('testFn()')()}}"); - expect(testFn).not.toHaveBeenCalled(); + it('should not able to do arbitrary code execution', () => { + const testFn = jest.fn(); + Object.assign(global, { testFn }); + evaluate("={{ Date['constructor']('testFn()')()}}"); + expect(testFn).not.toHaveBeenCalled(); + }); }); - }); - describe('Test all expression value fixtures', () => { - const nodeTypes = Helpers.NodeTypes(); - const workflow = new Workflow({ - nodes: [ - { - name: 'node', - typeVersion: 1, - type: 'test.set', - id: 'uuid-1234', - position: [0, 0], - parameters: {}, - }, - ], - connections: {}, - active: false, - nodeTypes, - }); + describe('Test all expression value fixtures', () => { + const nodeTypes = Helpers.NodeTypes(); + const workflow = new Workflow({ + nodes: [ + { + name: 'node', + typeVersion: 1, + type: 'test.set', + id: 'uuid-1234', + position: [0, 0], + parameters: {}, + }, + ], + connections: {}, + active: false, + nodeTypes, + }); - const expression = new Expression(workflow); + const expression = new Expression(workflow); - const evaluate = (value: string, data: INodeExecutionData[]) => { - return expression.getParameterValue(value, null, 0, 0, 'node', data, 'manual', '', {}); - }; + const evaluate = (value: string, data: INodeExecutionData[]) => { + return expression.getParameterValue(value, null, 0, 0, 'node', data, 'manual', '', {}); + }; - for (const t of baseFixtures) { - if (!t.tests.some((test) => test.type === 'evaluation')) { - continue; - } - test(t.expression, () => { - for (const test of t.tests.filter( - (test) => test.type === 'evaluation', - ) as ExpressionTestEvaluation[]) { - expect(evaluate(t.expression, test.input.map((d) => ({ json: d })) as any)).toStrictEqual( - test.output, - ); + for (const t of baseFixtures) { + if (!t.tests.some((test) => test.type === 'evaluation')) { + continue; } - }); - } - }); - - describe('Test all expression transform fixtures', () => { - for (const t of baseFixtures) { - if (!t.tests.some((test) => test.type === 'transform')) { - continue; + test(t.expression, () => { + for (const test of t.tests.filter( + (test) => test.type === 'evaluation', + ) as ExpressionTestEvaluation[]) { + expect( + evaluate(t.expression, test.input.map((d) => ({ json: d })) as any), + ).toStrictEqual(test.output); + } + }); } - test(t.expression, () => { - for (const test of t.tests.filter( - (test) => test.type === 'transform', - ) as ExpressionTestTransform[]) { - const expr = t.expression; - expect(extendSyntax(expr, test.forceTransform)).toEqual(test.result ?? expr); + }); + + describe('Test all expression transform fixtures', () => { + for (const t of baseFixtures) { + if (!t.tests.some((test) => test.type === 'transform')) { + continue; } - }); - } + test(t.expression, () => { + for (const test of t.tests.filter( + (test) => test.type === 'transform', + ) as ExpressionTestTransform[]) { + const expr = t.expression; + expect(extendSyntax(expr, test.forceTransform)).toEqual(test.result ?? expr); + } + }); + } + }); }); -}); +} From 5d322a00842b1a44d69dab44fe0de8eaba3ffbd6 Mon Sep 17 00:00:00 2001 From: Valya Bullions Date: Thu, 20 Jul 2023 16:26:25 +0100 Subject: [PATCH 02/14] feat: add expression difference reporter --- packages/cli/src/ExpressionEvalator.ts | 10 +- packages/cli/src/config/schema.ts | 6 ++ .../workflow/src/ExpressionEvaluatorProxy.ts | 100 ++++++++++++++++-- 3 files changed, 108 insertions(+), 8 deletions(-) diff --git a/packages/cli/src/ExpressionEvalator.ts b/packages/cli/src/ExpressionEvalator.ts index 4f1f5149a2d7b..1b53f845dc1c5 100644 --- a/packages/cli/src/ExpressionEvalator.ts +++ b/packages/cli/src/ExpressionEvalator.ts @@ -1,6 +1,14 @@ import config from '@/config'; -import { ExpressionEvaluatorProxy } from 'n8n-workflow'; +import { ErrorReporterProxy, ExpressionEvaluatorProxy } from 'n8n-workflow'; export const initExpressionEvaluator = () => { ExpressionEvaluatorProxy.setEvaluator(config.getEnv('expression.evaluator')); + ExpressionEvaluatorProxy.setDifferEnabled(config.getEnv('expression.reportDifference')); + ExpressionEvaluatorProxy.setDiffReporter((expr) => { + ErrorReporterProxy.warn('Expression difference', { + extra: { + expression: expr, + }, + }); + }); }; diff --git a/packages/cli/src/config/schema.ts b/packages/cli/src/config/schema.ts index c324cbe54db27..74473a07eb9c3 100644 --- a/packages/cli/src/config/schema.ts +++ b/packages/cli/src/config/schema.ts @@ -1103,5 +1103,11 @@ export const schema = { default: 'tmpl', env: 'N8N_EXPRESSION_EVALUATOR', }, + reportDifference: { + doc: 'Expression evaluator to use', + format: Boolean, + default: false, + env: 'N8N_EXPRESSION_REPORT_DIFFERENCE', + }, }, }; diff --git a/packages/workflow/src/ExpressionEvaluatorProxy.ts b/packages/workflow/src/ExpressionEvaluatorProxy.ts index 30ca62190ac0c..fa49f703709cd 100644 --- a/packages/workflow/src/ExpressionEvaluatorProxy.ts +++ b/packages/workflow/src/ExpressionEvaluatorProxy.ts @@ -1,7 +1,8 @@ import * as tmpl from '@n8n_io/riot-tmpl'; -import type { TmplDifference } from '@n8n/tournament'; +import type { ReturnValue, TmplDifference } from '@n8n/tournament'; import { Tournament } from '@n8n/tournament'; import type { ExpressionEvaluatorType } from './Interfaces'; +import * as ErrorReporterProxy from './ErrorReporterProxy'; type Evaluator = (expr: string, data: unknown) => tmpl.ReturnValue; type ErrorHandler = (error: Error) => void; @@ -19,10 +20,16 @@ const differenceChecker = (diff: TmplDifference) => { if (diff.has?.function || diff.has?.templateString) { return; } - differenceHandler(diff.expression as string); + if (diff.expression === 'UNPARSEABLE') { + differenceHandler(diff.expression); + } else { + differenceHandler(diff.expression.value); + } }; -const tournamentEvaluator = new Tournament(errorHandler, undefined, differenceChecker); +const tournamentEvaluator = new Tournament(errorHandler, undefined); let evaluator: Evaluator = tmpl.tmpl; +let currentEvaluatorType: ExpressionEvaluatorType = 'tmpl'; +let diffExpressions = false; export const setErrorHandler = (handler: ErrorHandler) => { errorHandler = handler; @@ -31,7 +38,7 @@ export const setErrorHandler = (handler: ErrorHandler) => { }; export const setEvaluator = (evalType: ExpressionEvaluatorType) => { - console.log('setEvaluator', evalType); + currentEvaluatorType = evalType; if (evalType === 'tmpl') { evaluator = tmpl.tmpl; } else if (evalType === 'tournament') { @@ -43,12 +50,91 @@ export const setDiffReporter = (reporter: (expr: string) => void) => { differenceHandler = reporter; }; -export const checkEvaluatorDifferences = (expr: string) => { - tournamentEvaluator.tmplDiff(expr); +export const setDifferEnabled = (enabled: boolean) => { + diffExpressions = enabled; +}; + +const diffCache: Record = {}; + +export const checkEvaluatorDifferences = (expr: string): TmplDifference | null => { + if (expr in diffCache) { + return diffCache[expr]; + } + let diff: TmplDifference | null; + try { + diff = tournamentEvaluator.tmplDiff(expr); + } catch (e) { + // We don't include the expression for privacy reasons + differenceHandler('ERROR'); + diff = null; + } + + if (diff?.same === false) { + differenceChecker(diff); + } + + diffCache[expr] = diff; + return diff; }; export const getEvaluator = () => { return evaluator; }; -export const evaluateExpression: Evaluator = (expr, data) => evaluator(expr, data); +export const evaluateExpression: Evaluator = (expr, data) => { + if (!diffExpressions) { + return evaluator(expr, data); + } + const diff = checkEvaluatorDifferences(expr); + + // We already know that they're different so don't bother + // evaluating with both evaluators + if (!diff?.same) { + return evaluator(expr, data); + } + + let tmplValue: tmpl.ReturnValue; + let tournValue: ReturnValue; + let wasTmplError = false; + let tmplError: unknown; + let wasTournError = false; + let tournError: unknown; + + try { + tmplValue = tmpl.tmpl(expr, data); + } catch (error) { + tmplError = error; + wasTmplError = true; + } + + try { + tournValue = tournamentEvaluator.execute(expr, data); + } catch (error) { + tournError = error; + wasTournError = true; + } + + if ( + wasTmplError !== wasTournError || + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + JSON.stringify(tmplValue!) !== JSON.stringify(tournValue!) + ) { + if (diff.expression) { + differenceHandler(diff.expression.value); + } + } + + if (currentEvaluatorType === 'tmpl') { + if (wasTmplError) { + throw tmplError; + } + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + return tmplValue!; + } + + if (wasTournError) { + throw tournError; + } + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + return tournValue!; +}; From 37b2e0f2bd39ccc0acecaca7eaf5be6a4ce42f4f Mon Sep 17 00:00:00 2001 From: Valya Bullions Date: Thu, 17 Aug 2023 16:25:20 +0100 Subject: [PATCH 03/14] add more error handling to expression evaluator proxy --- .../workflow/src/ExpressionEvaluatorProxy.ts | 43 +++++++++++-------- 1 file changed, 26 insertions(+), 17 deletions(-) diff --git a/packages/workflow/src/ExpressionEvaluatorProxy.ts b/packages/workflow/src/ExpressionEvaluatorProxy.ts index fa49f703709cd..a180df3c0f835 100644 --- a/packages/workflow/src/ExpressionEvaluatorProxy.ts +++ b/packages/workflow/src/ExpressionEvaluatorProxy.ts @@ -2,7 +2,7 @@ import * as tmpl from '@n8n_io/riot-tmpl'; import type { ReturnValue, TmplDifference } from '@n8n/tournament'; import { Tournament } from '@n8n/tournament'; import type { ExpressionEvaluatorType } from './Interfaces'; -import * as ErrorReporterProxy from './ErrorReporterProxy'; +import * as LoggerProxy from './LoggerProxy'; type Evaluator = (expr: string, data: unknown) => tmpl.ReturnValue; type ErrorHandler = (error: Error) => void; @@ -14,16 +14,20 @@ tmpl.brackets.set('{{ }}'); let errorHandler: ErrorHandler = () => {}; let differenceHandler: DifferenceHandler = () => {}; const differenceChecker = (diff: TmplDifference) => { - if (diff.same) { - return; - } - if (diff.has?.function || diff.has?.templateString) { - return; - } - if (diff.expression === 'UNPARSEABLE') { - differenceHandler(diff.expression); - } else { - differenceHandler(diff.expression.value); + try { + if (diff.same) { + return; + } + if (diff.has?.function || diff.has?.templateString) { + return; + } + if (diff.expression === 'UNPARSEABLE') { + differenceHandler(diff.expression); + } else { + differenceHandler(diff.expression.value); + } + } catch { + LoggerProxy.error('Expression evaluator difference checker failed'); } }; const tournamentEvaluator = new Tournament(errorHandler, undefined); @@ -65,7 +69,9 @@ export const checkEvaluatorDifferences = (expr: string): TmplDifference | null = diff = tournamentEvaluator.tmplDiff(expr); } catch (e) { // We don't include the expression for privacy reasons - differenceHandler('ERROR'); + try { + differenceHandler('ERROR'); + } catch {} diff = null; } @@ -116,11 +122,16 @@ export const evaluateExpression: Evaluator = (expr, data) => { if ( wasTmplError !== wasTournError || - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion JSON.stringify(tmplValue!) !== JSON.stringify(tournValue!) ) { - if (diff.expression) { - differenceHandler(diff.expression.value); + try { + if (diff.expression) { + differenceHandler(diff.expression.value); + } else { + differenceHandler('VALUEDIFF'); + } + } catch { + LoggerProxy.error('Failed to report error difference'); } } @@ -128,13 +139,11 @@ export const evaluateExpression: Evaluator = (expr, data) => { if (wasTmplError) { throw tmplError; } - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion return tmplValue!; } if (wasTournError) { throw tournError; } - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion return tournValue!; }; From 805cce0cf8cd9e8cbb9bbd76a442ea1a649228af Mon Sep 17 00:00:00 2001 From: Valya Bullions Date: Thu, 17 Aug 2023 16:25:35 +0100 Subject: [PATCH 04/14] add @n8n/tournament package to package.json --- packages/workflow/package.json | 1 + pnpm-lock.yaml | 29 +++++++++++++++++++++++------ 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/packages/workflow/package.json b/packages/workflow/package.json index 0b5da9ce35d94..651c754bcc2ab 100644 --- a/packages/workflow/package.json +++ b/packages/workflow/package.json @@ -48,6 +48,7 @@ "@types/xml2js": "^0.4.11" }, "dependencies": { + "@n8n/tournament": "^1.0.0", "@n8n_io/riot-tmpl": "^4.0.0", "ast-types": "0.15.2", "crypto-js": "^4.1.1", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 3211d6c7a071d..d0fb6cff020c2 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1271,6 +1271,9 @@ importers: packages/workflow: dependencies: + '@n8n/tournament': + specifier: ^1.0.0 + version: 1.0.0 '@n8n_io/riot-tmpl': specifier: ^4.0.0 version: 4.0.0 @@ -4135,6 +4138,16 @@ packages: - '@lezer/common' dev: false + /@n8n/tournament@1.0.0: + resolution: {integrity: sha512-jzMQfl8FAS8GHGTXKaZRCRk1dQbtjAODL17BKBojjCtGgBLHCNQlGhmU2p7ypgvWUeR19NrKW0YqD23iq8eYaQ==} + engines: {node: '>=18.10', pnpm: '>=8.6'} + dependencies: + '@n8n_io/riot-tmpl': 4.0.1 + ast-types: 0.16.1 + esprima-next: 5.8.4 + recast: 0.22.0 + dev: false + /@n8n_io/license-sdk@2.4.0: resolution: {integrity: sha512-99kuCVH4NcBi4nyn/WIpd6KSIMLk/pbBks0zr8bC65ALKj0se7/2MwC6N+WwGkG7NqH0kMdGe/7Y5KnJkMTefg==} engines: {node: '>=14.0.0', npm: '>=7.10.0'} @@ -4151,6 +4164,12 @@ packages: eslint-config-riot: 1.0.0 dev: false + /@n8n_io/riot-tmpl@4.0.1: + resolution: {integrity: sha512-/zdRbEfTFjsm1NqnpPQHgZTkTdbp5v3VUxGeMA9098sps8jRCTraQkc3AQstJgHUm7ylBXJcIVhnVeLUMWAfwQ==} + dependencies: + eslint-config-riot: 1.0.0 + dev: false + /@ndelangen/get-tarball@3.0.7: resolution: {integrity: sha512-NqGfTZIZpRFef1GoVaShSSRwDC3vde3ThtTeqFdcYd6ipKqnfEVhjK2hUeHjCQUcptyZr2TONqcloFXM+5QBrQ==} dependencies: @@ -7980,7 +7999,6 @@ packages: is-nan: 1.3.2 object-is: 1.1.5 util: 0.12.5 - dev: true /assertion-error@1.1.0: resolution: {integrity: sha512-jgsaNduz+ndvGyFt3uSuWqvy4lCnIJiovtouQN5JZHOKCS2QuhEdbcQHFhVksz2N2U9hXJo8odG7ETyWlEeuDw==} @@ -8016,7 +8034,6 @@ packages: engines: {node: '>=4'} dependencies: tslib: 2.6.1 - dev: true /astral-regex@2.0.0: resolution: {integrity: sha512-Z7tMw1ytTXt5jqMcOP+OQteU1VuNK9Y02uuJtKQ1Sv69jXQKKg5cibLwGJow8yzZP+eAc18EmLGPal0bp36rvQ==} @@ -8149,7 +8166,7 @@ packages: /axios@0.21.4: resolution: {integrity: sha512-ut5vewkiu8jjGBdqpM44XxjuCjq9LAKeHVmoVfHVzy8eHgxxq8SbAVQNovDA8mVi05kP0Ea/n/UzcSHcTJQfNg==} dependencies: - follow-redirects: 1.15.2(debug@4.3.4) + follow-redirects: 1.15.2(debug@3.2.7) transitivePeerDependencies: - debug dev: false @@ -8178,6 +8195,7 @@ packages: form-data: 4.0.0 transitivePeerDependencies: - debug + dev: true /babel-core@7.0.0-bridge.0(@babel/core@7.22.9): resolution: {integrity: sha512-poPX9mZH/5CSanm50Q+1toVci6pv5KSRv/5TWCwtzQS5XEwn40BcCrgIeMFWP9CKKIniKXNxoIOnOq4VVlGXhg==} @@ -10583,7 +10601,6 @@ packages: /es6-object-assign@1.1.0: resolution: {integrity: sha512-MEl9uirslVwqQU369iHNWZXsI8yaZYGg/D65aOgZkeyFJwHYSxilf7rQzXKI7DdDuBPrBXbfk3sl9hJhmd5AUw==} - dev: true /es6-symbol@3.1.3: resolution: {integrity: sha512-NJ6Yn3FuDinBaBRWl/q5X/s4koRHBrgKAu+yGI6JCBeiu3qrcbJhwT2GeR/EXVfylRk8dpQVJoLEFhK+Mu31NA==} @@ -11758,6 +11775,7 @@ packages: optional: true dependencies: debug: 4.3.4(supports-color@8.1.1) + dev: true /for-each@0.3.3: resolution: {integrity: sha512-jqYfLp7mo9vIyQf8ykW2v7A+2N4QjeCeI5+Dz9XraiO1ign81wjiH7Fb9vSOWvQfNtmSa4H2RoQTrrXivdUZmw==} @@ -17117,7 +17135,7 @@ packages: resolution: {integrity: sha512-aXYe/D+28kF63W8Cz53t09ypEORz+ULeDCahdAqhVrRm2scbOXFbtnn0GGhvMpYe45grepLKuwui9KxrZ2ZuMw==} engines: {node: '>=14.17.0'} dependencies: - axios: 0.27.2(debug@4.3.4) + axios: 0.27.2(debug@3.2.7) transitivePeerDependencies: - debug dev: false @@ -17781,7 +17799,6 @@ packages: esprima: 4.0.1 source-map: 0.6.1 tslib: 2.6.1 - dev: true /recast@0.23.3: resolution: {integrity: sha512-HbCVFh2ANP6a09nzD4lx7XthsxMOJWKX5pIcUwtLrmeEIl3I0DwjCoVXDE0Aobk+7k/mS3H50FK4iuYArpcT6Q==} From 4e6084bda92acb9837ee1ae82e8666aef839ed0a Mon Sep 17 00:00:00 2001 From: Valya Bullions Date: Thu, 17 Aug 2023 16:25:49 +0100 Subject: [PATCH 05/14] linting fixes --- packages/editor-ui/src/mixins/workflowHelpers.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/editor-ui/src/mixins/workflowHelpers.ts b/packages/editor-ui/src/mixins/workflowHelpers.ts index 567033b6747a4..daf5bccc7b688 100644 --- a/packages/editor-ui/src/mixins/workflowHelpers.ts +++ b/packages/editor-ui/src/mixins/workflowHelpers.ts @@ -9,7 +9,7 @@ import { MODAL_CONFIRM, } from '@/constants'; -import { +import type { IConnections, IDataObject, INode, @@ -27,8 +27,8 @@ import { IExecuteData, INodeConnection, IWebhookDescription, - ExpressionEvaluatorProxy, } from 'n8n-workflow'; +import { ExpressionEvaluatorProxy } from 'n8n-workflow'; import { NodeHelpers } from 'n8n-workflow'; import type { From fecc276151226252292ae56aec1a1ece0d779480 Mon Sep 17 00:00:00 2001 From: Omar Ajoue Date: Mon, 28 Aug 2023 15:19:01 +0200 Subject: [PATCH 06/14] fix: Import type --- packages/workflow/src/Expression.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/workflow/src/Expression.ts b/packages/workflow/src/Expression.ts index ce05b084e1b6f..27f42e0b10964 100644 --- a/packages/workflow/src/Expression.ts +++ b/packages/workflow/src/Expression.ts @@ -1,5 +1,5 @@ import { DateTime, Duration, Interval } from 'luxon'; -import type * as tmpl from '@n8n_io/riot-tmpl'; +import * as tmpl from '@n8n_io/riot-tmpl'; import type { IDataObject, From 02fadf972095b6a759583f4795ee145771c01e96 Mon Sep 17 00:00:00 2001 From: Val <68596159+valya@users.noreply.github.com> Date: Tue, 5 Sep 2023 11:11:06 +0100 Subject: [PATCH 07/14] chore: Update tournament --- packages/workflow/package.json | 2 +- pnpm-lock.yaml | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/workflow/package.json b/packages/workflow/package.json index 3dbbabde46a67..f381f47f96366 100644 --- a/packages/workflow/package.json +++ b/packages/workflow/package.json @@ -48,7 +48,7 @@ "@types/xml2js": "^0.4.11" }, "dependencies": { - "@n8n/tournament": "^1.0.0", + "@n8n/tournament": "^1.0.2", "@n8n_io/riot-tmpl": "^4.0.0", "ast-types": "0.15.2", "crypto-js": "^4.1.1", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 4dde7d5b4ead9..9f828e6406a2b 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1287,8 +1287,8 @@ importers: packages/workflow: dependencies: '@n8n/tournament': - specifier: ^1.0.0 - version: 1.0.0 + specifier: ^1.0.2 + version: 1.0.2 '@n8n_io/riot-tmpl': specifier: ^4.0.0 version: 4.0.0 @@ -4647,8 +4647,8 @@ packages: - '@lezer/common' dev: false - /@n8n/tournament@1.0.0: - resolution: {integrity: sha512-jzMQfl8FAS8GHGTXKaZRCRk1dQbtjAODL17BKBojjCtGgBLHCNQlGhmU2p7ypgvWUeR19NrKW0YqD23iq8eYaQ==} + /@n8n/tournament@1.0.2: + resolution: {integrity: sha512-fTpi7F8ra5flGSVfRzohPyG7czAAKCZPlLjdKdwbLJivLoI/Ekhgodov1jfVSCVFVbwQ06gRQRxLEDzl2jl8ig==} engines: {node: '>=18.10', pnpm: '>=8.6'} dependencies: '@n8n_io/riot-tmpl': 4.0.1 From 9b7e3a3eb2ab481c834ff3d259814fe5c8364d56 Mon Sep 17 00:00:00 2001 From: Val <68596159+valya@users.noreply.github.com> Date: Thu, 7 Sep 2023 16:08:14 +0100 Subject: [PATCH 08/14] update lock file --- pnpm-lock.yaml | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index efcfc7c8cfd50..9f3d91bcb7311 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1292,6 +1292,9 @@ importers: packages/workflow: dependencies: + '@n8n/tournament': + specifier: ^1.0.2 + version: 1.0.2 '@n8n_io/riot-tmpl': specifier: ^4.0.0 version: 4.0.0 @@ -4650,6 +4653,16 @@ packages: - '@lezer/common' dev: false + /@n8n/tournament@1.0.2: + resolution: {integrity: sha512-fTpi7F8ra5flGSVfRzohPyG7czAAKCZPlLjdKdwbLJivLoI/Ekhgodov1jfVSCVFVbwQ06gRQRxLEDzl2jl8ig==} + engines: {node: '>=18.10', pnpm: '>=8.6'} + dependencies: + '@n8n_io/riot-tmpl': 4.0.1 + ast-types: 0.16.1 + esprima-next: 5.8.4 + recast: 0.22.0 + dev: false + /@n8n/vm2@3.9.20: resolution: {integrity: sha512-qk2oJYkuFRVSTxoro4obX/sv/wT1pViZjHh/isjOvFB93D52QIg3TCjMPsHOfHTmkxCKJffjLrUvjIwvWzSMCQ==} engines: {node: '>=18.10', pnpm: '>=8.6.12'} @@ -4675,6 +4688,12 @@ packages: eslint-config-riot: 1.0.0 dev: false + /@n8n_io/riot-tmpl@4.0.1: + resolution: {integrity: sha512-/zdRbEfTFjsm1NqnpPQHgZTkTdbp5v3VUxGeMA9098sps8jRCTraQkc3AQstJgHUm7ylBXJcIVhnVeLUMWAfwQ==} + dependencies: + eslint-config-riot: 1.0.0 + dev: false + /@ndelangen/get-tarball@3.0.7: resolution: {integrity: sha512-NqGfTZIZpRFef1GoVaShSSRwDC3vde3ThtTeqFdcYd6ipKqnfEVhjK2hUeHjCQUcptyZr2TONqcloFXM+5QBrQ==} dependencies: @@ -8913,7 +8932,6 @@ packages: is-nan: 1.3.2 object-is: 1.1.5 util: 0.12.5 - dev: true /assertion-error@1.1.0: resolution: {integrity: sha512-jgsaNduz+ndvGyFt3uSuWqvy4lCnIJiovtouQN5JZHOKCS2QuhEdbcQHFhVksz2N2U9hXJo8odG7ETyWlEeuDw==} @@ -8942,7 +8960,6 @@ packages: engines: {node: '>=4'} dependencies: tslib: 2.6.1 - dev: true /astral-regex@2.0.0: resolution: {integrity: sha512-Z7tMw1ytTXt5jqMcOP+OQteU1VuNK9Y02uuJtKQ1Sv69jXQKKg5cibLwGJow8yzZP+eAc18EmLGPal0bp36rvQ==} @@ -11506,7 +11523,6 @@ packages: /es6-object-assign@1.1.0: resolution: {integrity: sha512-MEl9uirslVwqQU369iHNWZXsI8yaZYGg/D65aOgZkeyFJwHYSxilf7rQzXKI7DdDuBPrBXbfk3sl9hJhmd5AUw==} - dev: true /es6-symbol@3.1.3: resolution: {integrity: sha512-NJ6Yn3FuDinBaBRWl/q5X/s4koRHBrgKAu+yGI6JCBeiu3qrcbJhwT2GeR/EXVfylRk8dpQVJoLEFhK+Mu31NA==} @@ -18669,7 +18685,6 @@ packages: esprima: 4.0.1 source-map: 0.6.1 tslib: 2.6.1 - dev: true /recast@0.23.3: resolution: {integrity: sha512-HbCVFh2ANP6a09nzD4lx7XthsxMOJWKX5pIcUwtLrmeEIl3I0DwjCoVXDE0Aobk+7k/mS3H50FK4iuYArpcT6Q==} From f1209b1517e530589627f07fef2bf6aa466d647b Mon Sep 17 00:00:00 2001 From: Val <68596159+valya@users.noreply.github.com> Date: Thu, 14 Sep 2023 14:14:46 +0100 Subject: [PATCH 09/14] fix: Expression evaluator not being set for node subtitles --- packages/editor-ui/src/mixins/nodeHelpers.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/editor-ui/src/mixins/nodeHelpers.ts b/packages/editor-ui/src/mixins/nodeHelpers.ts index ad517e4ad47f3..ce53b03c313e0 100644 --- a/packages/editor-ui/src/mixins/nodeHelpers.ts +++ b/packages/editor-ui/src/mixins/nodeHelpers.ts @@ -19,7 +19,7 @@ import type { INodePropertyOptions, IDataObject, } from 'n8n-workflow'; -import { NodeHelpers } from 'n8n-workflow'; +import { NodeHelpers, ExpressionEvaluatorProxy } from 'n8n-workflow'; import type { ICredentialsResponse, @@ -531,6 +531,9 @@ export const nodeHelpers = defineComponent({ if (nodeType !== null && nodeType.subtitle !== undefined) { try { + ExpressionEvaluatorProxy.setEvaluator( + useSettingsStore().settings.expressions?.evaluator ?? 'tmpl', + ); return workflow.expression.getSimpleParameterValue( data as INode, nodeType.subtitle, From b3340add8c839cd92ecb79359e18bb5e5e4b657c Mon Sep 17 00:00:00 2001 From: Val <68596159+valya@users.noreply.github.com> Date: Tue, 19 Sep 2023 13:15:56 +0100 Subject: [PATCH 10/14] feat: make tournament the default expression evaluator --- packages/cli/src/config/schema.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli/src/config/schema.ts b/packages/cli/src/config/schema.ts index f58ee09534f13..8e997a9620af9 100644 --- a/packages/cli/src/config/schema.ts +++ b/packages/cli/src/config/schema.ts @@ -1209,7 +1209,7 @@ export const schema = { evaluator: { doc: 'Expression evaluator to use', format: ['tmpl', 'tournament'] as const, - default: 'tmpl', + default: 'tournament', env: 'N8N_EXPRESSION_EVALUATOR', }, reportDifference: { From 61a8d84c05e4e2b21e88f822e5da21f36aa63720 Mon Sep 17 00:00:00 2001 From: Val <68596159+valya@users.noreply.github.com> Date: Tue, 19 Sep 2023 13:28:14 +0100 Subject: [PATCH 11/14] remove variable from bad merge --- packages/cli/src/commands/BaseCommand.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/cli/src/commands/BaseCommand.ts b/packages/cli/src/commands/BaseCommand.ts index 0f783e6e38b10..dada17ed70c3e 100644 --- a/packages/cli/src/commands/BaseCommand.ts +++ b/packages/cli/src/commands/BaseCommand.ts @@ -23,9 +23,6 @@ import { License } from '@/License'; import { ExternalSecretsManager } from '@/ExternalSecrets/ExternalSecretsManager.ee'; import { initExpressionEvaluator } from '@/ExpressionEvalator'; -export const UM_FIX_INSTRUCTION = - 'Please fix the database by running ./packages/cli/bin/n8n user-management:reset'; - export abstract class BaseCommand extends Command { protected logger = LoggerProxy.init(getLogger()); From 3822e90ab22a6d13cd5ff122b61d45a89ba8470b Mon Sep 17 00:00:00 2001 From: Val <68596159+valya@users.noreply.github.com> Date: Tue, 19 Sep 2023 13:30:41 +0100 Subject: [PATCH 12/14] update frontend default expression evaluator to be inline with backend --- packages/editor-ui/src/mixins/nodeHelpers.ts | 2 +- packages/editor-ui/src/mixins/workflowHelpers.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/editor-ui/src/mixins/nodeHelpers.ts b/packages/editor-ui/src/mixins/nodeHelpers.ts index ce53b03c313e0..232e0c17cf503 100644 --- a/packages/editor-ui/src/mixins/nodeHelpers.ts +++ b/packages/editor-ui/src/mixins/nodeHelpers.ts @@ -532,7 +532,7 @@ export const nodeHelpers = defineComponent({ if (nodeType !== null && nodeType.subtitle !== undefined) { try { ExpressionEvaluatorProxy.setEvaluator( - useSettingsStore().settings.expressions?.evaluator ?? 'tmpl', + useSettingsStore().settings.expressions?.evaluator ?? 'tournament', ); return workflow.expression.getSimpleParameterValue( data as INode, diff --git a/packages/editor-ui/src/mixins/workflowHelpers.ts b/packages/editor-ui/src/mixins/workflowHelpers.ts index c7b15927672a9..8e175f2201bcb 100644 --- a/packages/editor-ui/src/mixins/workflowHelpers.ts +++ b/packages/editor-ui/src/mixins/workflowHelpers.ts @@ -165,7 +165,7 @@ export function resolveParameter( const _executeData = executeData(parentNode, activeNode!.name, inputName, runIndexCurrent); ExpressionEvaluatorProxy.setEvaluator( - useSettingsStore().settings.expressions?.evaluator ?? 'tmpl', + useSettingsStore().settings.expressions?.evaluator ?? 'tournament', ); return workflow.expression.getParameterValue( From 17c0030387d9dc5a34a2e470e90e1bb7e68a4d03 Mon Sep 17 00:00:00 2001 From: Val <68596159+valya@users.noreply.github.com> Date: Wed, 20 Sep 2023 13:15:58 +0100 Subject: [PATCH 13/14] review changes --- packages/cli/src/Server.ts | 2 +- packages/cli/src/config/schema.ts | 2 +- packages/editor-ui/src/App.vue | 2 ++ packages/editor-ui/src/mixins/nodeHelpers.ts | 5 +---- packages/editor-ui/src/mixins/workflowHelpers.ts | 5 ----- packages/workflow/src/Expression.ts | 1 + packages/workflow/src/ExpressionEvaluatorProxy.ts | 2 +- 7 files changed, 7 insertions(+), 12 deletions(-) diff --git a/packages/cli/src/Server.ts b/packages/cli/src/Server.ts index a145252e3b879..1d3d226ec45ac 100644 --- a/packages/cli/src/Server.ts +++ b/packages/cli/src/Server.ts @@ -337,7 +337,7 @@ export class Server extends AbstractServer { limit: 0, }, expressions: { - evaluator: config.get('expression.evaluator') as 'tmpl' | 'tournament', + evaluator: config.getEnv('expression.evaluator'), }, banners: { dismissed: [], diff --git a/packages/cli/src/config/schema.ts b/packages/cli/src/config/schema.ts index 8e997a9620af9..3dd4b7952bc84 100644 --- a/packages/cli/src/config/schema.ts +++ b/packages/cli/src/config/schema.ts @@ -1213,7 +1213,7 @@ export const schema = { env: 'N8N_EXPRESSION_EVALUATOR', }, reportDifference: { - doc: 'Expression evaluator to use', + doc: 'Whether to report differences in the evaluator outputs', format: Boolean, default: false, env: 'N8N_EXPRESSION_REPORT_DIFFERENCE', diff --git a/packages/editor-ui/src/App.vue b/packages/editor-ui/src/App.vue index 503f0a1a0a9e7..b262123be568e 100644 --- a/packages/editor-ui/src/App.vue +++ b/packages/editor-ui/src/App.vue @@ -59,6 +59,7 @@ import { useHistoryHelper } from '@/composables/useHistoryHelper'; import { newVersions } from '@/mixins/newVersions'; import { useRoute } from 'vue-router'; import { useExternalHooks } from '@/composables'; +import { ExpressionEvaluatorProxy } from 'n8n-workflow'; export default defineComponent({ name: 'App', @@ -151,6 +152,7 @@ export default defineComponent({ }, async initialize(): Promise { await this.initSettings(); + ExpressionEvaluatorProxy.setEvaluator(useSettingsStore().settings.expressions.evaluator); await Promise.all([this.loginWithCookie(), this.initTemplates()]); }, trackPage(): void { diff --git a/packages/editor-ui/src/mixins/nodeHelpers.ts b/packages/editor-ui/src/mixins/nodeHelpers.ts index 232e0c17cf503..ad517e4ad47f3 100644 --- a/packages/editor-ui/src/mixins/nodeHelpers.ts +++ b/packages/editor-ui/src/mixins/nodeHelpers.ts @@ -19,7 +19,7 @@ import type { INodePropertyOptions, IDataObject, } from 'n8n-workflow'; -import { NodeHelpers, ExpressionEvaluatorProxy } from 'n8n-workflow'; +import { NodeHelpers } from 'n8n-workflow'; import type { ICredentialsResponse, @@ -531,9 +531,6 @@ export const nodeHelpers = defineComponent({ if (nodeType !== null && nodeType.subtitle !== undefined) { try { - ExpressionEvaluatorProxy.setEvaluator( - useSettingsStore().settings.expressions?.evaluator ?? 'tournament', - ); return workflow.expression.getSimpleParameterValue( data as INode, nodeType.subtitle, diff --git a/packages/editor-ui/src/mixins/workflowHelpers.ts b/packages/editor-ui/src/mixins/workflowHelpers.ts index 8e175f2201bcb..92d5f3f04a52a 100644 --- a/packages/editor-ui/src/mixins/workflowHelpers.ts +++ b/packages/editor-ui/src/mixins/workflowHelpers.ts @@ -30,7 +30,6 @@ import type { INodeProperties, IWorkflowSettings, } from 'n8n-workflow'; -import { ExpressionEvaluatorProxy } from 'n8n-workflow'; import { NodeHelpers } from 'n8n-workflow'; import type { @@ -164,10 +163,6 @@ export function resolveParameter( } const _executeData = executeData(parentNode, activeNode!.name, inputName, runIndexCurrent); - ExpressionEvaluatorProxy.setEvaluator( - useSettingsStore().settings.expressions?.evaluator ?? 'tournament', - ); - return workflow.expression.getParameterValue( parameter, runExecutionData, diff --git a/packages/workflow/src/Expression.ts b/packages/workflow/src/Expression.ts index 27f42e0b10964..7b5a6633a1bc1 100644 --- a/packages/workflow/src/Expression.ts +++ b/packages/workflow/src/Expression.ts @@ -41,6 +41,7 @@ export const isExpressionError = (error: unknown): error is ExpressionError => export const isTypeError = (error: unknown): error is TypeError => error instanceof TypeError || (error instanceof Error && error.name === 'TypeError'); +// Make sure that error get forwarded setErrorHandler((error: Error) => { if (isExpressionError(error)) throw error; }); diff --git a/packages/workflow/src/ExpressionEvaluatorProxy.ts b/packages/workflow/src/ExpressionEvaluatorProxy.ts index a180df3c0f835..60e33e5a7d15c 100644 --- a/packages/workflow/src/ExpressionEvaluatorProxy.ts +++ b/packages/workflow/src/ExpressionEvaluatorProxy.ts @@ -67,7 +67,7 @@ export const checkEvaluatorDifferences = (expr: string): TmplDifference | null = let diff: TmplDifference | null; try { diff = tournamentEvaluator.tmplDiff(expr); - } catch (e) { + } catch { // We don't include the expression for privacy reasons try { differenceHandler('ERROR'); From 8ed4d83d92a50ef26e55a3be1693dc8c34a6edb7 Mon Sep 17 00:00:00 2001 From: Val <68596159+valya@users.noreply.github.com> Date: Wed, 20 Sep 2023 14:49:11 +0100 Subject: [PATCH 14/14] linting --- packages/editor-ui/src/mixins/workflowHelpers.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/editor-ui/src/mixins/workflowHelpers.ts b/packages/editor-ui/src/mixins/workflowHelpers.ts index 92d5f3f04a52a..6b580c2475d33 100644 --- a/packages/editor-ui/src/mixins/workflowHelpers.ts +++ b/packages/editor-ui/src/mixins/workflowHelpers.ts @@ -62,7 +62,6 @@ import { useNodeTypesStore } from '@/stores/nodeTypes.store'; import { useWorkflowsEEStore } from '@/stores/workflows.ee.store'; import { useEnvironmentsStore } from '@/stores/environments.ee.store'; import { useUsersStore } from '@/stores/users.store'; -import { useSettingsStore } from '@/stores/settings.store'; import { getWorkflowPermissions } from '@/permissions'; import type { IPermissions } from '@/permissions';