fix(core): Filter out task runner errors originating from user code from sentry (no-changelog) (#12537)

packages/@n8n/task-runner/src/__tests__/task-runner-sentry.test.ts

@@ -0,0 +1,178 @@
+import type { ErrorEvent } from '@sentry/types';
+import { mock } from 'jest-mock-extended';
+import type { ErrorReporter } from 'n8n-core';
+
+import { TaskRunnerSentry } from '../task-runner-sentry';
+
+describe('TaskRunnerSentry', () => {
+	afterEach(() => {
+		jest.resetAllMocks();
+	});
+
+	describe('filterOutUserCodeErrors', () => {
+		const sentry = new TaskRunnerSentry(
+			{
+				dsn: 'https://sentry.io/123',
+				n8nVersion: '1.0.0',
+				environment: 'local',
+				deploymentName: 'test',
+			},
+			mock(),
+		);
+
+		it('should filter out user code errors', () => {
+			const event: ErrorEvent = {
+				type: undefined,
+				exception: {
+					values: [
+						{
+							type: 'ReferenceError',
+							value: 'fetch is not defined',
+							stacktrace: {
+								frames: [
+									{
+										filename: 'app:///dist/js-task-runner/js-task-runner.js',
+										module: 'js-task-runner:js-task-runner',
+										function: 'JsTaskRunner.executeTask',
+									},
+									{
+										filename: 'app:///dist/js-task-runner/js-task-runner.js',
+										module: 'js-task-runner:js-task-runner',
+										function: 'JsTaskRunner.runForAllItems',
+									},
+									{
+										filename: '<anonymous>',
+										module: '<anonymous>',
+										function: 'new Promise',
+									},
+									{
+										filename: 'app:///dist/js-task-runner/js-task-runner.js',
+										module: 'js-task-runner:js-task-runner',
+										function: 'result',
+									},
+									{
+										filename: 'node:vm',
+										module: 'node:vm',
+										function: 'runInContext',
+									},
+									{
+										filename: 'node:vm',
+										module: 'node:vm',
+										function: 'Script.runInContext',
+									},
+									{
+										filename: 'evalmachine.<anonymous>',
+										module: 'evalmachine.<anonymous>',
+										function: '?',
+									},
+									{
+										filename: 'evalmachine.<anonymous>',
+										module: 'evalmachine.<anonymous>',
+										function: 'VmCodeWrapper',
+									},
+									{
+										filename: '<anonymous>',
+										module: '<anonymous>',
+										function: 'new Promise',
+									},
+									{
+										filename: 'evalmachine.<anonymous>',
+										module: 'evalmachine.<anonymous>',
+									},
+								],
+							},
+							mechanism: { type: 'onunhandledrejection', handled: false },
+						},
+					],
+				},
+				event_id: '18bb78bb3d9d44c4acf3d774c2cfbfd8',
+				platform: 'node',
+				contexts: {
+					trace: { trace_id: '3c3614d33a6b47f09b85ec7d2710acea', span_id: 'ad00fdf6d6173aeb' },
+					runtime: { name: 'node', version: 'v20.17.0' },
+				},
+			};
+
+			expect(sentry.filterOutUserCodeErrors(event)).toBe(true);
+		});
+	});
+
+	describe('initIfEnabled', () => {
+		const mockErrorReporter = mock<ErrorReporter>();
+
+		it('should not configure sentry if dsn is not set', async () => {
+			const sentry = new TaskRunnerSentry(
+				{
+					dsn: '',
+					n8nVersion: '1.0.0',
+					environment: 'local',
+					deploymentName: 'test',
+				},
+				mockErrorReporter,
+			);
+
+			await sentry.initIfEnabled();
+
+			expect(mockErrorReporter.init).not.toHaveBeenCalled();
+		});
+
+		it('should configure sentry if dsn is set', async () => {
+			const sentry = new TaskRunnerSentry(
+				{
+					dsn: 'https://sentry.io/123',
+					n8nVersion: '1.0.0',
+					environment: 'local',
+					deploymentName: 'test',
+				},
+				mockErrorReporter,
+			);
+
+			await sentry.initIfEnabled();
+
+			expect(mockErrorReporter.init).toHaveBeenCalledWith({
+				dsn: 'https://sentry.io/123',
+				beforeSendFilter: sentry.filterOutUserCodeErrors,
+				release: '1.0.0',
+				environment: 'local',
+				serverName: 'test',
+				serverType: 'task_runner',
+			});
+		});
+	});
+
+	describe('shutdown', () => {
+		const mockErrorReporter = mock<ErrorReporter>();
+
+		it('should not shutdown sentry if dsn is not set', async () => {
+			const sentry = new TaskRunnerSentry(
+				{
+					dsn: '',
+					n8nVersion: '1.0.0',
+					environment: 'local',
+					deploymentName: 'test',
+				},
+				mockErrorReporter,
+			);
+
+			await sentry.shutdown();
+
+			expect(mockErrorReporter.shutdown).not.toHaveBeenCalled();
+		});
+
+		it('should shutdown sentry if dsn is set', async () => {
+			const sentry = new TaskRunnerSentry(
+				{
+					dsn: 'https://sentry.io/123',
+					n8nVersion: '1.0.0',
+					environment: 'local',
+					deploymentName: 'test',
+				},
+				mockErrorReporter,
+			);
+
+			await sentry.shutdown();
+
+			expect(mockErrorReporter.shutdown).toHaveBeenCalled();
+		});
+	});
+});

packages/@n8n/task-runner/src/start.ts

@@ -1,16 +1,16 @@
 import './polyfills';
 import { Container } from '@n8n/di';
-import type { ErrorReporter } from 'n8n-core';
 import { ensureError, setGlobalState } from 'n8n-workflow';
 
 import { MainConfig } from './config/main-config';
 import type { HealthCheckServer } from './health-check-server';
 import { JsTaskRunner } from './js-task-runner/js-task-runner';
+import { TaskRunnerSentry } from './task-runner-sentry';
 
 let healthCheckServer: HealthCheckServer | undefined;
 let runner: JsTaskRunner | undefined;
 let isShuttingDown = false;
-let errorReporter: ErrorReporter | undefined;
+let sentry: TaskRunnerSentry | undefined;
 
 function createSignalHandler(signal: string, timeoutInS = 10) {
 	return async function onSignal() {
@@ -33,9 +33,9 @@ function createSignalHandler(signal: string, timeoutInS = 10) {
 				void healthCheckServer?.stop();
 			}
 
-			if (errorReporter) {
-				await errorReporter.shutdown();
-				errorReporter = undefined;
+			if (sentry) {
+				await sentry.shutdown();
+				sentry = undefined;
 			}
 		} catch (e) {
 			const error = ensureError(e);
@@ -54,20 +54,8 @@ void (async function start() {
 		defaultTimezone: config.baseRunnerConfig.timezone,
 	});
 
-	const { dsn } = config.sentryConfig;
-
-	if (dsn) {
-		const { ErrorReporter } = await import('n8n-core');
-		errorReporter = Container.get(ErrorReporter);
-		const { deploymentName, environment, n8nVersion } = config.sentryConfig;
-		await errorReporter.init({
-			serverType: 'task_runner',
-			dsn,
-			serverName: deploymentName,
-			environment,
-			release: n8nVersion,
-		});
-	}
+	sentry = Container.get(TaskRunnerSentry);
+	await sentry.initIfEnabled();
 
 	runner = new JsTaskRunner(config);
 	runner.on('runner:reached-idle-timeout', () => {

packages/@n8n/task-runner/src/task-runner-sentry.ts

@@ -0,0 +1,62 @@
+import { Service } from '@n8n/di';
+import type { ErrorEvent, Exception } from '@sentry/types';
+import { ErrorReporter } from 'n8n-core';
+
+import { SentryConfig } from './config/sentry-config';
+
+/**
+ * Sentry service for the task runner.
+ */
+@Service()
+export class TaskRunnerSentry {
+	constructor(
+		private readonly config: SentryConfig,
+		private readonly errorReporter: ErrorReporter,
+	) {}
+
+	async initIfEnabled() {
+		const { dsn, n8nVersion, environment, deploymentName } = this.config;
+
+		if (!dsn) return;
+
+		await this.errorReporter.init({
+			serverType: 'task_runner',
+			dsn,
+			release: n8nVersion,
+			environment,
+			serverName: deploymentName,
+			beforeSendFilter: this.filterOutUserCodeErrors,
+		});
+	}
+
+	async shutdown() {
+		if (!this.config.dsn) return;
+
+		await this.errorReporter.shutdown();
+	}
+
+	/**
+	 * Filter out errors originating from user provided code.
+	 * It is possible for users to create code that causes unhandledrejections
+	 * that end up in the sentry error reporting.
+	 */
+	filterOutUserCodeErrors = (event: ErrorEvent) => {
+		const error = event?.exception?.values?.[0];
+
+		return error ? this.isUserCodeError(error) : false;
+	};
+
+	/**
+	 * Check if the error is originating from user provided code.
+	 * It is possible for users to create code that causes unhandledrejections
+	 * that end up in the sentry error reporting.
+	 */
+	private isUserCodeError(error: Exception) {
+		const frames = error.stacktrace?.frames;
+		if (!frames) return false;
+
+		return frames.some(
+			(frame) => frame.filename === 'node:vm' && frame.function === 'runInContext',
+		);
+	}
+}

packages/core/src/error-reporter.ts

@@ -15,6 +15,11 @@ type ErrorReporterInitOptions = {
 	release: string;
 	environment: string;
 	serverName: string;
+	/**
+	 * Function to allow filtering out errors before they are sent to Sentry.
+	 * Return true if the error should be filtered out.
+	 */
+	beforeSendFilter?: (event: ErrorEvent, hint: EventHint) => boolean;
 };
 
 @Service()
@@ -24,6 +29,8 @@ export class ErrorReporter {
 
 	private report: (error: Error | string, options?: ReportingOptions) => void;
 
+	private beforeSendFilter?: (event: ErrorEvent, hint: EventHint) => boolean;
+
 	constructor(private readonly logger: Logger) {
 		// eslint-disable-next-line @typescript-eslint/unbound-method
 		this.report = this.defaultReport;
@@ -52,7 +59,14 @@ export class ErrorReporter {
 		await close(timeoutInMs);
 	}
 
-	async init({ dsn, serverType, release, environment, serverName }: ErrorReporterInitOptions) {
+	async init({
+		beforeSendFilter,
+		dsn,
+		serverType,
+		release,
+		environment,
+		serverName,
+	}: ErrorReporterInitOptions) {
 		process.on('uncaughtException', (error) => {
 			this.error(error);
 		});
@@ -100,31 +114,34 @@ export class ErrorReporter {
 		setTag('server_type', serverType);
 
 		this.report = (error, options) => captureException(error, options);
+		this.beforeSendFilter = beforeSendFilter;
 	}
 
-	async beforeSend(event: ErrorEvent, { originalException }: EventHint) {
+	async beforeSend(event: ErrorEvent, hint: EventHint) {
+		let { originalException } = hint;
+
 		if (!originalException) return null;
 
 		if (originalException instanceof Promise) {
 			originalException = await originalException.catch((error) => error as Error);
 		}
 
-		if (originalException instanceof AxiosError) return null;
-
 		if (
-			originalException instanceof Error &&
-			originalException.name === 'QueryFailedError' &&
-			['SQLITE_FULL', 'SQLITE_IOERR'].some((errMsg) => originalException.message.includes(errMsg))
+			this.beforeSendFilter?.(event, {
+				...hint,
+				originalException,
+			})
 		) {
 			return null;
 		}
 
-		if (originalException instanceof ApplicationError) {
-			const { level, extra, tags } = originalException;
-			if (level === 'warning') return null;
-			event.level = level;
-			if (extra) event.extra = { ...event.extra, ...extra };
-			if (tags) event.tags = { ...event.tags, ...tags };
+		if (originalException instanceof AxiosError) return null;
+
+		if (this.isIgnoredSqliteError(originalException)) return null;
+		if (this.isApplicationError(originalException)) {
+			if (this.isIgnoredApplicationError(originalException)) return null;
+
+			this.extractEventDetailsFromApplicationError(event, originalException);
 		}
 
 		if (
@@ -166,4 +183,31 @@ export class ErrorReporter {
 		if (typeof e === 'string') return new ApplicationError(e);
 		return;
 	}
+
+	/** @returns true if the error should be filtered out */
+	private isIgnoredSqliteError(error: unknown) {
+		return (
+			error instanceof Error &&
+			error.name === 'QueryFailedError' &&
+			['SQLITE_FULL', 'SQLITE_IOERR'].some((errMsg) => error.message.includes(errMsg))
+		);
+	}
+
+	private isApplicationError(error: unknown): error is ApplicationError {
+		return error instanceof ApplicationError;
+	}
+
+	private isIgnoredApplicationError(error: ApplicationError) {
+		return error.level === 'warning';
+	}
+
+	private extractEventDetailsFromApplicationError(
+		event: ErrorEvent,
+		originalException: ApplicationError,
+	) {
+		const { level, extra, tags } = originalException;
+		event.level = level;
+		if (extra) event.extra = { ...event.extra, ...extra };
+		if (tags) event.tags = { ...event.tags, ...tags };
+	}
 }

packages/core/test/error-reporter.test.ts

@@ -101,6 +101,37 @@ describe('ErrorReporter', () => {
 			const result = await errorReporter.beforeSend(event, { originalException });
 			expect(result).toBeNull();
 		});
+
+		describe('beforeSendFilter', () => {
+			const newErrorReportedWithBeforeSendFilter = (beforeSendFilter: jest.Mock) => {
+				const errorReporter = new ErrorReporter(mock());
+				// @ts-expect-error - beforeSendFilter is private
+				errorReporter.beforeSendFilter = beforeSendFilter;
+				return errorReporter;
+			};
+
+			it('should filter out based on the beforeSendFilter', async () => {
+				const beforeSendFilter = jest.fn().mockReturnValue(true);
+				const errorReporter = newErrorReportedWithBeforeSendFilter(beforeSendFilter);
+				const hint = { originalException: new Error() };
+
+				const result = await errorReporter.beforeSend(event, hint);
+
+				expect(result).toBeNull();
+				expect(beforeSendFilter).toHaveBeenCalledWith(event, hint);
+			});
+
+			it('should not filter out when beforeSendFilter returns false', async () => {
+				const beforeSendFilter = jest.fn().mockReturnValue(false);
+				const errorReporter = newErrorReportedWithBeforeSendFilter(beforeSendFilter);
+				const hint = { originalException: new Error() };
+
+				const result = await errorReporter.beforeSend(event, hint);
+
+				expect(result).toEqual(event);
+				expect(beforeSendFilter).toHaveBeenCalledWith(event, hint);
+			});
+		});
 	});
 
 	describe('error', () => {