Skip to content

Commit

Permalink
feat(core): Add Tournament as the new default expression evaluator (#…
Browse files Browse the repository at this point in the history
…6964)

Github issue / Community forum post (link here to close automatically):

---------

Co-authored-by: Omar Ajoue <krynble@gmail.com>
Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <aditya@netroy.in>
  • Loading branch information
3 people authored Sep 21, 2023
1 parent 67b985f commit bf74f09
Show file tree
Hide file tree
Showing 12 changed files with 419 additions and 208 deletions.
14 changes: 14 additions & 0 deletions packages/cli/src/ExpressionEvalator.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import config from '@/config';
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,
},
});
});
};
3 changes: 3 additions & 0 deletions packages/cli/src/Server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,9 @@ export class Server extends AbstractServer {
variables: {
limit: 0,
},
expressions: {
evaluator: config.getEnv('expression.evaluator'),
},
banners: {
dismissed: [],
},
Expand Down
2 changes: 2 additions & 0 deletions packages/cli/src/commands/BaseCommand.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { InternalHooks } from '@/InternalHooks';
import { PostHogClient } from '@/posthog';
import { License } from '@/License';
import { ExternalSecretsManager } from '@/ExternalSecrets/ExternalSecretsManager.ee';
import { initExpressionEvaluator } from '@/ExpressionEvalator';

export abstract class BaseCommand extends Command {
protected logger = LoggerProxy.init(getLogger());
Expand All @@ -39,6 +40,7 @@ export abstract class BaseCommand extends Command {

async init(): Promise<void> {
await initErrorHandling();
initExpressionEvaluator();

process.once('SIGTERM', async () => this.stopProcess());
process.once('SIGINT', async () => this.stopProcess());
Expand Down
15 changes: 15 additions & 0 deletions packages/cli/src/config/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1199,6 +1199,21 @@ export const schema = {
},
},

expression: {
evaluator: {
doc: 'Expression evaluator to use',
format: ['tmpl', 'tournament'] as const,
default: 'tournament',
env: 'N8N_EXPRESSION_EVALUATOR',
},
reportDifference: {
doc: 'Whether to report differences in the evaluator outputs',
format: Boolean,
default: false,
env: 'N8N_EXPRESSION_REPORT_DIFFERENCE',
},
},

sourceControl: {
defaultKeyPairType: {
doc: 'Default SSH key type to use when generating SSH keys',
Expand Down
2 changes: 2 additions & 0 deletions packages/editor-ui/src/App.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -148,6 +149,7 @@ export default defineComponent({
},
async initialize(): Promise<void> {
await this.initSettings();
ExpressionEvaluatorProxy.setEvaluator(useSettingsStore().settings.expressions.evaluator);
await Promise.all([this.loginWithCookie(), this.initTemplates()]);
},
trackPage(): void {
Expand Down
1 change: 1 addition & 0 deletions packages/workflow/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
"@types/xml2js": "^0.4.11"
},
"dependencies": {
"@n8n/tournament": "^1.0.2",
"@n8n_io/riot-tmpl": "^4.0.0",
"ast-types": "0.15.2",
"crypto-js": "^4.1.1",
Expand Down
12 changes: 5 additions & 7 deletions packages/workflow/src/Expression.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as tmpl from '@n8n_io/riot-tmpl';
import { DateTime, Duration, Interval } from 'luxon';
import * as tmpl from '@n8n_io/riot-tmpl';

import type {
IDataObject,
Expand All @@ -22,6 +22,7 @@ import type { Workflow } from './Workflow';
import { extend, extendOptional } from './Extensions';
import { extendedFunctions } from './Extensions/ExtendedFunctions';
import { extendSyntax } from './Extensions/ExpressionExtension';
import { evaluateExpression, setErrorHandler } from './ExpressionEvaluatorProxy';

const IS_FRONTEND_IN_DEV_MODE =
typeof process === 'object' &&
Expand All @@ -40,13 +41,10 @@ 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');

// 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) => {
setErrorHandler((error: Error) => {
if (isExpressionError(error)) throw error;
};
});

// eslint-disable-next-line @typescript-eslint/naming-convention
const AsyncFunction = (async () => {}).constructor as FunctionConstructor;
Expand Down Expand Up @@ -339,7 +337,7 @@ export class Expression {
[Function, AsyncFunction].forEach(({ prototype }) =>
Object.defineProperty(prototype, 'constructor', { value: fnConstructors.mock }),
);
return tmpl.tmpl(expression, data);
return evaluateExpression(expression, data);
} catch (error) {
if (isExpressionError(error)) throw error;

Expand Down
149 changes: 149 additions & 0 deletions packages/workflow/src/ExpressionEvaluatorProxy.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
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 LoggerProxy from './LoggerProxy';

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) => {
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);
let evaluator: Evaluator = tmpl.tmpl;
let currentEvaluatorType: ExpressionEvaluatorType = 'tmpl';
let diffExpressions = false;

export const setErrorHandler = (handler: ErrorHandler) => {
errorHandler = handler;
tmpl.tmpl.errorHandler = handler;
tournamentEvaluator.errorHandler = handler;
};

export const setEvaluator = (evalType: ExpressionEvaluatorType) => {
currentEvaluatorType = 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 setDifferEnabled = (enabled: boolean) => {
diffExpressions = enabled;
};

const diffCache: Record<string, TmplDifference | null> = {};

export const checkEvaluatorDifferences = (expr: string): TmplDifference | null => {
if (expr in diffCache) {
return diffCache[expr];
}
let diff: TmplDifference | null;
try {
diff = tournamentEvaluator.tmplDiff(expr);
} catch {
// We don't include the expression for privacy reasons
try {
differenceHandler('ERROR');
} catch {}
diff = null;
}

if (diff?.same === false) {
differenceChecker(diff);
}

diffCache[expr] = diff;
return diff;
};

export const getEvaluator = () => {
return evaluator;
};

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 ||
JSON.stringify(tmplValue!) !== JSON.stringify(tournValue!)
) {
try {
if (diff.expression) {
differenceHandler(diff.expression.value);
} else {
differenceHandler('VALUEDIFF');
}
} catch {
LoggerProxy.error('Failed to report error difference');
}
}

if (currentEvaluatorType === 'tmpl') {
if (wasTmplError) {
throw tmplError;
}
return tmplValue!;
}

if (wasTournError) {
throw tournError;
}
return tournValue!;
};
5 changes: 5 additions & 0 deletions packages/workflow/src/Interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2117,6 +2117,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;
Expand Down Expand Up @@ -2203,6 +2205,9 @@ export interface IN8nUISettings {
variables: {
limit: number;
};
expressions: {
evaluator: ExpressionEvaluatorType;
};
mfa: {
enabled: boolean;
};
Expand Down
1 change: 1 addition & 0 deletions packages/workflow/src/index.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down
Loading

0 comments on commit bf74f09

Please sign in to comment.