fix: Don't throw errors for NaN in number operators in the filter com…

…ponent (#9506)
n8n-io · May 30, 2024 · 936bbb2 · 936bbb2
1 parent e55bf03
commit 936bbb2
Show file tree

Hide file tree

Showing 6 changed files with 66 additions and 8 deletions.
diff --git a/packages/editor-ui/src/composables/useExpressionEditor.ts b/packages/editor-ui/src/composables/useExpressionEditor.ts
@@ -331,10 +331,6 @@ export const useExpressionEditor = ({
 			result.error = true;
 		}
 
-		if (typeof result.resolved === 'number' && isNaN(result.resolved)) {
-			result.resolved = i18n.baseText('expressionModalInput.null');
-		}
-
 		return result;
 	}
 

diff --git a/packages/editor-ui/src/utils/__tests__/expressions.test.ts b/packages/editor-ui/src/utils/__tests__/expressions.test.ts
@@ -24,6 +24,10 @@ describe('stringifyExpressionResult()', () => {
 		expect(stringifyExpressionResult({ ok: true, result: null })).toEqual('');
 	});
 
+	it('should return NaN when result is NaN', () => {
+		expect(stringifyExpressionResult({ ok: true, result: NaN })).toEqual('NaN');
+	});
+
 	it('should return [empty] message when result is empty string', () => {
 		expect(stringifyExpressionResult({ ok: true, result: '' })).toEqual('[empty]');
 	});

diff --git a/packages/editor-ui/src/utils/expressions.ts b/packages/editor-ui/src/utils/expressions.ts
@@ -1,7 +1,7 @@
-import type { ResolvableState } from '@/types/expressions';
-import { ExpressionError, ExpressionParser, type Result } from 'n8n-workflow';
 import { i18n } from '@/plugins/i18n';
 import { useWorkflowsStore } from '@/stores/workflows.store';
+import type { ResolvableState } from '@/types/expressions';
+import { ExpressionError, ExpressionParser, type Result } from 'n8n-workflow';
 
 export const isExpression = (expr: unknown) => {
 	if (typeof expr !== 'string') return false;
@@ -128,5 +128,5 @@ export const stringifyExpressionResult = (result: Result<unknown, Error>): strin
 		return i18n.baseText('parameterInput.emptyString');
 	}
 
-	return typeof result.result === 'string' ? result.result : JSON.stringify(result.result);
+	return typeof result.result === 'string' ? result.result : String(result.result);
 };
diff --git a/packages/workflow/src/Expression.ts b/packages/workflow/src/Expression.ts
@@ -77,6 +77,10 @@ export class Expression {
 			throw new ApplicationError('invalid DateTime');
 		}
 
+		if (value === null) {
+			return 'null';
+		}
+
 		let typeName = value.constructor.name ?? 'Object';
 		if (DateTime.isDateTime(value)) {
 			typeName = 'DateTime';

diff --git a/packages/workflow/src/NodeParameters/FilterParameter.ts b/packages/workflow/src/NodeParameters/FilterParameter.ts
@@ -40,6 +40,10 @@ function parseSingleFilterValue(
 		return { valid: true, newValue: Boolean(value) };
 	}
 
+	if (type === 'number' && Number.isNaN(value)) {
+		return { valid: true, newValue: value };
+	}
+
 	return validateFieldType('filter', value, type, { strict, parseStrings: true });
 }
 
@@ -149,7 +153,7 @@ export function executeFilterCondition(
 
 	let { left: leftValue, right: rightValue } = parsedValues.result;
 
-	const exists = leftValue !== undefined && leftValue !== null;
+	const exists = leftValue !== undefined && leftValue !== null && !Number.isNaN(leftValue);
 	if (condition.operator.operation === 'exists') {
 		return exists;
 	} else if (condition.operator.operation === 'notExists') {

diff --git a/packages/workflow/test/FilterParameter.test.ts b/packages/workflow/test/FilterParameter.test.ts
@@ -528,11 +528,55 @@ describe('FilterParameter', () => {
 			});
 
 			describe('number', () => {
+				it.each([
+					{ left: 0, expected: true },
+					{ left: 15, expected: true },
+					{ left: -15.4, expected: true },
+					{ left: NaN, expected: false },
+					{ left: null, expected: false },
+				])('number:exists($left) === $expected', ({ left, expected }) => {
+					const result = executeFilter(
+						filterFactory({
+							conditions: [
+								{
+									id: '1',
+									leftValue: left,
+									operator: { operation: 'exists', type: 'number' },
+								},
+							],
+						}),
+					);
+					expect(result).toBe(expected);
+				});
+
+				it.each([
+					{ left: 0, expected: false },
+					{ left: 15, expected: false },
+					{ left: -15.4, expected: false },
+					{ left: NaN, expected: true },
+					{ left: null, expected: true },
+				])('number:notExists($left) === $expected', ({ left, expected }) => {
+					const result = executeFilter(
+						filterFactory({
+							conditions: [
+								{
+									id: '1',
+									leftValue: left,
+									operator: { operation: 'notExists', type: 'number' },
+								},
+							],
+						}),
+					);
+					expect(result).toBe(expected);
+				});
+
 				it.each([
 					{ left: 0, right: 0, expected: true },
 					{ left: 15, right: 15, expected: true },
 					{ left: 15.34, right: 15.34, expected: true },
 					{ left: 15, right: 15.3249038, expected: false },
+					{ left: 15, right: NaN, expected: false },
+					{ left: NaN, right: NaN, expected: false },
 				])('number:equals($left,$right) === $expected', ({ left, right, expected }) => {
 					const result = executeFilter(
 						filterFactory({
@@ -554,6 +598,8 @@ describe('FilterParameter', () => {
 					{ left: 15, right: 15, expected: false },
 					{ left: 15.34, right: 15.34, expected: false },
 					{ left: 15, right: 15.3249038, expected: true },
+					{ left: 15, right: NaN, expected: true },
+					{ left: NaN, right: NaN, expected: true },
 				])('number:notEquals($left,$right) === $expected', ({ left, right, expected }) => {
 					const result = executeFilter(
 						filterFactory({
@@ -575,6 +621,7 @@ describe('FilterParameter', () => {
 					{ left: 15, right: 16, expected: false },
 					{ left: 16, right: 15, expected: true },
 					{ left: 15.34001, right: 15.34, expected: true },
+					{ left: 15, right: NaN, expected: false },
 				])('number:gt($left,$right) === $expected', ({ left, right, expected }) => {
 					const result = executeFilter(
 						filterFactory({
@@ -596,6 +643,7 @@ describe('FilterParameter', () => {
 					{ left: 15, right: 16, expected: true },
 					{ left: 16, right: 15, expected: false },
 					{ left: 15.34001, right: 15.34, expected: false },
+					{ left: 15, right: NaN, expected: false },
 				])('number:lt($left,$right) === $expected', ({ left, right, expected }) => {
 					const result = executeFilter(
 						filterFactory({
@@ -617,6 +665,7 @@ describe('FilterParameter', () => {
 					{ left: 15, right: 16, expected: false },
 					{ left: 16, right: 15, expected: true },
 					{ left: 15.34001, right: 15.34, expected: true },
+					{ left: 15, right: NaN, expected: false },
 				])('number:gte($left,$right) === $expected', ({ left, right, expected }) => {
 					const result = executeFilter(
 						filterFactory({
@@ -638,6 +687,7 @@ describe('FilterParameter', () => {
 					{ left: 15, right: 16, expected: true },
 					{ left: 16, right: 15, expected: false },
 					{ left: 15.34001, right: 15.34, expected: false },
+					{ left: 15, right: NaN, expected: false },
 				])('number:lte($left,$right) === $expected', ({ left, right, expected }) => {
 					const result = executeFilter(
 						filterFactory({