Skip to content

Commit

Permalink
fix: Prevent unnecessary error messages also for data loaded flag (#6201
Browse files Browse the repository at this point in the history
)

* fix: Prevent unnecessary error messages also for data loaded flag

* refactor: Search if data has been loaded before trying to save and fire other events

* fix broken test

* fix lint issue
  • Loading branch information
krynble authored and netroy committed May 15, 2023
1 parent 7718c18 commit 85b9b3f
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 15 deletions.
36 changes: 24 additions & 12 deletions packages/cli/src/events/WorkflowStatistics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,23 +135,35 @@ export async function nodeFetchedData(
node: INode,
): Promise<void> {
if (!workflowId) return;
// Try to insert the data loaded statistic
try {
await Db.collections.WorkflowStatistics.insert({

const hasLoadedDataPreviously = await Db.collections.WorkflowStatistics.findOne({
select: ['count'],
where: {
workflowId,
name: StatisticsNames.dataLoaded,
count: 1,
latestEvent: new Date(),
});
} catch (error) {
// if it's a duplicate key error then that's fine, otherwise throw the error
if (!(error instanceof QueryFailedError)) {
throw error;
}
// If it is a query failed error, we return
},
});

if (hasLoadedDataPreviously) {
return;
}

// Try to insert the data loaded statistic
try {
await Db.collections.WorkflowStatistics.createQueryBuilder('workflowStatistics')
.insert()
.values({
workflowId,
name: StatisticsNames.dataLoaded,
count: 1,
latestEvent: new Date(),
})
.orIgnore()
.execute();
} catch (error) {
LoggerProxy.warn('Failed saving loaded data statistics');
}

// Compile the metrics since this was a new data loaded event
const owner = await getWorkflowOwner(workflowId);
let metrics = {
Expand Down
13 changes: 10 additions & 3 deletions packages/cli/test/unit/Events.test.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import type { IRun, WorkflowExecuteMode } from 'n8n-workflow';
import { LoggerProxy } from 'n8n-workflow';
import { QueryFailedError } from 'typeorm';
import { mock } from 'jest-mock-extended';

import config from '@/config';
import * as Db from '@/Db';
import { User } from '@db/entities/User';
import { StatisticsNames } from '@db/entities/WorkflowStatistics';
import type { WorkflowStatistics } from '@db/entities/WorkflowStatistics';
import type { WorkflowStatisticsRepository } from '@db/repositories';
import { nodeFetchedData, workflowExecutionCompleted } from '@/events/WorkflowStatistics';
Expand All @@ -15,6 +15,7 @@ import { InternalHooks } from '@/InternalHooks';

import { mockInstance } from '../integration/shared/utils';
import { UserService } from '@/user/user.service';
import { WorkflowEntity } from '@/databases/entities/WorkflowEntity';

jest.mock('@/Db', () => {
return {
Expand Down Expand Up @@ -202,8 +203,14 @@ describe('Events', () => {

test('should not send metrics for entries that already have the flag set', async () => {
// Fetch data for workflow 2 which is set up to not be altered in the mocks
workflowStatisticsRepository.insert.mockImplementationOnce(() => {
throw new QueryFailedError('invalid insert', [], '');
workflowStatisticsRepository.findOne.mockImplementationOnce(async () => {
return {
count: 1,
name: StatisticsNames.dataLoaded,
latestEvent: new Date(),
workflowId: '2',
workflow: new WorkflowEntity(),
};
});
const workflowId = '1';
const node = {
Expand Down

0 comments on commit 85b9b3f

Please sign in to comment.