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: Better error when calling expression function on input that is undefined or null #10009

Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
9 changes: 3 additions & 6 deletions packages/nodes-base/nodes/Filter/V2/FilterV2.node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import type {
INodeTypeBaseDescription,
INodeTypeDescription,
} from 'n8n-workflow';
import { ENABLE_LESS_STRICT_TYPE_VALIDATION } from '../../../utils/constants';

export class FilterV2 implements INodeType {
description: INodeTypeDescription;
Expand Down Expand Up @@ -79,12 +80,8 @@ export class FilterV2 implements INodeType {
extractValue: true,
}) as boolean;
} catch (error) {
if (!options.looseTypeValidation) {
set(
error,
'description',
"Try changing the type of comparison. Alternatively you can enable 'Less Strict Type Validation' in the options.",
);
if (!options.looseTypeValidation && !error.description) {
set(error, 'description', ENABLE_LESS_STRICT_TYPE_VALIDATION);
}
set(error, 'context.itemIndex', itemIndex);
set(error, 'node', this.getNode());
Expand Down
9 changes: 3 additions & 6 deletions packages/nodes-base/nodes/If/V2/IfV2.node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import type {
INodeTypeBaseDescription,
INodeTypeDescription,
} from 'n8n-workflow';
import { ENABLE_LESS_STRICT_TYPE_VALIDATION } from '../../../utils/constants';

export class IfV2 implements INodeType {
description: INodeTypeDescription;
Expand Down Expand Up @@ -79,12 +80,8 @@ export class IfV2 implements INodeType {
extractValue: true,
}) as boolean;
} catch (error) {
if (!options.looseTypeValidation) {
set(
error,
'description',
"Try changing the type of comparison. Alternatively you can enable 'Less Strict Type Validation' in the options.",
);
if (!options.looseTypeValidation && !error.description) {
set(error, 'description', ENABLE_LESS_STRICT_TYPE_VALIDATION);
}
set(error, 'context.itemIndex', itemIndex);
set(error, 'node', this.getNode());
Expand Down
6 changes: 3 additions & 3 deletions packages/nodes-base/nodes/Switch/V3/SwitchV3.node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import type {
import { NodeConnectionType, NodeOperationError } from 'n8n-workflow';
import set from 'lodash/set';
import { capitalize } from '@utils/utilities';
import { ENABLE_LESS_STRICT_TYPE_VALIDATION } from '../../../utils/constants';

const configuredOutputs = (parameters: INodeParameters) => {
const mode = parameters.mode as string;
Expand Down Expand Up @@ -348,9 +349,8 @@ export class SwitchV3 implements INodeType {
},
) as boolean;
} catch (error) {
if (!options.looseTypeValidation) {
error.description =
"Try changing the type of comparison. Alternatively you can enable 'Less Strict Type Validation' in the options.";
if (!options.looseTypeValidation && !error.description) {
error.description = ENABLE_LESS_STRICT_TYPE_VALIDATION;
}
set(error, 'context.itemIndex', itemIndex);
set(error, 'node', this.getNode());
Expand Down
3 changes: 3 additions & 0 deletions packages/nodes-base/utils/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,6 @@ export const NODE_RAN_MULTIPLE_TIMES_WARNING =
"This node ran multiple times - once for each input item. You can change this by setting 'execute once' in the node settings. <a href='https://docs.n8n.io/flow-logic/looping/#executing-nodes-once' target='_blank'>More Info</a>";

export const LOCALHOST = '127.0.0.1';

export const ENABLE_LESS_STRICT_TYPE_VALIDATION =
"Try changing the type of comparison. Alternatively you can enable 'Less Strict Type Validation' in the options.";
2 changes: 2 additions & 0 deletions packages/workflow/src/Extensions/ExpressionExtension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import type { ExpressionChunk, ExpressionCode } from './ExpressionParser';
import { joinExpression, splitExpression } from './ExpressionParser';
import { booleanExtensions } from './BooleanExtensions';
import type { ExtensionMap } from './Extensions';
import { checkIfValueDefinedOrThrow } from './utils';

const EXPRESSION_EXTENDER = 'extend';
const EXPRESSION_EXTENDER_OPTIONAL = 'extendOptional';
Expand Down Expand Up @@ -514,6 +515,7 @@ export function extend(input: unknown, functionName: string, args: unknown[]) {
// any types have a function with that name. Then throw an error
// letting the user know the available types.
if (!foundFunction) {
checkIfValueDefinedOrThrow(input, functionName);
const haveFunction = EXTENSION_OBJECTS.filter((v) => functionName in v.functions);
if (!haveFunction.length) {
// This shouldn't really be possible but we should cover it anyway
Expand Down
13 changes: 13 additions & 0 deletions packages/workflow/src/Extensions/utils.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { DateTime } from 'luxon';
import { ExpressionExtensionError } from '../errors/expression-extension.error';

// Utility functions and type guards for expression extensions

Expand All @@ -17,3 +18,15 @@ export const convertToDateTime = (value: string | Date | DateTime): DateTime | u
}
return converted;
};

export function checkIfValueDefinedOrThrow<T>(value: T, functionName: string): void {
if (value === undefined || value === null) {
throw new ExpressionExtensionError(
`${functionName}() could not be called on "${String(value)}" type`,
{
description:
'Likely you are accesing a field that does not exist, modify your expression or set a default value',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Slight tweak

You are trying to access a field that does not exist, modify your expression or set a default value

},
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@

/* eslint-disable n8n-local-rules/no-interpolation-in-regular-string */

import { extendTransform } from '@/Extensions';
import { extendTransform, extend } from '@/Extensions';
import { joinExpression, splitExpression } from '@/Extensions/ExpressionParser';
import { evaluate } from './Helpers';
import { ExpressionExtensionError } from '../../src/errors/expression-extension.error';

describe('Expression Extension Transforms', () => {
describe('extend() transform', () => {
Expand Down Expand Up @@ -242,4 +243,31 @@ describe('tmpl Expression Parser', () => {
expect(evaluate('={{ $ifEmpty({a: 1}, "default") }}')).toEqual({ a: 1 });
});
});

describe('Test estend with undefined', () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo... extend

test('input is undefined', () => {
try {
extend(undefined, 'toDateTime', []);
} catch (error) {
expect(error).toBeInstanceOf(ExpressionExtensionError);
expect(error).toHaveProperty(
'message',
'toDateTime() could not be called on "undefined" type',
);
}
});
test('input is null', () => {
try {
extend(null, 'startsWith', []);
} catch (error) {
expect(error).toBeInstanceOf(ExpressionExtensionError);
expect(error).toHaveProperty('message', 'startsWith() could not be called on "null" type');
}
});
test('input should be converted to upper case', () => {
const result = extend('TEST', 'toUpperCase', []);

expect(result).toEqual('TEST');
});
});
});
Loading