From 73145b70b8ca82036d35218a1ce4866e99c43b2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Wed, 11 Dec 2024 15:36:17 +0100 Subject: [PATCH] chore: Convert ErrorReporting to a Service to use DI. Add some tests (no-changelog) (#11279) --- .../cli/src/__tests__/error-reporting.test.ts | 60 ------ .../load-nodes-and-credentials.test.ts | 2 +- packages/cli/src/active-workflow-manager.ts | 18 +- .../collaboration/collaboration.service.ts | 6 +- packages/cli/src/commands/base-command.ts | 15 +- packages/cli/src/commands/execute-batch.ts | 4 +- .../repositories/execution.repository.ts | 11 +- .../repositories/settings.repository.ts | 9 +- .../databases/subscribers/user-subscriber.ts | 9 +- packages/cli/src/db.ts | 9 +- .../decorators/__tests__/on-shutdown.test.ts | 2 +- .../source-control-import.service.ee.ts | 12 +- packages/cli/src/error-reporting.ts | 120 ------------ .../__tests__/save-execution-progress.test.ts | 18 +- .../save-execution-progress.ts | 4 +- packages/cli/src/expression-evaluator.ts | 6 +- .../cli/src/load-nodes-and-credentials.ts | 6 +- packages/cli/src/push/abstract.push.ts | 6 +- packages/cli/src/push/websocket.push.ts | 4 +- packages/cli/src/response-helper.ts | 9 +- .../cli/src/runners/task-runner-module.ts | 6 +- .../__tests__/job-processor.service.test.ts | 9 +- .../scaling/__tests__/scaling.service.test.ts | 1 + packages/cli/src/scaling/job-processor.ts | 12 +- packages/cli/src/scaling/scaling.service.ts | 14 +- .../credentials-tester.service.test.ts | 8 +- .../services/credentials-tester.service.ts | 6 +- .../__tests__/shutdown.service.test.ts | 12 +- packages/cli/src/shutdown/shutdown.service.ts | 11 +- .../src/user-management/email/node-mailer.ts | 5 +- packages/cli/src/webhooks/webhook-helpers.ts | 8 +- .../src/workflow-execute-additional-data.ts | 21 +-- packages/cli/src/workflow-runner.ts | 14 +- .../workflow-execution.service.test.ts | 1 + .../workflows/workflow-execution.service.ts | 10 +- .../workflows/workflow-static-data.service.ts | 6 +- .../active-workflow-manager.test.ts | 2 + .../source-control-import.service.test.ts | 1 + .../task-runner-module.external.test.ts | 2 +- packages/core/package.json | 1 + packages/core/src/ActiveWorkflows.ts | 9 +- packages/core/src/WorkflowExecute.ts | 5 +- packages/core/src/error-reporter.ts | 171 ++++++++++++++++++ packages/core/src/index.ts | 1 + packages/core/test/error-reporter.test.ts | 110 +++++++++++ packages/workflow/src/ErrorReporterProxy.ts | 47 ----- packages/workflow/src/errors/index.ts | 2 +- packages/workflow/src/index.ts | 1 - pnpm-lock.yaml | 3 + 49 files changed, 443 insertions(+), 386 deletions(-) delete mode 100644 packages/cli/src/__tests__/error-reporting.test.ts delete mode 100644 packages/cli/src/error-reporting.ts create mode 100644 packages/core/src/error-reporter.ts create mode 100644 packages/core/test/error-reporter.test.ts delete mode 100644 packages/workflow/src/ErrorReporterProxy.ts diff --git a/packages/cli/src/__tests__/error-reporting.test.ts b/packages/cli/src/__tests__/error-reporting.test.ts deleted file mode 100644 index da23a8cfa8..0000000000 --- a/packages/cli/src/__tests__/error-reporting.test.ts +++ /dev/null @@ -1,60 +0,0 @@ -import { GlobalConfig } from '@n8n/config'; -import type { ClientOptions, ErrorEvent } from '@sentry/types'; -import { strict as assert } from 'node:assert'; -import { Container } from 'typedi'; - -import { InternalServerError } from '@/errors/response-errors/internal-server.error'; - -const init = jest.fn(); - -jest.mock('@sentry/node', () => ({ - init, - setTag: jest.fn(), - captureException: jest.fn(), - Integrations: {}, -})); - -jest.spyOn(process, 'on'); - -describe('initErrorHandling', () => { - let beforeSend: ClientOptions['beforeSend']; - - beforeAll(async () => { - Container.get(GlobalConfig).sentry.backendDsn = 'backend-dsn'; - const errorReporting = require('@/error-reporting'); - await errorReporting.initErrorHandling(); - const options = (init.mock.calls[0] as [ClientOptions])[0]; - beforeSend = options.beforeSend; - }); - - it('ignores errors with level warning', async () => { - const originalException = new InternalServerError('test'); - originalException.level = 'warning'; - - const event = {} as ErrorEvent; - - assert(beforeSend); - expect(await beforeSend(event, { originalException })).toEqual(null); - }); - - it('keeps events with a cause with error level', async () => { - const cause = new Error('cause-error'); - - const originalException = new InternalServerError('test', cause); - const event = {} as ErrorEvent; - - assert(beforeSend); - expect(await beforeSend(event, { originalException })).toEqual(event); - }); - - it('ignores events with error cause with warning level', async () => { - const cause: Error & { level?: 'warning' } = new Error('cause-error'); - cause.level = 'warning'; - - const originalException = new InternalServerError('test', cause); - const event = {} as ErrorEvent; - - assert(beforeSend); - expect(await beforeSend(event, { originalException })).toEqual(null); - }); -}); diff --git a/packages/cli/src/__tests__/load-nodes-and-credentials.test.ts b/packages/cli/src/__tests__/load-nodes-and-credentials.test.ts index bcf485445f..ec8fd06977 100644 --- a/packages/cli/src/__tests__/load-nodes-and-credentials.test.ts +++ b/packages/cli/src/__tests__/load-nodes-and-credentials.test.ts @@ -8,7 +8,7 @@ describe('LoadNodesAndCredentials', () => { let instance: LoadNodesAndCredentials; beforeEach(() => { - instance = new LoadNodesAndCredentials(mock(), mock(), mock()); + instance = new LoadNodesAndCredentials(mock(), mock(), mock(), mock()); instance.loaders.package1 = mock({ directory: '/icons/package1', }); diff --git a/packages/cli/src/active-workflow-manager.ts b/packages/cli/src/active-workflow-manager.ts index 22cc0f5700..1d8d06a857 100644 --- a/packages/cli/src/active-workflow-manager.ts +++ b/packages/cli/src/active-workflow-manager.ts @@ -2,6 +2,7 @@ import { ActiveWorkflows, + ErrorReporter, InstanceSettings, NodeExecuteFunctions, PollContext, @@ -25,7 +26,6 @@ import type { import { Workflow, WorkflowActivationError, - ErrorReporterProxy as ErrorReporter, WebhookPathTakenError, ApplicationError, } from 'n8n-workflow'; @@ -41,10 +41,12 @@ import { import type { WorkflowEntity } from '@/databases/entities/workflow-entity'; import { WorkflowRepository } from '@/databases/repositories/workflow.repository'; import { OnShutdown } from '@/decorators/on-shutdown'; +import { ExecutionService } from '@/executions/execution.service'; import { ExternalHooks } from '@/external-hooks'; import type { IWorkflowDb } from '@/interfaces'; import { Logger } from '@/logging/logger.service'; import { NodeTypes } from '@/node-types'; +import { Publisher } from '@/scaling/pubsub/publisher.service'; import { ActiveWorkflowsService } from '@/services/active-workflows.service'; import { OrchestrationService } from '@/services/orchestration.service'; import * as WebhookHelpers from '@/webhooks/webhook-helpers'; @@ -53,9 +55,6 @@ import * as WorkflowExecuteAdditionalData from '@/workflow-execute-additional-da import { WorkflowExecutionService } from '@/workflows/workflow-execution.service'; import { WorkflowStaticDataService } from '@/workflows/workflow-static-data.service'; -import { ExecutionService } from './executions/execution.service'; -import { Publisher } from './scaling/pubsub/publisher.service'; - interface QueuedActivation { activationMode: WorkflowActivateMode; lastTimeout: number; @@ -69,6 +68,7 @@ export class ActiveWorkflowManager { constructor( private readonly logger: Logger, + private readonly errorReporter: ErrorReporter, private readonly activeWorkflows: ActiveWorkflows, private readonly activeExecutions: ActiveExecutions, private readonly externalHooks: ExternalHooks, @@ -205,7 +205,7 @@ export class ActiveWorkflowManager { try { await this.clearWebhooks(workflow.id); } catch (error1) { - ErrorReporter.error(error1); + this.errorReporter.error(error1); this.logger.error( `Could not remove webhooks of workflow "${workflow.id}" because of error: "${error1.message}"`, ); @@ -439,7 +439,7 @@ export class ActiveWorkflowManager { this.logger.info(' => Started'); } } catch (error) { - ErrorReporter.error(error); + this.errorReporter.error(error); this.logger.info( ' => ERROR: Workflow could not be activated on first try, keep on trying if not an auth issue', ); @@ -635,7 +635,7 @@ export class ActiveWorkflowManager { try { await this.add(workflowId, activationMode, workflowData); } catch (error) { - ErrorReporter.error(error); + this.errorReporter.error(error); let lastTimeout = this.queuedActivations[workflowId].lastTimeout; if (lastTimeout < WORKFLOW_REACTIVATE_MAX_TIMEOUT) { lastTimeout = Math.min(lastTimeout * 2, WORKFLOW_REACTIVATE_MAX_TIMEOUT); @@ -707,7 +707,7 @@ export class ActiveWorkflowManager { try { await this.clearWebhooks(workflowId); } catch (error) { - ErrorReporter.error(error); + this.errorReporter.error(error); this.logger.error( `Could not remove webhooks of workflow "${workflowId}" because of error: "${error.message}"`, ); @@ -724,7 +724,7 @@ export class ActiveWorkflowManager { try { await this.clearWebhooks(workflowId); } catch (error) { - ErrorReporter.error(error); + this.errorReporter.error(error); this.logger.error( `Could not remove webhooks of workflow "${workflowId}" because of error: "${error.message}"`, ); diff --git a/packages/cli/src/collaboration/collaboration.service.ts b/packages/cli/src/collaboration/collaboration.service.ts index cb2ca0d77a..ece93bd5b2 100644 --- a/packages/cli/src/collaboration/collaboration.service.ts +++ b/packages/cli/src/collaboration/collaboration.service.ts @@ -1,6 +1,7 @@ import type { PushPayload } from '@n8n/api-types'; +import { ErrorReporter } from 'n8n-core'; import type { Workflow } from 'n8n-workflow'; -import { ApplicationError, ErrorReporterProxy } from 'n8n-workflow'; +import { ApplicationError } from 'n8n-workflow'; import { Service } from 'typedi'; import { CollaborationState } from '@/collaboration/collaboration.state'; @@ -20,6 +21,7 @@ import { parseWorkflowMessage } from './collaboration.message'; @Service() export class CollaborationService { constructor( + private readonly errorReporter: ErrorReporter, private readonly push: Push, private readonly state: CollaborationState, private readonly userRepository: UserRepository, @@ -31,7 +33,7 @@ export class CollaborationService { try { await this.handleUserMessage(event.userId, event.msg); } catch (error) { - ErrorReporterProxy.error( + this.errorReporter.error( new ApplicationError('Error handling CollaborationService push message', { extra: { msg: event.msg, diff --git a/packages/cli/src/commands/base-command.ts b/packages/cli/src/commands/base-command.ts index b8f15d9f33..33b7a28bf3 100644 --- a/packages/cli/src/commands/base-command.ts +++ b/packages/cli/src/commands/base-command.ts @@ -6,13 +6,9 @@ import { InstanceSettings, ObjectStoreService, DataDeduplicationService, + ErrorReporter, } from 'n8n-core'; -import { - ApplicationError, - ensureError, - ErrorReporterProxy as ErrorReporter, - sleep, -} from 'n8n-workflow'; +import { ApplicationError, ensureError, sleep } from 'n8n-workflow'; import { Container } from 'typedi'; import type { AbstractServer } from '@/abstract-server'; @@ -22,7 +18,6 @@ import * as CrashJournal from '@/crash-journal'; import * as Db from '@/db'; import { getDataDeduplicationService } from '@/deduplication'; import { DeprecationService } from '@/deprecation/deprecation.service'; -import { initErrorHandling } from '@/error-reporting'; import { MessageEventBus } from '@/eventbus/message-event-bus/message-event-bus'; import { TelemetryEventRelay } from '@/events/relays/telemetry.event-relay'; import { initExpressionEvaluator } from '@/expression-evaluator'; @@ -39,6 +34,8 @@ import { WorkflowHistoryManager } from '@/workflows/workflow-history/workflow-hi export abstract class BaseCommand extends Command { protected logger = Container.get(Logger); + protected readonly errorReporter = Container.get(ErrorReporter); + protected externalHooks?: ExternalHooks; protected nodeTypes: NodeTypes; @@ -63,7 +60,7 @@ export abstract class BaseCommand extends Command { protected needsCommunityPackages = false; async init(): Promise { - await initErrorHandling(); + await this.errorReporter.init(); initExpressionEvaluator(); process.once('SIGTERM', this.onTerminationSignal('SIGTERM')); @@ -130,7 +127,7 @@ export abstract class BaseCommand extends Command { } protected async exitWithCrash(message: string, error: unknown) { - ErrorReporter.error(new Error(message, { cause: error }), { level: 'fatal' }); + this.errorReporter.error(new Error(message, { cause: error }), { level: 'fatal' }); await sleep(2000); process.exit(1); } diff --git a/packages/cli/src/commands/execute-batch.ts b/packages/cli/src/commands/execute-batch.ts index a70717c40b..0b19e25652 100644 --- a/packages/cli/src/commands/execute-batch.ts +++ b/packages/cli/src/commands/execute-batch.ts @@ -4,7 +4,7 @@ import fs from 'fs'; import { diff } from 'json-diff'; import pick from 'lodash/pick'; import type { IRun, ITaskData, IWorkflowExecutionDataProcess } from 'n8n-workflow'; -import { ApplicationError, jsonParse, ErrorReporterProxy } from 'n8n-workflow'; +import { ApplicationError, jsonParse } from 'n8n-workflow'; import os from 'os'; import { sep } from 'path'; import { Container } from 'typedi'; @@ -822,7 +822,7 @@ export class ExecuteBatch extends BaseCommand { } } } catch (e) { - ErrorReporterProxy.error(e, { + this.errorReporter.error(e, { extra: { workflowId: workflowData.id, }, diff --git a/packages/cli/src/databases/repositories/execution.repository.ts b/packages/cli/src/databases/repositories/execution.repository.ts index 5bef675a79..fbcb7de445 100644 --- a/packages/cli/src/databases/repositories/execution.repository.ts +++ b/packages/cli/src/databases/repositories/execution.repository.ts @@ -21,12 +21,8 @@ import { import { DateUtils } from '@n8n/typeorm/util/DateUtils'; import { parse, stringify } from 'flatted'; import pick from 'lodash/pick'; -import { BinaryDataService } from 'n8n-core'; -import { - ExecutionCancelledError, - ErrorReporterProxy as ErrorReporter, - ApplicationError, -} from 'n8n-workflow'; +import { BinaryDataService, ErrorReporter } from 'n8n-core'; +import { ExecutionCancelledError, ApplicationError } from 'n8n-workflow'; import type { AnnotationVote, ExecutionStatus, @@ -125,6 +121,7 @@ export class ExecutionRepository extends Repository { dataSource: DataSource, private readonly globalConfig: GlobalConfig, private readonly logger: Logger, + private readonly errorReporter: ErrorReporter, private readonly executionDataRepository: ExecutionDataRepository, private readonly binaryDataService: BinaryDataService, ) { @@ -209,7 +206,7 @@ export class ExecutionRepository extends Repository { reportInvalidExecutions(executions: ExecutionEntity[]) { if (executions.length === 0) return; - ErrorReporter.error( + this.errorReporter.error( new ApplicationError('Found executions without executionData', { extra: { executionIds: executions.map(({ id }) => id) }, }), diff --git a/packages/cli/src/databases/repositories/settings.repository.ts b/packages/cli/src/databases/repositories/settings.repository.ts index 8d87fcffff..aa4410d6d4 100644 --- a/packages/cli/src/databases/repositories/settings.repository.ts +++ b/packages/cli/src/databases/repositories/settings.repository.ts @@ -1,5 +1,5 @@ import { DataSource, Repository } from '@n8n/typeorm'; -import { ErrorReporterProxy as ErrorReporter } from 'n8n-workflow'; +import { ErrorReporter } from 'n8n-core'; import { Service } from 'typedi'; import config from '@/config'; @@ -9,7 +9,10 @@ import { Settings } from '../entities/settings'; @Service() export class SettingsRepository extends Repository { - constructor(dataSource: DataSource) { + constructor( + dataSource: DataSource, + private readonly errorReporter: ErrorReporter, + ) { super(Settings, dataSource.manager); } @@ -49,7 +52,7 @@ export class SettingsRepository extends Repository { config.set(key, value); return { success: true }; } catch (error) { - ErrorReporter.error(error); + this.errorReporter.error(error); } return { success: false }; } diff --git a/packages/cli/src/databases/subscribers/user-subscriber.ts b/packages/cli/src/databases/subscribers/user-subscriber.ts index 2f9e698890..1c55572b14 100644 --- a/packages/cli/src/databases/subscribers/user-subscriber.ts +++ b/packages/cli/src/databases/subscribers/user-subscriber.ts @@ -1,6 +1,7 @@ import type { EntitySubscriberInterface, UpdateEvent } from '@n8n/typeorm'; import { EventSubscriber } from '@n8n/typeorm'; -import { ApplicationError, ErrorReporterProxy } from 'n8n-workflow'; +import { ErrorReporter } from 'n8n-core'; +import { ApplicationError } from 'n8n-workflow'; import { Container } from 'typedi'; import { Logger } from '@/logging/logger.service'; @@ -11,6 +12,8 @@ import { UserRepository } from '../repositories/user.repository'; @EventSubscriber() export class UserSubscriber implements EntitySubscriberInterface { + private readonly eventReporter = Container.get(ErrorReporter); + listenTo() { return User; } @@ -47,7 +50,7 @@ export class UserSubscriber implements EntitySubscriberInterface { const message = "Could not update the personal project's name"; Container.get(Logger).warn(message, event.entity); const exception = new ApplicationError(message); - ErrorReporterProxy.warn(exception, event.entity); + this.eventReporter.warn(exception, event.entity); return; } @@ -69,7 +72,7 @@ export class UserSubscriber implements EntitySubscriberInterface { const message = "Could not update the personal project's name"; Container.get(Logger).warn(message, event.entity); const exception = new ApplicationError(message); - ErrorReporterProxy.warn(exception, event.entity); + this.eventReporter.warn(exception, event.entity); } } } diff --git a/packages/cli/src/db.ts b/packages/cli/src/db.ts index 13147b4106..e1c2b0e402 100644 --- a/packages/cli/src/db.ts +++ b/packages/cli/src/db.ts @@ -2,11 +2,8 @@ import type { EntityManager } from '@n8n/typeorm'; // eslint-disable-next-line n8n-local-rules/misplaced-n8n-typeorm-import import { DataSource as Connection } from '@n8n/typeorm'; -import { - DbConnectionTimeoutError, - ensureError, - ErrorReporterProxy as ErrorReporter, -} from 'n8n-workflow'; +import { ErrorReporter } from 'n8n-core'; +import { DbConnectionTimeoutError, ensureError } from 'n8n-workflow'; import { Container } from 'typedi'; import { inTest } from '@/constants'; @@ -38,7 +35,7 @@ if (!inTest) { connectionState.connected = true; return; } catch (error) { - ErrorReporter.error(error); + Container.get(ErrorReporter).error(error); } finally { pingTimer = setTimeout(pingDBFn, 2000); } diff --git a/packages/cli/src/decorators/__tests__/on-shutdown.test.ts b/packages/cli/src/decorators/__tests__/on-shutdown.test.ts index 28e70dac47..774ae2ef48 100644 --- a/packages/cli/src/decorators/__tests__/on-shutdown.test.ts +++ b/packages/cli/src/decorators/__tests__/on-shutdown.test.ts @@ -8,7 +8,7 @@ describe('OnShutdown', () => { let shutdownService: ShutdownService; beforeEach(() => { - shutdownService = new ShutdownService(mock()); + shutdownService = new ShutdownService(mock(), mock()); Container.set(ShutdownService, shutdownService); jest.spyOn(shutdownService, 'register'); }); diff --git a/packages/cli/src/environments/source-control/source-control-import.service.ee.ts b/packages/cli/src/environments/source-control/source-control-import.service.ee.ts index b5012d2762..b24fe74530 100644 --- a/packages/cli/src/environments/source-control/source-control-import.service.ee.ts +++ b/packages/cli/src/environments/source-control/source-control-import.service.ee.ts @@ -1,13 +1,8 @@ // eslint-disable-next-line n8n-local-rules/misplaced-n8n-typeorm-import import { In } from '@n8n/typeorm'; import glob from 'fast-glob'; -import { Credentials, InstanceSettings } from 'n8n-core'; -import { - ApplicationError, - jsonParse, - ErrorReporterProxy as ErrorReporter, - ensureError, -} from 'n8n-workflow'; +import { Credentials, ErrorReporter, InstanceSettings } from 'n8n-core'; +import { ApplicationError, jsonParse, ensureError } from 'n8n-workflow'; import { readFile as fsReadFile } from 'node:fs/promises'; import path from 'path'; import { Container, Service } from 'typedi'; @@ -56,6 +51,7 @@ export class SourceControlImportService { constructor( private readonly logger: Logger, + private readonly errorReporter: ErrorReporter, private readonly variablesService: VariablesService, private readonly activeWorkflowManager: ActiveWorkflowManager, private readonly tagRepository: TagRepository, @@ -104,7 +100,7 @@ export class SourceControlImportService { if (local.updatedAt instanceof Date) { updatedAt = local.updatedAt; } else { - ErrorReporter.warn('updatedAt is not a Date', { + this.errorReporter.warn('updatedAt is not a Date', { extra: { type: typeof local.updatedAt, value: local.updatedAt, diff --git a/packages/cli/src/error-reporting.ts b/packages/cli/src/error-reporting.ts deleted file mode 100644 index 0214d1d6a1..0000000000 --- a/packages/cli/src/error-reporting.ts +++ /dev/null @@ -1,120 +0,0 @@ -import { GlobalConfig } from '@n8n/config'; -// eslint-disable-next-line n8n-local-rules/misplaced-n8n-typeorm-import -import { QueryFailedError } from '@n8n/typeorm'; -import { AxiosError } from 'axios'; -import { createHash } from 'crypto'; -import { InstanceSettings } from 'n8n-core'; -import { ErrorReporterProxy, ApplicationError } from 'n8n-workflow'; -import Container from 'typedi'; - -let initialized = false; - -export const initErrorHandling = async () => { - if (initialized) return; - - process.on('uncaughtException', (error) => { - ErrorReporterProxy.error(error); - }); - - const dsn = Container.get(GlobalConfig).sentry.backendDsn; - if (!dsn) { - initialized = true; - return; - } - - // Collect longer stacktraces - Error.stackTraceLimit = 50; - - const { - N8N_VERSION: release, - ENVIRONMENT: environment, - DEPLOYMENT_NAME: serverName, - } = process.env; - - const { init, captureException, setTag } = await import('@sentry/node'); - - const { requestDataIntegration, rewriteFramesIntegration } = await import('@sentry/node'); - - const enabledIntegrations = [ - 'InboundFilters', - 'FunctionToString', - 'LinkedErrors', - 'OnUnhandledRejection', - 'ContextLines', - ]; - const seenErrors = new Set(); - - init({ - dsn, - release, - environment, - enableTracing: false, - serverName, - beforeBreadcrumb: () => null, - integrations: (integrations) => [ - ...integrations.filter(({ name }) => enabledIntegrations.includes(name)), - rewriteFramesIntegration({ root: process.cwd() }), - requestDataIntegration({ - include: { - cookies: false, - data: false, - headers: false, - query_string: false, - url: true, - user: false, - }, - }), - ], - async beforeSend(event, { originalException }) { - 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 QueryFailedError && - ['SQLITE_FULL', 'SQLITE_IOERR'].some((errMsg) => originalException.message.includes(errMsg)) - ) { - 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 Error && - 'cause' in originalException && - originalException.cause instanceof Error && - 'level' in originalException.cause && - originalException.cause.level === 'warning' - ) { - // handle underlying errors propagating from dependencies like ai-assistant-sdk - return null; - } - - if (originalException instanceof Error && originalException.stack) { - const eventHash = createHash('sha1').update(originalException.stack).digest('base64'); - if (seenErrors.has(eventHash)) return null; - seenErrors.add(eventHash); - } - - return event; - }, - }); - - setTag('server_type', Container.get(InstanceSettings).instanceType); - - ErrorReporterProxy.init({ - report: (error, options) => captureException(error, options), - }); - - initialized = true; -}; diff --git a/packages/cli/src/execution-lifecycle-hooks/__tests__/save-execution-progress.test.ts b/packages/cli/src/execution-lifecycle-hooks/__tests__/save-execution-progress.test.ts index d89f2fb734..eedbf27c9e 100644 --- a/packages/cli/src/execution-lifecycle-hooks/__tests__/save-execution-progress.test.ts +++ b/packages/cli/src/execution-lifecycle-hooks/__tests__/save-execution-progress.test.ts @@ -1,9 +1,5 @@ -import { - ErrorReporterProxy, - type IRunExecutionData, - type ITaskData, - type IWorkflowBase, -} from 'n8n-workflow'; +import { ErrorReporter } from 'n8n-core'; +import type { IRunExecutionData, ITaskData, IWorkflowBase } from 'n8n-workflow'; import { ExecutionRepository } from '@/databases/repositories/execution.repository'; import { saveExecutionProgress } from '@/execution-lifecycle-hooks/save-execution-progress'; @@ -13,7 +9,7 @@ import { Logger } from '@/logging/logger.service'; import { mockInstance } from '@test/mocking'; mockInstance(Logger); - +const errorReporter = mockInstance(ErrorReporter); const executionRepository = mockInstance(ExecutionRepository); afterEach(() => { @@ -63,8 +59,6 @@ test('should update execution when saving progress is enabled', async () => { progress: true, }); - const reporterSpy = jest.spyOn(ErrorReporterProxy, 'error'); - executionRepository.findSingleExecution.mockResolvedValue({} as IExecutionResponse); await saveExecutionProgress(...commonArgs); @@ -83,7 +77,7 @@ test('should update execution when saving progress is enabled', async () => { status: 'running', }); - expect(reporterSpy).not.toHaveBeenCalled(); + expect(errorReporter.error).not.toHaveBeenCalled(); }); test('should report error on failure', async () => { @@ -92,8 +86,6 @@ test('should report error on failure', async () => { progress: true, }); - const reporterSpy = jest.spyOn(ErrorReporterProxy, 'error'); - const error = new Error('Something went wrong'); executionRepository.findSingleExecution.mockImplementation(() => { @@ -103,5 +95,5 @@ test('should report error on failure', async () => { await saveExecutionProgress(...commonArgs); expect(executionRepository.updateExistingExecution).not.toHaveBeenCalled(); - expect(reporterSpy).toHaveBeenCalledWith(error); + expect(errorReporter.error).toHaveBeenCalledWith(error); }); diff --git a/packages/cli/src/execution-lifecycle-hooks/save-execution-progress.ts b/packages/cli/src/execution-lifecycle-hooks/save-execution-progress.ts index ca9899e1ec..c1de2646c0 100644 --- a/packages/cli/src/execution-lifecycle-hooks/save-execution-progress.ts +++ b/packages/cli/src/execution-lifecycle-hooks/save-execution-progress.ts @@ -1,5 +1,5 @@ +import { ErrorReporter } from 'n8n-core'; import type { IRunExecutionData, ITaskData, IWorkflowBase } from 'n8n-workflow'; -import { ErrorReporterProxy as ErrorReporter } from 'n8n-workflow'; import { Container } from 'typedi'; import { ExecutionRepository } from '@/databases/repositories/execution.repository'; @@ -85,7 +85,7 @@ export async function saveExecutionProgress( } catch (e) { const error = e instanceof Error ? e : new Error(`${e}`); - ErrorReporter.error(error); + Container.get(ErrorReporter).error(error); // TODO: Improve in the future! // Errors here might happen because of database access // For busy machines, we may get "Database is locked" errors. diff --git a/packages/cli/src/expression-evaluator.ts b/packages/cli/src/expression-evaluator.ts index 9a91b4864f..434c78e114 100644 --- a/packages/cli/src/expression-evaluator.ts +++ b/packages/cli/src/expression-evaluator.ts @@ -1,4 +1,6 @@ -import { ErrorReporterProxy, ExpressionEvaluatorProxy } from 'n8n-workflow'; +import { ErrorReporter } from 'n8n-core'; +import { ExpressionEvaluatorProxy } from 'n8n-workflow'; +import Container from 'typedi'; import config from '@/config'; @@ -6,7 +8,7 @@ export const initExpressionEvaluator = () => { ExpressionEvaluatorProxy.setEvaluator(config.getEnv('expression.evaluator')); ExpressionEvaluatorProxy.setDifferEnabled(config.getEnv('expression.reportDifference')); ExpressionEvaluatorProxy.setDiffReporter((expr) => { - ErrorReporterProxy.warn('Expression difference', { + Container.get(ErrorReporter).warn('Expression difference', { extra: { expression: expr, }, diff --git a/packages/cli/src/load-nodes-and-credentials.ts b/packages/cli/src/load-nodes-and-credentials.ts index 0e38affd0f..5a46d6c70d 100644 --- a/packages/cli/src/load-nodes-and-credentials.ts +++ b/packages/cli/src/load-nodes-and-credentials.ts @@ -4,6 +4,7 @@ import fsPromises from 'fs/promises'; import type { Class, DirectoryLoader, Types } from 'n8n-core'; import { CUSTOM_EXTENSION_ENV, + ErrorReporter, InstanceSettings, CustomDirectoryLoader, PackageDirectoryLoader, @@ -22,7 +23,7 @@ import type { INodeType, IVersionedNodeType, } from 'n8n-workflow'; -import { NodeHelpers, ApplicationError, ErrorReporterProxy as ErrorReporter } from 'n8n-workflow'; +import { NodeHelpers, ApplicationError } from 'n8n-workflow'; import path from 'path'; import picocolors from 'picocolors'; import { Container, Service } from 'typedi'; @@ -63,6 +64,7 @@ export class LoadNodesAndCredentials { constructor( private readonly logger: Logger, + private readonly errorReporter: ErrorReporter, private readonly instanceSettings: InstanceSettings, private readonly globalConfig: GlobalConfig, ) {} @@ -155,7 +157,7 @@ export class LoadNodesAndCredentials { ); } catch (error) { this.logger.error((error as Error).message); - ErrorReporter.error(error); + this.errorReporter.error(error); } } } diff --git a/packages/cli/src/push/abstract.push.ts b/packages/cli/src/push/abstract.push.ts index 24cafa8121..f74be9b363 100644 --- a/packages/cli/src/push/abstract.push.ts +++ b/packages/cli/src/push/abstract.push.ts @@ -1,4 +1,5 @@ import type { PushPayload, PushType } from '@n8n/api-types'; +import { ErrorReporter } from 'n8n-core'; import { assert, jsonStringify } from 'n8n-workflow'; import { Service } from 'typedi'; @@ -27,7 +28,10 @@ export abstract class AbstractPush extends TypedEmitter this.pingAll(), 60 * 1000); diff --git a/packages/cli/src/push/websocket.push.ts b/packages/cli/src/push/websocket.push.ts index a2ea39c500..97e45028b2 100644 --- a/packages/cli/src/push/websocket.push.ts +++ b/packages/cli/src/push/websocket.push.ts @@ -1,4 +1,4 @@ -import { ApplicationError, ErrorReporterProxy } from 'n8n-workflow'; +import { ApplicationError } from 'n8n-workflow'; import { Service } from 'typedi'; import type WebSocket from 'ws'; @@ -24,7 +24,7 @@ export class WebSocketPush extends AbstractPush { this.onMessageReceived(pushRef, JSON.parse(buffer.toString('utf8'))); } catch (error) { - ErrorReporterProxy.error( + this.errorReporter.error( new ApplicationError('Error parsing push message', { extra: { userId, diff --git a/packages/cli/src/response-helper.ts b/packages/cli/src/response-helper.ts index 2b993c266c..0e70aa312f 100644 --- a/packages/cli/src/response-helper.ts +++ b/packages/cli/src/response-helper.ts @@ -1,10 +1,7 @@ /* eslint-disable @typescript-eslint/no-unsafe-assignment */ import type { Request, Response } from 'express'; -import { - ErrorReporterProxy as ErrorReporter, - FORM_TRIGGER_PATH_IDENTIFIER, - NodeApiError, -} from 'n8n-workflow'; +import { ErrorReporter } from 'n8n-core'; +import { FORM_TRIGGER_PATH_IDENTIFIER, NodeApiError } from 'n8n-workflow'; import { Readable } from 'node:stream'; import picocolors from 'picocolors'; import Container from 'typedi'; @@ -141,7 +138,7 @@ export const isUniqueConstraintError = (error: Error) => export function reportError(error: Error) { if (!(error instanceof ResponseError) || error.httpStatusCode > 404) { - ErrorReporter.error(error); + Container.get(ErrorReporter).error(error); } } diff --git a/packages/cli/src/runners/task-runner-module.ts b/packages/cli/src/runners/task-runner-module.ts index 97631e0763..434daa066a 100644 --- a/packages/cli/src/runners/task-runner-module.ts +++ b/packages/cli/src/runners/task-runner-module.ts @@ -1,5 +1,6 @@ import { TaskRunnersConfig } from '@n8n/config'; -import { ErrorReporterProxy, sleep } from 'n8n-workflow'; +import { ErrorReporter } from 'n8n-core'; +import { sleep } from 'n8n-workflow'; import * as a from 'node:assert/strict'; import Container, { Service } from 'typedi'; @@ -33,6 +34,7 @@ export class TaskRunnerModule { constructor( private readonly logger: Logger, + private readonly errorReporter: ErrorReporter, private readonly runnerConfig: TaskRunnersConfig, ) { this.logger = this.logger.scoped('task-runner'); @@ -114,7 +116,7 @@ export class TaskRunnerModule { private onRunnerRestartLoopDetected = async (error: TaskRunnerRestartLoopError) => { this.logger.error(error.message); - ErrorReporterProxy.error(error); + this.errorReporter.error(error); // Allow some time for the error to be flushed await sleep(1000); diff --git a/packages/cli/src/scaling/__tests__/job-processor.service.test.ts b/packages/cli/src/scaling/__tests__/job-processor.service.test.ts index 6a3fa5caa4..73264e6382 100644 --- a/packages/cli/src/scaling/__tests__/job-processor.service.test.ts +++ b/packages/cli/src/scaling/__tests__/job-processor.service.test.ts @@ -12,7 +12,14 @@ describe('JobProcessor', () => { executionRepository.findSingleExecution.mockResolvedValue( mock({ status: 'crashed' }), ); - const jobProcessor = new JobProcessor(mock(), executionRepository, mock(), mock(), mock()); + const jobProcessor = new JobProcessor( + mock(), + mock(), + executionRepository, + mock(), + mock(), + mock(), + ); const result = await jobProcessor.processJob(mock()); diff --git a/packages/cli/src/scaling/__tests__/scaling.service.test.ts b/packages/cli/src/scaling/__tests__/scaling.service.test.ts index 0b5f80da48..dbd914fdee 100644 --- a/packages/cli/src/scaling/__tests__/scaling.service.test.ts +++ b/packages/cli/src/scaling/__tests__/scaling.service.test.ts @@ -77,6 +77,7 @@ describe('ScalingService', () => { scalingService = new ScalingService( mockLogger(), mock(), + mock(), jobProcessor, globalConfig, mock(), diff --git a/packages/cli/src/scaling/job-processor.ts b/packages/cli/src/scaling/job-processor.ts index 6bf2524304..51b86c3922 100644 --- a/packages/cli/src/scaling/job-processor.ts +++ b/packages/cli/src/scaling/job-processor.ts @@ -1,12 +1,7 @@ import type { RunningJobSummary } from '@n8n/api-types'; -import { InstanceSettings, WorkflowExecute } from 'n8n-core'; +import { ErrorReporter, InstanceSettings, WorkflowExecute } from 'n8n-core'; import type { ExecutionStatus, IExecuteResponsePromiseData, IRun } from 'n8n-workflow'; -import { - BINARY_ENCODING, - ApplicationError, - Workflow, - ErrorReporterProxy as ErrorReporter, -} from 'n8n-workflow'; +import { BINARY_ENCODING, ApplicationError, Workflow } from 'n8n-workflow'; import type PCancelable from 'p-cancelable'; import { Service } from 'typedi'; @@ -35,6 +30,7 @@ export class JobProcessor { constructor( private readonly logger: Logger, + private readonly errorReporter: ErrorReporter, private readonly executionRepository: ExecutionRepository, private readonly workflowRepository: WorkflowRepository, private readonly nodeTypes: NodeTypes, @@ -155,7 +151,7 @@ export class JobProcessor { workflowExecute = new WorkflowExecute(additionalData, execution.mode, execution.data); workflowRun = workflowExecute.processRunExecutionData(workflow); } else { - ErrorReporter.info(`Worker found execution ${executionId} without data`); + this.errorReporter.info(`Worker found execution ${executionId} without data`); // Execute all nodes // Can execute without webhook so go on workflowExecute = new WorkflowExecute(additionalData, execution.mode); diff --git a/packages/cli/src/scaling/scaling.service.ts b/packages/cli/src/scaling/scaling.service.ts index f7731e26c2..a6ad4cf51f 100644 --- a/packages/cli/src/scaling/scaling.service.ts +++ b/packages/cli/src/scaling/scaling.service.ts @@ -1,13 +1,6 @@ import { GlobalConfig } from '@n8n/config'; -import { InstanceSettings } from 'n8n-core'; -import { - ApplicationError, - BINARY_ENCODING, - sleep, - jsonStringify, - ErrorReporterProxy, - ensureError, -} from 'n8n-workflow'; +import { ErrorReporter, InstanceSettings } from 'n8n-core'; +import { ApplicationError, BINARY_ENCODING, sleep, jsonStringify, ensureError } from 'n8n-workflow'; import type { IExecuteResponsePromiseData } from 'n8n-workflow'; import { strict } from 'node:assert'; import Container, { Service } from 'typedi'; @@ -43,6 +36,7 @@ export class ScalingService { constructor( private readonly logger: Logger, + private readonly errorReporter: ErrorReporter, private readonly activeExecutions: ActiveExecutions, private readonly jobProcessor: JobProcessor, private readonly globalConfig: GlobalConfig, @@ -119,7 +113,7 @@ export class ScalingService { await job.progress(msg); - ErrorReporterProxy.error(error, { executionId }); + this.errorReporter.error(error, { executionId }); throw error; } diff --git a/packages/cli/src/services/__tests__/credentials-tester.service.test.ts b/packages/cli/src/services/__tests__/credentials-tester.service.test.ts index 4da925c532..60c49b7773 100644 --- a/packages/cli/src/services/__tests__/credentials-tester.service.test.ts +++ b/packages/cli/src/services/__tests__/credentials-tester.service.test.ts @@ -8,7 +8,13 @@ import { CredentialsTester } from '@/services/credentials-tester.service'; describe('CredentialsTester', () => { const credentialTypes = mock(); const nodeTypes = mock(); - const credentialsTester = new CredentialsTester(mock(), credentialTypes, nodeTypes, mock()); + const credentialsTester = new CredentialsTester( + mock(), + mock(), + credentialTypes, + nodeTypes, + mock(), + ); beforeEach(() => { jest.clearAllMocks(); diff --git a/packages/cli/src/services/credentials-tester.service.ts b/packages/cli/src/services/credentials-tester.service.ts index 30504e464b..4c3b574d6f 100644 --- a/packages/cli/src/services/credentials-tester.service.ts +++ b/packages/cli/src/services/credentials-tester.service.ts @@ -4,7 +4,7 @@ /* eslint-disable @typescript-eslint/no-unsafe-return */ /* eslint-disable @typescript-eslint/no-unsafe-call */ import get from 'lodash/get'; -import { NodeExecuteFunctions } from 'n8n-core'; +import { ErrorReporter, NodeExecuteFunctions } from 'n8n-core'; import type { ICredentialsDecrypted, ICredentialTestFunction, @@ -28,7 +28,6 @@ import { NodeHelpers, RoutingNode, Workflow, - ErrorReporterProxy as ErrorReporter, ApplicationError, } from 'n8n-workflow'; import { Service } from 'typedi'; @@ -75,6 +74,7 @@ const mockNodeTypes: INodeTypes = { export class CredentialsTester { constructor( private readonly logger: Logger, + private readonly errorReporter: ErrorReporter, private readonly credentialTypes: CredentialTypes, private readonly nodeTypes: NodeTypes, private readonly credentialsHelper: CredentialsHelper, @@ -316,7 +316,7 @@ export class CredentialsTester { credentialsDecrypted, ); } catch (error) { - ErrorReporter.error(error); + this.errorReporter.error(error); // Do not fail any requests to allow custom error messages and // make logic easier if (error.cause?.response) { diff --git a/packages/cli/src/shutdown/__tests__/shutdown.service.test.ts b/packages/cli/src/shutdown/__tests__/shutdown.service.test.ts index 9c2f5b4887..26d6471584 100644 --- a/packages/cli/src/shutdown/__tests__/shutdown.service.test.ts +++ b/packages/cli/src/shutdown/__tests__/shutdown.service.test.ts @@ -1,5 +1,6 @@ import { mock } from 'jest-mock-extended'; -import { ApplicationError, ErrorReporterProxy } from 'n8n-workflow'; +import type { ErrorReporter } from 'n8n-core'; +import { ApplicationError } from 'n8n-workflow'; import Container from 'typedi'; import type { ServiceClass } from '@/shutdown/shutdown.service'; @@ -13,14 +14,13 @@ describe('ShutdownService', () => { let shutdownService: ShutdownService; let mockComponent: MockComponent; let onShutdownSpy: jest.SpyInstance; - let mockErrorReporterProxy: jest.SpyInstance; + const errorReporter = mock(); beforeEach(() => { - shutdownService = new ShutdownService(mock()); + shutdownService = new ShutdownService(mock(), errorReporter); mockComponent = new MockComponent(); Container.set(MockComponent, mockComponent); onShutdownSpy = jest.spyOn(mockComponent, 'onShutdown'); - mockErrorReporterProxy = jest.spyOn(ErrorReporterProxy, 'error').mockImplementation(() => {}); }); describe('shutdown', () => { @@ -83,8 +83,8 @@ describe('ShutdownService', () => { shutdownService.shutdown(); await shutdownService.waitForShutdown(); - expect(mockErrorReporterProxy).toHaveBeenCalledTimes(1); - const error = mockErrorReporterProxy.mock.calls[0][0]; + expect(errorReporter.error).toHaveBeenCalledTimes(1); + const error = errorReporter.error.mock.calls[0][0] as ApplicationError; expect(error).toBeInstanceOf(ApplicationError); expect(error.message).toBe('Failed to shutdown gracefully'); expect(error.extra).toEqual({ diff --git a/packages/cli/src/shutdown/shutdown.service.ts b/packages/cli/src/shutdown/shutdown.service.ts index 1bedc3a7d4..8ff8570757 100644 --- a/packages/cli/src/shutdown/shutdown.service.ts +++ b/packages/cli/src/shutdown/shutdown.service.ts @@ -1,5 +1,5 @@ -import type { Class } from 'n8n-core'; -import { ApplicationError, ErrorReporterProxy, assert } from 'n8n-workflow'; +import { type Class, ErrorReporter } from 'n8n-core'; +import { ApplicationError, assert } from 'n8n-workflow'; import { Container, Service } from 'typedi'; import { LOWEST_SHUTDOWN_PRIORITY, HIGHEST_SHUTDOWN_PRIORITY } from '@/constants'; @@ -31,7 +31,10 @@ export class ShutdownService { private shutdownPromise: Promise | undefined; - constructor(private readonly logger: Logger) {} + constructor( + private readonly logger: Logger, + private readonly errorReporter: ErrorReporter, + ) {} /** Registers given listener to be notified when the application is shutting down */ register(priority: number, handler: ShutdownHandler) { @@ -108,7 +111,7 @@ export class ShutdownService { await method.call(service); } catch (error) { assert(error instanceof Error); - ErrorReporterProxy.error(new ComponentShutdownError(name, error)); + this.errorReporter.error(new ComponentShutdownError(name, error)); } } } diff --git a/packages/cli/src/user-management/email/node-mailer.ts b/packages/cli/src/user-management/email/node-mailer.ts index 661c3fed7f..a35ab77318 100644 --- a/packages/cli/src/user-management/email/node-mailer.ts +++ b/packages/cli/src/user-management/email/node-mailer.ts @@ -1,6 +1,6 @@ import { GlobalConfig } from '@n8n/config'; import { pick } from 'lodash'; -import { ErrorReporterProxy as ErrorReporter } from 'n8n-workflow'; +import { ErrorReporter } from 'n8n-core'; import path from 'node:path'; import type { Transporter } from 'nodemailer'; import { createTransport } from 'nodemailer'; @@ -20,6 +20,7 @@ export class NodeMailer { constructor( globalConfig: GlobalConfig, private readonly logger: Logger, + private readonly errorReporter: ErrorReporter, ) { const smtpConfig = globalConfig.userManagement.emails.smtp; const transportConfig: SMTPConnection.Options = pick(smtpConfig, ['host', 'port', 'secure']); @@ -66,7 +67,7 @@ export class NodeMailer { `Email sent successfully to the following recipients: ${mailData.emailRecipients.toString()}`, ); } catch (error) { - ErrorReporter.error(error); + this.errorReporter.error(error); this.logger.error('Failed to send email', { recipients: mailData.emailRecipients, error: error as Error, diff --git a/packages/cli/src/webhooks/webhook-helpers.ts b/packages/cli/src/webhooks/webhook-helpers.ts index 259142561a..bc38f63fb4 100644 --- a/packages/cli/src/webhooks/webhook-helpers.ts +++ b/packages/cli/src/webhooks/webhook-helpers.ts @@ -9,7 +9,7 @@ import { GlobalConfig } from '@n8n/config'; import type express from 'express'; import get from 'lodash/get'; -import { BinaryDataService, NodeExecuteFunctions } from 'n8n-core'; +import { BinaryDataService, ErrorReporter, NodeExecuteFunctions } from 'n8n-core'; import type { IBinaryData, IBinaryKeyData, @@ -33,8 +33,6 @@ import { ApplicationError, BINARY_ENCODING, createDeferredPromise, - ErrorReporterProxy as ErrorReporter, - ErrorReporterProxy, ExecutionCancelledError, FORM_NODE_TYPE, NodeHelpers, @@ -280,7 +278,7 @@ export async function executeWebhook( errorMessage = err.message; } - ErrorReporterProxy.error(err, { + Container.get(ErrorReporter).error(err, { extra: { nodeName: workflowStartNode.name, nodeType: workflowStartNode.type, @@ -521,7 +519,7 @@ export async function executeWebhook( didSendResponse = true; }) .catch(async (error) => { - ErrorReporter.error(error); + Container.get(ErrorReporter).error(error); Container.get(Logger).error( `Error with Webhook-Response for execution "${executionId}": "${error.message}"`, { executionId, workflowId: workflow.id }, diff --git a/packages/cli/src/workflow-execute-additional-data.ts b/packages/cli/src/workflow-execute-additional-data.ts index 5248dbc65c..a97bb3d3fa 100644 --- a/packages/cli/src/workflow-execute-additional-data.ts +++ b/packages/cli/src/workflow-execute-additional-data.ts @@ -1,19 +1,12 @@ /* eslint-disable @typescript-eslint/no-unsafe-argument */ /* eslint-disable @typescript-eslint/no-use-before-define */ - /* eslint-disable @typescript-eslint/no-unsafe-member-access */ /* eslint-disable @typescript-eslint/no-unsafe-assignment */ import type { PushType } from '@n8n/api-types'; import { GlobalConfig } from '@n8n/config'; import { stringify } from 'flatted'; -import { WorkflowExecute } from 'n8n-core'; -import { - ApplicationError, - ErrorReporterProxy as ErrorReporter, - NodeOperationError, - Workflow, - WorkflowHooks, -} from 'n8n-workflow'; +import { ErrorReporter, WorkflowExecute } from 'n8n-core'; +import { ApplicationError, NodeOperationError, Workflow, WorkflowHooks } from 'n8n-workflow'; import type { IDataObject, IExecuteData, @@ -215,7 +208,7 @@ export function executeErrorWorkflow( ); }) .catch((error: Error) => { - ErrorReporter.error(error); + Container.get(ErrorReporter).error(error); logger.error( `Could not execute ErrorWorkflow for execution ID ${this.executionId} because of error querying the workflow owner`, { @@ -423,7 +416,7 @@ function hookFunctionsSave(): IWorkflowExecuteHooks { newStaticData, ); } catch (e) { - ErrorReporter.error(e); + Container.get(ErrorReporter).error(e); logger.error( `There was a problem saving the workflow with id "${this.workflowData.id}" to save changed staticData: "${e.message}" (hookFunctionsSave)`, { executionId: this.executionId, workflowId: this.workflowData.id }, @@ -502,7 +495,7 @@ function hookFunctionsSave(): IWorkflowExecuteHooks { ); } } catch (error) { - ErrorReporter.error(error); + Container.get(ErrorReporter).error(error); logger.error(`Failed saving execution data to DB on execution ID ${this.executionId}`, { executionId: this.executionId, workflowId: this.workflowData.id, @@ -584,7 +577,7 @@ function hookFunctionsSaveWorker(): IWorkflowExecuteHooks { newStaticData, ); } catch (e) { - ErrorReporter.error(e); + Container.get(ErrorReporter).error(e); logger.error( `There was a problem saving the workflow with id "${this.workflowData.id}" to save changed staticData: "${e.message}" (workflowExecuteAfter)`, { pushRef: this.pushRef, workflowId: this.workflowData.id }, @@ -653,7 +646,7 @@ function hookFunctionsSaveWorker(): IWorkflowExecuteHooks { this.executionId, ]); } catch (error) { - ErrorReporter.error(error); + Container.get(ErrorReporter).error(error); Container.get(Logger).error( 'There was a problem running hook "workflow.postExecute"', error, diff --git a/packages/cli/src/workflow-runner.ts b/packages/cli/src/workflow-runner.ts index 43beee1d05..7dc65e341c 100644 --- a/packages/cli/src/workflow-runner.ts +++ b/packages/cli/src/workflow-runner.ts @@ -5,6 +5,7 @@ import * as a from 'assert/strict'; import { DirectedGraph, + ErrorReporter, InstanceSettings, WorkflowExecute, filterDisabledNodes, @@ -22,11 +23,7 @@ import type { IRunExecutionData, IWorkflowExecuteAdditionalData, } from 'n8n-workflow'; -import { - ErrorReporterProxy as ErrorReporter, - ExecutionCancelledError, - Workflow, -} from 'n8n-workflow'; +import { ExecutionCancelledError, Workflow } from 'n8n-workflow'; import PCancelable from 'p-cancelable'; import { Container, Service } from 'typedi'; @@ -55,6 +52,7 @@ export class WorkflowRunner { constructor( private readonly logger: Logger, + private readonly errorReporter: ErrorReporter, private readonly activeExecutions: ActiveExecutions, private readonly executionRepository: ExecutionRepository, private readonly externalHooks: ExternalHooks, @@ -82,7 +80,7 @@ export class WorkflowRunner { return; } - ErrorReporter.error(error, { executionId }); + this.errorReporter.error(error, { executionId }); const isQueueMode = config.getEnv('executions.mode') === 'queue'; @@ -193,14 +191,14 @@ export class WorkflowRunner { executionId, ]); } catch (error) { - ErrorReporter.error(error); + this.errorReporter.error(error); this.logger.error('There was a problem running hook "workflow.postExecute"', error); } } }) .catch((error) => { if (error instanceof ExecutionCancelledError) return; - ErrorReporter.error(error); + this.errorReporter.error(error); this.logger.error( 'There was a problem running internal hook "onWorkflowPostExecute"', error, diff --git a/packages/cli/src/workflows/__tests__/workflow-execution.service.test.ts b/packages/cli/src/workflows/__tests__/workflow-execution.service.test.ts index 35228dcfd4..3d0bec39de 100644 --- a/packages/cli/src/workflows/__tests__/workflow-execution.service.test.ts +++ b/packages/cli/src/workflows/__tests__/workflow-execution.service.test.ts @@ -57,6 +57,7 @@ describe('WorkflowExecutionService', () => { mock(), mock(), mock(), + mock(), workflowRunner, mock(), mock(), diff --git a/packages/cli/src/workflows/workflow-execution.service.ts b/packages/cli/src/workflows/workflow-execution.service.ts index 1df4af2f76..27b673c245 100644 --- a/packages/cli/src/workflows/workflow-execution.service.ts +++ b/packages/cli/src/workflows/workflow-execution.service.ts @@ -1,4 +1,5 @@ import { GlobalConfig } from '@n8n/config'; +import { ErrorReporter } from 'n8n-core'; import type { IDeferredPromise, IExecuteData, @@ -11,11 +12,7 @@ import type { WorkflowExecuteMode, IWorkflowExecutionDataProcess, } from 'n8n-workflow'; -import { - SubworkflowOperationError, - Workflow, - ErrorReporterProxy as ErrorReporter, -} from 'n8n-workflow'; +import { SubworkflowOperationError, Workflow } from 'n8n-workflow'; import { Service } from 'typedi'; import type { Project } from '@/databases/entities/project'; @@ -36,6 +33,7 @@ import type { WorkflowRequest } from '@/workflows/workflow.request'; export class WorkflowExecutionService { constructor( private readonly logger: Logger, + private readonly errorReporter: ErrorReporter, private readonly executionRepository: ExecutionRepository, private readonly workflowRepository: WorkflowRepository, private readonly nodeTypes: NodeTypes, @@ -293,7 +291,7 @@ export class WorkflowExecutionService { await this.workflowRunner.run(runData); } catch (error) { - ErrorReporter.error(error); + this.errorReporter.error(error); this.logger.error( // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access `Calling Error Workflow for "${workflowErrorData.workflow.id}": "${error.message}"`, diff --git a/packages/cli/src/workflows/workflow-static-data.service.ts b/packages/cli/src/workflows/workflow-static-data.service.ts index 10655b77c7..3e5159dc9a 100644 --- a/packages/cli/src/workflows/workflow-static-data.service.ts +++ b/packages/cli/src/workflows/workflow-static-data.service.ts @@ -1,5 +1,6 @@ import { GlobalConfig } from '@n8n/config'; -import { type IDataObject, type Workflow, ErrorReporterProxy as ErrorReporter } from 'n8n-workflow'; +import { ErrorReporter } from 'n8n-core'; +import type { IDataObject, Workflow } from 'n8n-workflow'; import { Service } from 'typedi'; import { WorkflowRepository } from '@/databases/repositories/workflow.repository'; @@ -11,6 +12,7 @@ export class WorkflowStaticDataService { constructor( private readonly globalConfig: GlobalConfig, private readonly logger: Logger, + private readonly errorReporter: ErrorReporter, private readonly workflowRepository: WorkflowRepository, ) {} @@ -33,7 +35,7 @@ export class WorkflowStaticDataService { await this.saveStaticDataById(workflow.id, workflow.staticData); workflow.staticData.__dataChanged = false; } catch (error) { - ErrorReporter.error(error); + this.errorReporter.error(error); this.logger.error( // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access `There was a problem saving the workflow with id "${workflow.id}" to save changed Data: "${error.message}"`, diff --git a/packages/cli/test/integration/active-workflow-manager.test.ts b/packages/cli/test/integration/active-workflow-manager.test.ts index 24b6b5469a..d61a97ade7 100644 --- a/packages/cli/test/integration/active-workflow-manager.test.ts +++ b/packages/cli/test/integration/active-workflow-manager.test.ts @@ -283,6 +283,7 @@ describe('shouldAddWebhooks', () => { mock(), mock(), mock(), + mock(), mock({ isLeader: true, isFollower: false }), mock(), ); @@ -322,6 +323,7 @@ describe('shouldAddWebhooks', () => { mock(), mock(), mock(), + mock(), mock({ isLeader: false, isFollower: true }), mock(), ); diff --git a/packages/cli/test/integration/environments/source-control-import.service.test.ts b/packages/cli/test/integration/environments/source-control-import.service.test.ts index 78732a99bc..4d2a3d668a 100644 --- a/packages/cli/test/integration/environments/source-control-import.service.test.ts +++ b/packages/cli/test/integration/environments/source-control-import.service.test.ts @@ -30,6 +30,7 @@ describe('SourceControlImportService', () => { mock(), mock(), mock(), + mock(), mock({ n8nFolder: '/some-path' }), ); diff --git a/packages/cli/test/integration/runners/task-runner-module.external.test.ts b/packages/cli/test/integration/runners/task-runner-module.external.test.ts index 8e872c25ec..bb61dae6d4 100644 --- a/packages/cli/test/integration/runners/task-runner-module.external.test.ts +++ b/packages/cli/test/integration/runners/task-runner-module.external.test.ts @@ -33,7 +33,7 @@ describe('TaskRunnerModule in external mode', () => { runnerConfig.enabled = true; runnerConfig.authToken = ''; - const module = new TaskRunnerModule(mock(), runnerConfig); + const module = new TaskRunnerModule(mock(), mock(), runnerConfig); await expect(module.start()).rejects.toThrowError(MissingAuthTokenError); }); diff --git a/packages/core/package.json b/packages/core/package.json index e0bfe88e6d..a68592fed4 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -39,6 +39,7 @@ "@langchain/core": "catalog:", "@n8n/client-oauth2": "workspace:*", "@n8n/config": "workspace:*", + "@sentry/node": "catalog:", "aws4": "1.11.0", "axios": "catalog:", "concat-stream": "2.0.0", diff --git a/packages/core/src/ActiveWorkflows.ts b/packages/core/src/ActiveWorkflows.ts index 93e67488d5..de919b0649 100644 --- a/packages/core/src/ActiveWorkflows.ts +++ b/packages/core/src/ActiveWorkflows.ts @@ -11,7 +11,6 @@ import type { } from 'n8n-workflow'; import { ApplicationError, - ErrorReporterProxy as ErrorReporter, LoggerProxy as Logger, toCronExpression, TriggerCloseError, @@ -20,12 +19,16 @@ import { } from 'n8n-workflow'; import { Service } from 'typedi'; +import { ErrorReporter } from './error-reporter'; import type { IWorkflowData } from './Interfaces'; import { ScheduledTaskManager } from './ScheduledTaskManager'; @Service() export class ActiveWorkflows { - constructor(private readonly scheduledTaskManager: ScheduledTaskManager) {} + constructor( + private readonly scheduledTaskManager: ScheduledTaskManager, + private readonly errorReporter: ErrorReporter, + ) {} private activeWorkflows: { [workflowId: string]: IWorkflowData } = {}; @@ -218,7 +221,7 @@ export class ActiveWorkflows { Logger.error( `There was a problem calling "closeFunction" on "${e.node.name}" in workflow "${workflowId}"`, ); - ErrorReporter.error(e, { extra: { workflowId } }); + this.errorReporter.error(e, { extra: { workflowId } }); return; } diff --git a/packages/core/src/WorkflowExecute.ts b/packages/core/src/WorkflowExecute.ts index 27244fafc3..215119922e 100644 --- a/packages/core/src/WorkflowExecute.ts +++ b/packages/core/src/WorkflowExecute.ts @@ -46,11 +46,12 @@ import { ApplicationError, NodeExecutionOutput, sleep, - ErrorReporterProxy, ExecutionCancelledError, } from 'n8n-workflow'; import PCancelable from 'p-cancelable'; +import Container from 'typedi'; +import { ErrorReporter } from './error-reporter'; import * as NodeExecuteFunctions from './NodeExecuteFunctions'; import { DirectedGraph, @@ -1428,7 +1429,7 @@ export class WorkflowExecute { toReport = error; } if (toReport) { - ErrorReporterProxy.error(toReport, { + Container.get(ErrorReporter).error(toReport, { extra: { nodeName: executionNode.name, nodeType: executionNode.type, diff --git a/packages/core/src/error-reporter.ts b/packages/core/src/error-reporter.ts new file mode 100644 index 0000000000..10e58b1f45 --- /dev/null +++ b/packages/core/src/error-reporter.ts @@ -0,0 +1,171 @@ +import { GlobalConfig } from '@n8n/config'; +import type { NodeOptions } from '@sentry/node'; +import type { ErrorEvent, EventHint } from '@sentry/types'; +import { AxiosError } from 'axios'; +import { ApplicationError, LoggerProxy, type ReportingOptions } from 'n8n-workflow'; +import { createHash } from 'node:crypto'; +import { Service } from 'typedi'; + +import { InstanceSettings } from './InstanceSettings'; + +@Service() +export class ErrorReporter { + private initialized = false; + + /** Hashes of error stack traces, to deduplicate error reports. */ + private seenErrors = new Set(); + + private report: (error: Error | string, options?: ReportingOptions) => void; + + constructor( + private readonly globalConfig: GlobalConfig, + private readonly instanceSettings: InstanceSettings, + ) { + // eslint-disable-next-line @typescript-eslint/unbound-method + this.report = this.defaultReport; + } + + private defaultReport(error: Error | string, options?: ReportingOptions) { + if (error instanceof Error) { + let e = error; + + const { executionId } = options ?? {}; + const context = executionId ? ` (execution ${executionId})` : ''; + + do { + const msg = [e.message + context, e.stack ? `\n${e.stack}\n` : ''].join(''); + const meta = e instanceof ApplicationError ? e.extra : undefined; + LoggerProxy.error(msg, meta); + e = e.cause as Error; + } while (e); + } + } + + async init() { + if (this.initialized) return; + + process.on('uncaughtException', (error) => { + this.error(error); + }); + + const dsn = this.globalConfig.sentry.backendDsn; + if (!dsn) { + this.initialized = true; + return; + } + + // Collect longer stacktraces + Error.stackTraceLimit = 50; + + const { + N8N_VERSION: release, + ENVIRONMENT: environment, + DEPLOYMENT_NAME: serverName, + } = process.env; + + const { init, captureException, setTag } = await import('@sentry/node'); + const { requestDataIntegration, rewriteFramesIntegration } = await import('@sentry/node'); + + const enabledIntegrations = [ + 'InboundFilters', + 'FunctionToString', + 'LinkedErrors', + 'OnUnhandledRejection', + 'ContextLines', + ]; + + init({ + dsn, + release, + environment, + enableTracing: false, + serverName, + beforeBreadcrumb: () => null, + beforeSend: this.beforeSend.bind(this) as NodeOptions['beforeSend'], + integrations: (integrations) => [ + ...integrations.filter(({ name }) => enabledIntegrations.includes(name)), + rewriteFramesIntegration({ root: process.cwd() }), + requestDataIntegration({ + include: { + cookies: false, + data: false, + headers: false, + query_string: false, + url: true, + user: false, + }, + }), + ], + }); + + setTag('server_type', this.instanceSettings.instanceType); + + this.report = (error, options) => captureException(error, options); + + this.initialized = true; + } + + async beforeSend(event: ErrorEvent, { originalException }: EventHint) { + 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)) + ) { + 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 Error && + 'cause' in originalException && + originalException.cause instanceof Error && + 'level' in originalException.cause && + originalException.cause.level === 'warning' + ) { + // handle underlying errors propagating from dependencies like ai-assistant-sdk + return null; + } + + if (originalException instanceof Error && originalException.stack) { + const eventHash = createHash('sha1').update(originalException.stack).digest('base64'); + if (this.seenErrors.has(eventHash)) return null; + this.seenErrors.add(eventHash); + } + + return event; + } + + error(e: unknown, options?: ReportingOptions) { + const toReport = this.wrap(e); + if (toReport) this.report(toReport, options); + } + + warn(warning: Error | string, options?: ReportingOptions) { + this.error(warning, { ...options, level: 'warning' }); + } + + info(msg: string, options?: ReportingOptions) { + this.report(msg, { ...options, level: 'info' }); + } + + private wrap(e: unknown) { + if (e instanceof Error) return e; + if (typeof e === 'string') return new ApplicationError(e); + return; + } +} diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 4b3e479018..51ddd32678 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -21,3 +21,4 @@ export { isStoredMode as isValidNonDefaultMode } from './BinaryData/utils'; export * from './ExecutionMetadata'; export * from './node-execution-context'; export * from './PartialExecutionUtils'; +export { ErrorReporter } from './error-reporter'; diff --git a/packages/core/test/error-reporter.test.ts b/packages/core/test/error-reporter.test.ts new file mode 100644 index 0000000000..b1ad419bb4 --- /dev/null +++ b/packages/core/test/error-reporter.test.ts @@ -0,0 +1,110 @@ +import type { GlobalConfig } from '@n8n/config'; +import { QueryFailedError } from '@n8n/typeorm'; +import type { ErrorEvent } from '@sentry/types'; +import { AxiosError } from 'axios'; +import { mock } from 'jest-mock-extended'; +import { ApplicationError } from 'n8n-workflow'; + +import { ErrorReporter } from '@/error-reporter'; +import type { InstanceSettings } from '@/InstanceSettings'; + +const init = jest.fn(); + +jest.mock('@sentry/node', () => ({ + init, + setTag: jest.fn(), + captureException: jest.fn(), + Integrations: {}, +})); + +jest.spyOn(process, 'on'); + +describe('ErrorReporter', () => { + const globalConfig = mock(); + const instanceSettings = mock(); + const errorReporting = new ErrorReporter(globalConfig, instanceSettings); + const event = {} as ErrorEvent; + + describe('beforeSend', () => { + it('should ignore errors with level warning', async () => { + const originalException = new ApplicationError('test'); + originalException.level = 'warning'; + + expect(await errorReporting.beforeSend(event, { originalException })).toEqual(null); + }); + + it('should keep events with a cause with error level', async () => { + const cause = new Error('cause-error'); + const originalException = new ApplicationError('test', cause); + + expect(await errorReporting.beforeSend(event, { originalException })).toEqual(event); + }); + + it('should ignore events with error cause with warning level', async () => { + const cause: Error & { level?: 'warning' } = new Error('cause-error'); + cause.level = 'warning'; + const originalException = new ApplicationError('test', cause); + + expect(await errorReporting.beforeSend(event, { originalException })).toEqual(null); + }); + + it('should set level, extra, and tags from ApplicationError', async () => { + const originalException = new ApplicationError('Test error', { + level: 'error', + extra: { foo: 'bar' }, + tags: { tag1: 'value1' }, + }); + + const testEvent = {} as ErrorEvent; + + const result = await errorReporting.beforeSend(testEvent, { originalException }); + + expect(result).toEqual({ + level: 'error', + extra: { foo: 'bar' }, + tags: { tag1: 'value1' }, + }); + }); + + it('should deduplicate errors with same stack trace', async () => { + const originalException = new Error(); + + const firstResult = await errorReporting.beforeSend(event, { originalException }); + expect(firstResult).toEqual(event); + + const secondResult = await errorReporting.beforeSend(event, { originalException }); + expect(secondResult).toBeNull(); + }); + + it('should handle Promise rejections', async () => { + const originalException = Promise.reject(new Error()); + + const result = await errorReporting.beforeSend(event, { originalException }); + + expect(result).toEqual(event); + }); + + test.each([ + ['undefined', undefined], + ['null', null], + ['an AxiosError', new AxiosError()], + ['a rejected Promise with AxiosError', Promise.reject(new AxiosError())], + [ + 'a QueryFailedError with SQLITE_FULL', + new QueryFailedError('', [], new Error('SQLITE_FULL')), + ], + [ + 'a QueryFailedError with SQLITE_IOERR', + new QueryFailedError('', [], new Error('SQLITE_IOERR')), + ], + ['an ApplicationError with "warning" level', new ApplicationError('', { level: 'warning' })], + [ + 'an Error with ApplicationError as cause with "warning" level', + new Error('', { cause: new ApplicationError('', { level: 'warning' }) }), + ], + ])('should ignore if originalException is %s', async (_, originalException) => { + const result = await errorReporting.beforeSend(event, { originalException }); + expect(result).toBeNull(); + }); + }); +}); diff --git a/packages/workflow/src/ErrorReporterProxy.ts b/packages/workflow/src/ErrorReporterProxy.ts deleted file mode 100644 index cedb921d5e..0000000000 --- a/packages/workflow/src/ErrorReporterProxy.ts +++ /dev/null @@ -1,47 +0,0 @@ -import { ApplicationError, type ReportingOptions } from './errors/application.error'; -import * as Logger from './LoggerProxy'; - -interface ErrorReporter { - report: (error: Error | string, options?: ReportingOptions) => void; -} - -const instance: ErrorReporter = { - report: (error, options) => { - if (error instanceof Error) { - let e = error; - - const { executionId } = options ?? {}; - const context = executionId ? ` (execution ${executionId})` : ''; - - do { - const msg = [e.message + context, e.stack ? `\n${e.stack}\n` : ''].join(''); - const meta = e instanceof ApplicationError ? e.extra : undefined; - Logger.error(msg, meta); - e = e.cause as Error; - } while (e); - } - }, -}; - -export function init(errorReporter: ErrorReporter) { - instance.report = errorReporter.report; -} - -const wrap = (e: unknown) => { - if (e instanceof Error) return e; - if (typeof e === 'string') return new ApplicationError(e); - return; -}; - -export const error = (e: unknown, options?: ReportingOptions) => { - const toReport = wrap(e); - if (toReport) instance.report(toReport, options); -}; - -export const info = (msg: string, options?: ReportingOptions) => { - Logger.info(msg); - instance.report(msg, { ...options, level: 'info' }); -}; - -export const warn = (warning: Error | string, options?: ReportingOptions) => - error(warning, { ...options, level: 'warning' }); diff --git a/packages/workflow/src/errors/index.ts b/packages/workflow/src/errors/index.ts index 5dea5b9e6d..593b2687ef 100644 --- a/packages/workflow/src/errors/index.ts +++ b/packages/workflow/src/errors/index.ts @@ -1,4 +1,4 @@ -export { ApplicationError } from './application.error'; +export { ApplicationError, ReportingOptions } from './application.error'; export { ExpressionError } from './expression.error'; export { CredentialAccessError } from './credential-access-error'; export { ExecutionCancelledError } from './execution-cancelled.error'; diff --git a/packages/workflow/src/index.ts b/packages/workflow/src/index.ts index 8c304664f1..6a7630722c 100644 --- a/packages/workflow/src/index.ts +++ b/packages/workflow/src/index.ts @@ -1,5 +1,4 @@ import * as LoggerProxy from './LoggerProxy'; -export * as ErrorReporterProxy from './ErrorReporterProxy'; export * as ExpressionEvaluatorProxy from './ExpressionEvaluatorProxy'; import * as NodeHelpers from './NodeHelpers'; import * as ObservableObject from './ObservableObject'; diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 755f16f0c0..301206462c 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1121,6 +1121,9 @@ importers: '@n8n/config': specifier: workspace:* version: link:../@n8n/config + '@sentry/node': + specifier: 'catalog:' + version: 8.42.0 aws4: specifier: 1.11.0 version: 1.11.0