Skip to content

Commit 73145b7

Browse files
netroynetroy
authored
chore: Convert ErrorReporting to a Service to use DI. Add some tests (no-changelog) (#11279)
1 parent 5300e0a commit 73145b7

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

49 files changed

+443
-386
lines changed

‎packages/cli/src/__tests__/error-reporting.test.ts

-60
This file was deleted.

‎packages/cli/src/__tests__/load-nodes-and-credentials.test.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ describe('LoadNodesAndCredentials', () => {
88
let instance: LoadNodesAndCredentials;
99

1010
beforeEach(() => {
11-
instance = new LoadNodesAndCredentials(mock(), mock(), mock());
11+
instance = new LoadNodesAndCredentials(mock(), mock(), mock(), mock());
1212
instance.loaders.package1 = mock<DirectoryLoader>({
1313
directory: '/icons/package1',
1414
});

‎packages/cli/src/active-workflow-manager.ts

+9-9
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import {
44
ActiveWorkflows,
5+
ErrorReporter,
56
InstanceSettings,
67
NodeExecuteFunctions,
78
PollContext,
@@ -25,7 +26,6 @@ import type {
2526
import {
2627
Workflow,
2728
WorkflowActivationError,
28-
ErrorReporterProxy as ErrorReporter,
2929
WebhookPathTakenError,
3030
ApplicationError,
3131
} from 'n8n-workflow';
@@ -41,10 +41,12 @@ import {
4141
import type { WorkflowEntity } from '@/databases/entities/workflow-entity';
4242
import { WorkflowRepository } from '@/databases/repositories/workflow.repository';
4343
import { OnShutdown } from '@/decorators/on-shutdown';
44+
import { ExecutionService } from '@/executions/execution.service';
4445
import { ExternalHooks } from '@/external-hooks';
4546
import type { IWorkflowDb } from '@/interfaces';
4647
import { Logger } from '@/logging/logger.service';
4748
import { NodeTypes } from '@/node-types';
49+
import { Publisher } from '@/scaling/pubsub/publisher.service';
4850
import { ActiveWorkflowsService } from '@/services/active-workflows.service';
4951
import { OrchestrationService } from '@/services/orchestration.service';
5052
import * as WebhookHelpers from '@/webhooks/webhook-helpers';
@@ -53,9 +55,6 @@ import * as WorkflowExecuteAdditionalData from '@/workflow-execute-additional-da
5355
import { WorkflowExecutionService } from '@/workflows/workflow-execution.service';
5456
import { WorkflowStaticDataService } from '@/workflows/workflow-static-data.service';
5557

56-
import { ExecutionService } from './executions/execution.service';
57-
import { Publisher } from './scaling/pubsub/publisher.service';
58-
5958
interface QueuedActivation {
6059
activationMode: WorkflowActivateMode;
6160
lastTimeout: number;
@@ -69,6 +68,7 @@ export class ActiveWorkflowManager {
6968

7069
constructor(
7170
private readonly logger: Logger,
71+
private readonly errorReporter: ErrorReporter,
7272
private readonly activeWorkflows: ActiveWorkflows,
7373
private readonly activeExecutions: ActiveExecutions,
7474
private readonly externalHooks: ExternalHooks,
@@ -205,7 +205,7 @@ export class ActiveWorkflowManager {
205205
try {
206206
await this.clearWebhooks(workflow.id);
207207
} catch (error1) {
208-
ErrorReporter.error(error1);
208+
this.errorReporter.error(error1);
209209
this.logger.error(
210210
`Could not remove webhooks of workflow "${workflow.id}" because of error: "${error1.message}"`,
211211
);
@@ -439,7 +439,7 @@ export class ActiveWorkflowManager {
439439
this.logger.info(' => Started');
440440
}
441441
} catch (error) {
442-
ErrorReporter.error(error);
442+
this.errorReporter.error(error);
443443
this.logger.info(
444444
' => ERROR: Workflow could not be activated on first try, keep on trying if not an auth issue',
445445
);
@@ -635,7 +635,7 @@ export class ActiveWorkflowManager {
635635
try {
636636
await this.add(workflowId, activationMode, workflowData);
637637
} catch (error) {
638-
ErrorReporter.error(error);
638+
this.errorReporter.error(error);
639639
let lastTimeout = this.queuedActivations[workflowId].lastTimeout;
640640
if (lastTimeout < WORKFLOW_REACTIVATE_MAX_TIMEOUT) {
641641
lastTimeout = Math.min(lastTimeout * 2, WORKFLOW_REACTIVATE_MAX_TIMEOUT);
@@ -707,7 +707,7 @@ export class ActiveWorkflowManager {
707707
try {
708708
await this.clearWebhooks(workflowId);
709709
} catch (error) {
710-
ErrorReporter.error(error);
710+
this.errorReporter.error(error);
711711
this.logger.error(
712712
`Could not remove webhooks of workflow "${workflowId}" because of error: "${error.message}"`,
713713
);
@@ -724,7 +724,7 @@ export class ActiveWorkflowManager {
724724
try {
725725
await this.clearWebhooks(workflowId);
726726
} catch (error) {
727-
ErrorReporter.error(error);
727+
this.errorReporter.error(error);
728728
this.logger.error(
729729
`Could not remove webhooks of workflow "${workflowId}" because of error: "${error.message}"`,
730730
);

‎packages/cli/src/collaboration/collaboration.service.ts

+4-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import type { PushPayload } from '@n8n/api-types';
2+
import { ErrorReporter } from 'n8n-core';
23
import type { Workflow } from 'n8n-workflow';
3-
import { ApplicationError, ErrorReporterProxy } from 'n8n-workflow';
4+
import { ApplicationError } from 'n8n-workflow';
45
import { Service } from 'typedi';
56

67
import { CollaborationState } from '@/collaboration/collaboration.state';
@@ -20,6 +21,7 @@ import { parseWorkflowMessage } from './collaboration.message';
2021
@Service()
2122
export class CollaborationService {
2223
constructor(
24+
private readonly errorReporter: ErrorReporter,
2325
private readonly push: Push,
2426
private readonly state: CollaborationState,
2527
private readonly userRepository: UserRepository,
@@ -31,7 +33,7 @@ export class CollaborationService {
3133
try {
3234
await this.handleUserMessage(event.userId, event.msg);
3335
} catch (error) {
34-
ErrorReporterProxy.error(
36+
this.errorReporter.error(
3537
new ApplicationError('Error handling CollaborationService push message', {
3638
extra: {
3739
msg: event.msg,

‎packages/cli/src/commands/base-command.ts

+6-9
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,9 @@ import {
66
InstanceSettings,
77
ObjectStoreService,
88
DataDeduplicationService,
9+
ErrorReporter,
910
} from 'n8n-core';
10-
import {
11-
ApplicationError,
12-
ensureError,
13-
ErrorReporterProxy as ErrorReporter,
14-
sleep,
15-
} from 'n8n-workflow';
11+
import { ApplicationError, ensureError, sleep } from 'n8n-workflow';
1612
import { Container } from 'typedi';
1713

1814
import type { AbstractServer } from '@/abstract-server';
@@ -22,7 +18,6 @@ import * as CrashJournal from '@/crash-journal';
2218
import * as Db from '@/db';
2319
import { getDataDeduplicationService } from '@/deduplication';
2420
import { DeprecationService } from '@/deprecation/deprecation.service';
25-
import { initErrorHandling } from '@/error-reporting';
2621
import { MessageEventBus } from '@/eventbus/message-event-bus/message-event-bus';
2722
import { TelemetryEventRelay } from '@/events/relays/telemetry.event-relay';
2823
import { initExpressionEvaluator } from '@/expression-evaluator';
@@ -39,6 +34,8 @@ import { WorkflowHistoryManager } from '@/workflows/workflow-history/workflow-hi
3934
export abstract class BaseCommand extends Command {
4035
protected logger = Container.get(Logger);
4136

37+
protected readonly errorReporter = Container.get(ErrorReporter);
38+
4239
protected externalHooks?: ExternalHooks;
4340

4441
protected nodeTypes: NodeTypes;
@@ -63,7 +60,7 @@ export abstract class BaseCommand extends Command {
6360
protected needsCommunityPackages = false;
6461

6562
async init(): Promise<void> {
66-
await initErrorHandling();
63+
await this.errorReporter.init();
6764
initExpressionEvaluator();
6865

6966
process.once('SIGTERM', this.onTerminationSignal('SIGTERM'));
@@ -130,7 +127,7 @@ export abstract class BaseCommand extends Command {
130127
}
131128

132129
protected async exitWithCrash(message: string, error: unknown) {
133-
ErrorReporter.error(new Error(message, { cause: error }), { level: 'fatal' });
130+
this.errorReporter.error(new Error(message, { cause: error }), { level: 'fatal' });
134131
await sleep(2000);
135132
process.exit(1);
136133
}

‎packages/cli/src/commands/execute-batch.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import fs from 'fs';
44
import { diff } from 'json-diff';
55
import pick from 'lodash/pick';
66
import type { IRun, ITaskData, IWorkflowExecutionDataProcess } from 'n8n-workflow';
7-
import { ApplicationError, jsonParse, ErrorReporterProxy } from 'n8n-workflow';
7+
import { ApplicationError, jsonParse } from 'n8n-workflow';
88
import os from 'os';
99
import { sep } from 'path';
1010
import { Container } from 'typedi';
@@ -822,7 +822,7 @@ export class ExecuteBatch extends BaseCommand {
822822
}
823823
}
824824
} catch (e) {
825-
ErrorReporterProxy.error(e, {
825+
this.errorReporter.error(e, {
826826
extra: {
827827
workflowId: workflowData.id,
828828
},

‎packages/cli/src/databases/repositories/execution.repository.ts

+4-7
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,8 @@ import {
2121
import { DateUtils } from '@n8n/typeorm/util/DateUtils';
2222
import { parse, stringify } from 'flatted';
2323
import pick from 'lodash/pick';
24-
import { BinaryDataService } from 'n8n-core';
25-
import {
26-
ExecutionCancelledError,
27-
ErrorReporterProxy as ErrorReporter,
28-
ApplicationError,
29-
} from 'n8n-workflow';
24+
import { BinaryDataService, ErrorReporter } from 'n8n-core';
25+
import { ExecutionCancelledError, ApplicationError } from 'n8n-workflow';
3026
import type {
3127
AnnotationVote,
3228
ExecutionStatus,
@@ -125,6 +121,7 @@ export class ExecutionRepository extends Repository<ExecutionEntity> {
125121
dataSource: DataSource,
126122
private readonly globalConfig: GlobalConfig,
127123
private readonly logger: Logger,
124+
private readonly errorReporter: ErrorReporter,
128125
private readonly executionDataRepository: ExecutionDataRepository,
129126
private readonly binaryDataService: BinaryDataService,
130127
) {
@@ -209,7 +206,7 @@ export class ExecutionRepository extends Repository<ExecutionEntity> {
209206
reportInvalidExecutions(executions: ExecutionEntity[]) {
210207
if (executions.length === 0) return;
211208

212-
ErrorReporter.error(
209+
this.errorReporter.error(
213210
new ApplicationError('Found executions without executionData', {
214211
extra: { executionIds: executions.map(({ id }) => id) },
215212
}),

‎packages/cli/src/databases/repositories/settings.repository.ts

+6-3
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { DataSource, Repository } from '@n8n/typeorm';
2-
import { ErrorReporterProxy as ErrorReporter } from 'n8n-workflow';
2+
import { ErrorReporter } from 'n8n-core';
33
import { Service } from 'typedi';
44

55
import config from '@/config';
@@ -9,7 +9,10 @@ import { Settings } from '../entities/settings';
99

1010
@Service()
1111
export class SettingsRepository extends Repository<Settings> {
12-
constructor(dataSource: DataSource) {
12+
constructor(
13+
dataSource: DataSource,
14+
private readonly errorReporter: ErrorReporter,
15+
) {
1316
super(Settings, dataSource.manager);
1417
}
1518

@@ -49,7 +52,7 @@ export class SettingsRepository extends Repository<Settings> {
4952
config.set(key, value);
5053
return { success: true };
5154
} catch (error) {
52-
ErrorReporter.error(error);
55+
this.errorReporter.error(error);
5356
}
5457
return { success: false };
5558
}

‎packages/cli/src/databases/subscribers/user-subscriber.ts

+6-3
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import type { EntitySubscriberInterface, UpdateEvent } from '@n8n/typeorm';
22
import { EventSubscriber } from '@n8n/typeorm';
3-
import { ApplicationError, ErrorReporterProxy } from 'n8n-workflow';
3+
import { ErrorReporter } from 'n8n-core';
4+
import { ApplicationError } from 'n8n-workflow';
45
import { Container } from 'typedi';
56

67
import { Logger } from '@/logging/logger.service';
@@ -11,6 +12,8 @@ import { UserRepository } from '../repositories/user.repository';
1112

1213
@EventSubscriber()
1314
export class UserSubscriber implements EntitySubscriberInterface<User> {
15+
private readonly eventReporter = Container.get(ErrorReporter);
16+
1417
listenTo() {
1518
return User;
1619
}
@@ -47,7 +50,7 @@ export class UserSubscriber implements EntitySubscriberInterface<User> {
4750
const message = "Could not update the personal project's name";
4851
Container.get(Logger).warn(message, event.entity);
4952
const exception = new ApplicationError(message);
50-
ErrorReporterProxy.warn(exception, event.entity);
53+
this.eventReporter.warn(exception, event.entity);
5154
return;
5255
}
5356

@@ -69,7 +72,7 @@ export class UserSubscriber implements EntitySubscriberInterface<User> {
6972
const message = "Could not update the personal project's name";
7073
Container.get(Logger).warn(message, event.entity);
7174
const exception = new ApplicationError(message);
72-
ErrorReporterProxy.warn(exception, event.entity);
75+
this.eventReporter.warn(exception, event.entity);
7376
}
7477
}
7578
}

‎packages/cli/src/db.ts

+3-6
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,8 @@
22
import type { EntityManager } from '@n8n/typeorm';
33
// eslint-disable-next-line n8n-local-rules/misplaced-n8n-typeorm-import
44
import { DataSource as Connection } from '@n8n/typeorm';
5-
import {
6-
DbConnectionTimeoutError,
7-
ensureError,
8-
ErrorReporterProxy as ErrorReporter,
9-
} from 'n8n-workflow';
5+
import { ErrorReporter } from 'n8n-core';
6+
import { DbConnectionTimeoutError, ensureError } from 'n8n-workflow';
107
import { Container } from 'typedi';
118

129
import { inTest } from '@/constants';
@@ -38,7 +35,7 @@ if (!inTest) {
3835
connectionState.connected = true;
3936
return;
4037
} catch (error) {
41-
ErrorReporter.error(error);
38+
Container.get(ErrorReporter).error(error);
4239
} finally {
4340
pingTimer = setTimeout(pingDBFn, 2000);
4441
}

‎packages/cli/src/decorators/__tests__/on-shutdown.test.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ describe('OnShutdown', () => {
88
let shutdownService: ShutdownService;
99

1010
beforeEach(() => {
11-
shutdownService = new ShutdownService(mock());
11+
shutdownService = new ShutdownService(mock(), mock());
1212
Container.set(ShutdownService, shutdownService);
1313
jest.spyOn(shutdownService, 'register');
1414
});

0 commit comments

Comments
 (0)