Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(core): Add Tournament as the new default expression evaluator #6964

Merged
merged 30 commits into from
Sep 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
75ba58f
add new expression evaluator and setting to toggle it
valya Jun 21, 2023
a15ac9b
Merge remote-tracking branch 'origin/master' into pay-356-enable-new-…
valya Jul 20, 2023
5d322a0
feat: add expression difference reporter
valya Jul 20, 2023
ddfff5e
Merge remote-tracking branch 'origin/master' into pay-356-enable-new-…
valya Aug 17, 2023
df0f3e0
Merge remote-tracking branch 'origin/master' into pay-356-enable-new-…
valya Aug 17, 2023
37b2e0f
add more error handling to expression evaluator proxy
valya Aug 17, 2023
805cce0
add @n8n/tournament package to package.json
valya Aug 17, 2023
4e6084b
linting fixes
valya Aug 17, 2023
ca46a41
Merge branch 'pay-356-enable-new-evaluator-config' of github.com:n8n-…
valya Aug 17, 2023
7fa896c
Merge remote-tracking branch 'origin/master' into pay-356-enable-new-…
valya Aug 17, 2023
e1a21e3
Merge remote-tracking branch 'origin/master' into pay-356-enable-new-…
valya Aug 21, 2023
6fe2318
Merge remote-tracking branch 'origin/master' into pay-356-enable-new-…
valya Aug 22, 2023
7389ede
Merge remote-tracking branch 'origin/master' into pay-356-enable-new-…
valya Aug 22, 2023
b03d926
Merge branch 'master' into pay-356-enable-new-evaluator-config
krynble Aug 28, 2023
fecc276
fix: Import type
krynble Aug 28, 2023
b965ccf
Merge remote-tracking branch 'origin/master' into pay-356-enable-new-…
netroy Aug 31, 2023
e925cbd
Merge remote-tracking branch 'origin/master' into pay-356-enable-new-…
valya Sep 5, 2023
02fadf9
chore: Update tournament
valya Sep 5, 2023
2f3fa01
Merge remote-tracking branch 'origin/master' into pay-356-enable-new-…
valya Sep 7, 2023
9b7e3a3
update lock file
valya Sep 7, 2023
23ff97d
Merge remote-tracking branch 'origin/master' into pay-356-enable-new-…
valya Sep 11, 2023
25249c9
Merge remote-tracking branch 'origin/master' into pay-356-enable-new-…
valya Sep 14, 2023
f1209b1
fix: Expression evaluator not being set for node subtitles
valya Sep 14, 2023
936dc62
Merge remote-tracking branch 'origin/master' into pay-356-enable-new-…
valya Sep 19, 2023
b3340ad
feat: make tournament the default expression evaluator
valya Sep 19, 2023
61a8d84
remove variable from bad merge
valya Sep 19, 2023
3822e90
update frontend default expression evaluator to be inline with backend
valya Sep 19, 2023
17c0030
review changes
valya Sep 20, 2023
98efcbd
Merge remote-tracking branch 'origin/master' into pay-356-enable-new-…
valya Sep 20, 2023
8ed4d83
linting
valya Sep 20, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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,
},
});
ivov marked this conversation as resolved.
Show resolved Hide resolved
});
};
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();
ivov marked this conversation as resolved.
Show resolved Hide resolved

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 @@ -1205,6 +1205,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 @@ -151,6 +152,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');
ivov marked this conversation as resolved.
Show resolved Hide resolved
}
}

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
Loading