Skip to content

Commit

Permalink
fix(core): Filter out task runner errors originating from user code f…
Browse files Browse the repository at this point in the history
…rom sentry (no-changelog)
  • Loading branch information
tomi committed Jan 9, 2025
1 parent 6711cbc commit 32ea4ac
Show file tree
Hide file tree
Showing 5 changed files with 330 additions and 36 deletions.
178 changes: 178 additions & 0 deletions packages/@n8n/task-runner/src/__tests__/task-runner-sentry.test.ts
Original file line number Diff line number Diff line change
@@ -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();
});
});
});
26 changes: 7 additions & 19 deletions packages/@n8n/task-runner/src/start.ts
Original file line number Diff line number Diff line change
@@ -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() {
Expand All @@ -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);
Expand All @@ -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', () => {
Expand Down
63 changes: 63 additions & 0 deletions packages/@n8n/task-runner/src/task-runner-sentry.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
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() {
if (!this.config.dsn) return;

await this.errorReporter.init({
serverType: 'task_runner',
dsn: this.config.dsn,
release: this.config.n8nVersion,
environment: this.config.environment,
serverName: this.config.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];
if (!error) return false;

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) {
if (!error) return false;

const frames = error.stacktrace?.frames;
if (!frames) return false;

return frames.some(
(frame) => frame.filename === 'node:vm' && frame.function === 'runInContext',
);
}
}
Loading

0 comments on commit 32ea4ac

Please sign in to comment.