From 519e57bda5115149357fb2b1c2270e481ea09e38 Mon Sep 17 00:00:00 2001
From: Michael Kret <88898367+michael-radency@users.noreply.github.com>
Date: Thu, 11 Jul 2024 15:36:39 +0300
Subject: [PATCH] feat: Better error when calling expression function on input
that is undefined or null (#10009)
---
.../nodes/Filter/V2/FilterV2.node.ts | 9 ++----
packages/nodes-base/nodes/If/V2/IfV2.node.ts | 9 ++----
.../nodes/Switch/V3/SwitchV3.node.ts | 6 ++--
packages/nodes-base/utils/constants.ts | 3 ++
.../src/Extensions/ExpressionExtension.ts | 2 ++
packages/workflow/src/Extensions/utils.ts | 13 ++++++++
.../ExpressionExtension.test.ts | 30 ++++++++++++++++++-
7 files changed, 56 insertions(+), 16 deletions(-)
diff --git a/packages/nodes-base/nodes/Filter/V2/FilterV2.node.ts b/packages/nodes-base/nodes/Filter/V2/FilterV2.node.ts
index c4405dba76..6c7e50c197 100644
--- a/packages/nodes-base/nodes/Filter/V2/FilterV2.node.ts
+++ b/packages/nodes-base/nodes/Filter/V2/FilterV2.node.ts
@@ -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;
@@ -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());
diff --git a/packages/nodes-base/nodes/If/V2/IfV2.node.ts b/packages/nodes-base/nodes/If/V2/IfV2.node.ts
index dc3d55c004..2e90747bed 100644
--- a/packages/nodes-base/nodes/If/V2/IfV2.node.ts
+++ b/packages/nodes-base/nodes/If/V2/IfV2.node.ts
@@ -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;
@@ -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());
diff --git a/packages/nodes-base/nodes/Switch/V3/SwitchV3.node.ts b/packages/nodes-base/nodes/Switch/V3/SwitchV3.node.ts
index 0b5b6863d4..60e3e9a34c 100644
--- a/packages/nodes-base/nodes/Switch/V3/SwitchV3.node.ts
+++ b/packages/nodes-base/nodes/Switch/V3/SwitchV3.node.ts
@@ -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;
@@ -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());
diff --git a/packages/nodes-base/utils/constants.ts b/packages/nodes-base/utils/constants.ts
index 400bbc5864..b41e2bd054 100644
--- a/packages/nodes-base/utils/constants.ts
+++ b/packages/nodes-base/utils/constants.ts
@@ -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. More Info";
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.";
diff --git a/packages/workflow/src/Extensions/ExpressionExtension.ts b/packages/workflow/src/Extensions/ExpressionExtension.ts
index a025082848..615e793ecc 100644
--- a/packages/workflow/src/Extensions/ExpressionExtension.ts
+++ b/packages/workflow/src/Extensions/ExpressionExtension.ts
@@ -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';
@@ -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
diff --git a/packages/workflow/src/Extensions/utils.ts b/packages/workflow/src/Extensions/utils.ts
index 563ecf31a1..a144116dea 100644
--- a/packages/workflow/src/Extensions/utils.ts
+++ b/packages/workflow/src/Extensions/utils.ts
@@ -1,4 +1,5 @@
import { DateTime } from 'luxon';
+import { ExpressionExtensionError } from '../errors/expression-extension.error';
// Utility functions and type guards for expression extensions
@@ -17,3 +18,15 @@ export const convertToDateTime = (value: string | Date | DateTime): DateTime | u
}
return converted;
};
+
+export function checkIfValueDefinedOrThrow(value: T, functionName: string): void {
+ if (value === undefined || value === null) {
+ throw new ExpressionExtensionError(
+ `${functionName}() could not be called on "${String(value)}" type`,
+ {
+ description:
+ 'You are trying to access a field that does not exist, modify your expression or set a default value',
+ },
+ );
+ }
+}
diff --git a/packages/workflow/test/ExpressionExtensions/ExpressionExtension.test.ts b/packages/workflow/test/ExpressionExtensions/ExpressionExtension.test.ts
index 2c2f8e391a..a255de4006 100644
--- a/packages/workflow/test/ExpressionExtensions/ExpressionExtension.test.ts
+++ b/packages/workflow/test/ExpressionExtensions/ExpressionExtension.test.ts
@@ -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', () => {
@@ -242,4 +243,31 @@ describe('tmpl Expression Parser', () => {
expect(evaluate('={{ $ifEmpty({a: 1}, "default") }}')).toEqual({ a: 1 });
});
});
+
+ describe('Test extend with undefined', () => {
+ 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');
+ });
+ });
});