fix(core): Ignore pairedItem when checking for incorrect output data from a node (#17340)

This commit is contained in:
Guillaume Jacquart
2025-07-22 09:22:35 +02:00
committed by GitHub
parent 73f8e72aca
commit 2708fe81a5
6 changed files with 66 additions and 14 deletions

View File

@@ -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);
});
});
});

View File

@@ -60,7 +60,10 @@ export class ErrorReporter {
meta = e.extra;
}
const msg = [e.message + context, stack].join('');
// 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);
}

View File

@@ -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 (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);
const jsonCompatibleResult = isJsonCompatible(data, new Set(['pairedItem']));
if (!jsonCompatibleResult.isValid) {
Container.get(ErrorReporter).error(
new UnexpectedError('node execution output incorrect data'),
{
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(

View File

@@ -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);
});
});

View File

@@ -2,6 +2,7 @@ const check = (
val: unknown,
path = 'value',
stack: Set<unknown> = new Set(),
keysToIgnore: Set<string> = 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<string, unknown>)[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<string> = new Set(),
):
| { isValid: true }
| {
isValid: false;
errorPath: string;
errorMessage: string;
} {
return check(value);
return check(value, undefined, undefined, keysToIgnore);
}

View File

@@ -7,6 +7,8 @@ export type ErrorTags = NonNullable<Event['tags']>;
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'];