From 2708fe81a5323687c59c3d483d6bf3c67464f657 Mon Sep 17 00:00:00 2001 From: Guillaume Jacquart Date: Tue, 22 Jul 2025 09:22:35 +0200 Subject: [PATCH] fix(core): Ignore pairedItem when checking for incorrect output data from a node (#17340) --- .../errors/__tests__/error-reporter.test.ts | 15 +++++++++++++ packages/core/src/errors/error-reporter.ts | 5 ++++- .../src/execution-engine/workflow-execute.ts | 19 ++++++++-------- .../__tests__/is-json-compatible.test.ts | 22 +++++++++++++++++++ packages/core/src/utils/is-json-compatible.ts | 17 ++++++++++---- packages/workflow/src/errors/error.types.ts | 2 ++ 6 files changed, 66 insertions(+), 14 deletions(-) diff --git a/packages/core/src/errors/__tests__/error-reporter.test.ts b/packages/core/src/errors/__tests__/error-reporter.test.ts index cc35d69e88..e5d3f830a6 100644 --- a/packages/core/src/errors/__tests__/error-reporter.test.ts +++ b/packages/core/src/errors/__tests__/error-reporter.test.ts @@ -194,5 +194,20 @@ describe('ErrorReporter', () => { errorReporter.error(error); expect(logger.error).toHaveBeenCalledWith('Test error', metadata); }); + + it.each([true, undefined])( + 'should log the error when shouldBeLogged is %s', + (shouldBeLogged) => { + error.level = 'error'; + errorReporter.error(error, { shouldBeLogged }); + expect(logger.error).toHaveBeenCalledTimes(1); + }, + ); + + it('should not log the error when shouldBeLogged is false', () => { + error.level = 'error'; + errorReporter.error(error, { shouldBeLogged: false }); + expect(logger.error).toHaveBeenCalledTimes(0); + }); }); }); diff --git a/packages/core/src/errors/error-reporter.ts b/packages/core/src/errors/error-reporter.ts index 79a12ef439..5d8e85423e 100644 --- a/packages/core/src/errors/error-reporter.ts +++ b/packages/core/src/errors/error-reporter.ts @@ -60,7 +60,10 @@ export class ErrorReporter { meta = e.extra; } const msg = [e.message + context, stack].join(''); - this.logger.error(msg, meta); + // Default to logging the error if option is not specified + if (options?.shouldBeLogged ?? true) { + this.logger.error(msg, meta); + } e = e.cause as Error; } while (e); } diff --git a/packages/core/src/execution-engine/workflow-execute.ts b/packages/core/src/execution-engine/workflow-execute.ts index bbe842bd70..3fa64592ea 100644 --- a/packages/core/src/execution-engine/workflow-execute.ts +++ b/packages/core/src/execution-engine/workflow-execute.ts @@ -2,6 +2,7 @@ /* eslint-disable @typescript-eslint/no-unsafe-member-access */ /* eslint-disable @typescript-eslint/no-unsafe-assignment */ /* eslint-disable @typescript-eslint/prefer-nullish-coalescing */ +import { GlobalConfig } from '@n8n/config'; import { TOOL_EXECUTOR_NODE_NAME } from '@n8n/constants'; import { Container } from '@n8n/di'; import * as assert from 'assert/strict'; @@ -1211,13 +1212,13 @@ export class WorkflowExecute { : await nodeType.execute.call(context); } - // If data is not json compatible then log it as incorrect output - // Does not block the execution from continuing - const jsonCompatibleResult = isJsonCompatible(data); - if (!jsonCompatibleResult.isValid) { - Container.get(ErrorReporter).error( - new UnexpectedError('node execution output incorrect data'), - { + if (Container.get(GlobalConfig).sentry.backendDsn) { + // If data is not json compatible then log it as incorrect output + // Does not block the execution from continuing + const jsonCompatibleResult = isJsonCompatible(data, new Set(['pairedItem'])); + if (!jsonCompatibleResult.isValid) { + Container.get(ErrorReporter).error('node execution returned incorrect data', { + shouldBeLogged: false, extra: { nodeName: node.name, nodeType: node.type, @@ -1228,8 +1229,8 @@ export class WorkflowExecute { errorPath: jsonCompatibleResult.errorPath, errorMessage: jsonCompatibleResult.errorMessage, }, - }, - ); + }); + } } const closeFunctionsResults = await Promise.allSettled( diff --git a/packages/core/src/utils/__tests__/is-json-compatible.test.ts b/packages/core/src/utils/__tests__/is-json-compatible.test.ts index 7e85ee1e41..23c5e787e6 100644 --- a/packages/core/src/utils/__tests__/is-json-compatible.test.ts +++ b/packages/core/src/utils/__tests__/is-json-compatible.test.ts @@ -27,6 +27,18 @@ describe('isJsonCompatible', () => { errorPath: 'value.date', errorMessage: 'has non-plain prototype (Date)', }, + { + name: 'a RegExp', + value: { regexp: new RegExp('') }, + errorPath: 'value.regexp', + errorMessage: 'has non-plain prototype (RegExp)', + }, + { + name: 'a Buffer', + value: { buffer: Buffer.from('') }, + errorPath: 'value.buffer', + errorMessage: 'has non-plain prototype (Buffer)', + }, { name: 'a function', value: { fn: () => {} }, @@ -131,4 +143,14 @@ describe('isJsonCompatible', () => { expect(result.isValid).toBe(true); }); + + test('skip keys that are in the keysToIgnore set', () => { + const value = { + invalidObject: { invalidBecauseUndefined: undefined }, + validObject: { key: 'value' }, + }; + const result = isJsonCompatible(value, new Set(['invalidObject'])); + + expect(result.isValid).toBe(true); + }); }); diff --git a/packages/core/src/utils/is-json-compatible.ts b/packages/core/src/utils/is-json-compatible.ts index 862b075fb8..8f3149c405 100644 --- a/packages/core/src/utils/is-json-compatible.ts +++ b/packages/core/src/utils/is-json-compatible.ts @@ -2,6 +2,7 @@ const check = ( val: unknown, path = 'value', stack: Set = new Set(), + keysToIgnore: Set = new Set(), ): { isValid: true } | { isValid: false; errorPath: string; errorMessage: string } => { const type = typeof val; @@ -38,7 +39,7 @@ const check = ( } stack.add(val); for (let i = 0; i < val.length; i++) { - const result = check(val[i], `${path}[${i}]`, stack); + const result = check(val[i], `${path}[${i}]`, stack, keysToIgnore); if (!result.isValid) return result; } stack.delete(val); @@ -74,8 +75,12 @@ const check = ( }; } + if (keysToIgnore.has(key)) { + continue; + } + const subVal = (val as Record)[key]; - const result = check(subVal, `${path}.${key}`, stack); + const result = check(subVal, `${path}.${key}`, stack, keysToIgnore); if (!result.isValid) return result; } stack.delete(val); @@ -92,14 +97,18 @@ const check = ( /** * This function checks if a value matches JSON data type restrictions. * @param value + * @param keysToIgnore - Set of keys to ignore for objects * @returns boolean */ -export function isJsonCompatible(value: unknown): +export function isJsonCompatible( + value: unknown, + keysToIgnore: Set = new Set(), +): | { isValid: true } | { isValid: false; errorPath: string; errorMessage: string; } { - return check(value); + return check(value, undefined, undefined, keysToIgnore); } diff --git a/packages/workflow/src/errors/error.types.ts b/packages/workflow/src/errors/error.types.ts index 54435298ec..7cbcc4ccf9 100644 --- a/packages/workflow/src/errors/error.types.ts +++ b/packages/workflow/src/errors/error.types.ts @@ -7,6 +7,8 @@ export type ErrorTags = NonNullable; export type ReportingOptions = { /** Whether the error should be reported to Sentry */ shouldReport?: boolean; + /** Whether the error log should be logged (default to true) */ + shouldBeLogged?: boolean; level?: ErrorLevel; tags?: ErrorTags; extra?: Event['extra'];