fix(core): Round fractional time saved values before inserting into insights BIGINT column (#29553)

This commit is contained in:
Ali Elkhateeb 2026-04-30 10:09:02 +03:00 committed by GitHub
parent ef56501d47
commit 74d55b9c68
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 160 additions and 78 deletions

View File

@ -17,6 +17,7 @@ import {
type IRun,
type WorkflowExecuteMode,
} from 'n8n-workflow';
import assert from 'node:assert';
import type { TypeUnit } from '@/modules/insights/database/entities/insights-shared';
import { InsightsMetadataRepository } from '@/modules/insights/database/repositories/insights-metadata.repository';
@ -95,9 +96,7 @@ describe('workflowExecuteAfterHandler', () => {
// ASSERT
const metadata = await insightsMetadataRepository.findOneBy({ workflowId: workflow.id });
if (!metadata) {
return fail('expected metadata to exist');
}
assert(metadata, 'Expected metadata to exist');
expect(metadata).toMatchObject({
workflowId: workflow.id,
@ -218,9 +217,7 @@ describe('workflowExecuteAfterHandler', () => {
// ASSERT
const metadata = await insightsMetadataRepository.findOneBy({ workflowId: workflow.id });
if (!metadata) {
return fail('expected metadata to exist');
}
assert(metadata, 'Expected metadata to exist');
expect(metadata).toMatchObject({
workflowId: workflow.id,
@ -629,6 +626,76 @@ describe('workflowExecuteAfterHandler - flushEvents', () => {
}
});
test('flushEvents rounds fractional time_saved_min for PostgreSQL BIGINT on insights_raw.value', async () => {
repoMocks.insertInsightsRaw.mockClear();
workflow.settings = {
timeSavedMode: 'dynamic',
};
const ctx = mock<WorkflowExecuteAfterContext>({
workflow,
runData: mock<IRun>({
mode: 'webhook',
status: 'success',
startedAt: startedAt.toJSDate(),
stoppedAt: stoppedAt.toJSDate(),
data: {
resultData: {
runData: {
timeSavedNode: [{ metadata: { timeSaved: { minutes: 5.4 } } }],
},
},
},
}),
});
await insightsCollectionService.handleWorkflowExecuteAfter(ctx);
await insightsCollectionService.flushEvents();
expect(repoMocks.insertInsightsRaw).toHaveBeenCalledWith(
expect.arrayContaining([expect.objectContaining({ type: 'time_saved_min', value: 5 })]),
);
});
test.each<{ label: string; timeSavedPerExecution: number }>([
{ label: 'NaN', timeSavedPerExecution: Number.NaN },
{ label: 'Infinity', timeSavedPerExecution: Number.POSITIVE_INFINITY },
])(
'flushEvents normalizes time_saved_min to 0 when timeSavedPerExecution is $label (PostgreSQL BIGINT)',
async ({ timeSavedPerExecution }) => {
repoMocks.insertInsightsRaw.mockClear();
workflow.settings = {
timeSavedMode: 'fixed',
timeSavedPerExecution,
};
const ctx = mock<WorkflowExecuteAfterContext>({ workflow, runData });
await insightsCollectionService.handleWorkflowExecuteAfter(ctx);
await insightsCollectionService.flushEvents();
expect(repoMocks.insertInsightsRaw).toHaveBeenCalledWith(
expect.arrayContaining([expect.objectContaining({ type: 'time_saved_min', value: 0 })]),
);
},
);
test('flushEvents normalizes runtime_ms to 0 when runtime is NaN (PostgreSQL BIGINT)', async () => {
repoMocks.insertInsightsRaw.mockClear();
const badRuntimeRunData = mock<IRun>({
mode: 'trigger',
status: 'success',
startedAt: new Date(Number.NaN),
stoppedAt: stoppedAt.toJSDate(),
});
const ctx = mock<WorkflowExecuteAfterContext>({ workflow, runData: badRuntimeRunData });
await insightsCollectionService.handleWorkflowExecuteAfter(ctx);
await insightsCollectionService.flushEvents();
expect(repoMocks.insertInsightsRaw).toHaveBeenCalledWith(
expect.arrayContaining([expect.objectContaining({ type: 'runtime_ms', value: 0 })]),
);
});
test('waits for ongoing flush during shutdown', async () => {
// ARRANGE
const config = Container.get(InsightsConfig);

View File

@ -55,6 +55,17 @@ const MIN_RUNTIME = 0;
// PostgreSQL INTEGER max (signed 32-bit)
const MAX_RUNTIME = 2 ** 31 - 1;
/**
* `insights_raw.value` is stored as BIGINT in PostgreSQL. Non-integer JavaScript
* numbers are serialized with a fractional part and rejected by the driver
*/
function integerValueForInsightsRaw(value: number): number {
if (!Number.isFinite(value)) {
return 0;
}
return Math.round(value);
}
type BufferedInsight = Pick<InsightsRaw, 'type' | 'value' | 'timestamp'> & {
workflowId: string;
workflowName: string;
@ -256,7 +267,7 @@ export class InsightsCollectionService {
}
insight.metaId = metadata.metaId;
insight.type = event.type;
insight.value = event.value;
insight.value = integerValueForInsightsRaw(event.value);
insight.timestamp = event.timestamp;
events.push(insight);

View File

@ -1419,6 +1419,7 @@ onBeforeUnmount(() => {
:disabled="readOnlyEnv || !workflowPermissions.update"
data-test-id="workflow-settings-time-saved-per-execution"
:min="0"
:precision="0"
@update:model-value="updateTimeSavedPerExecution"
/>
<span>{{ i18n.baseText('workflowSettings.timeSavedPerExecution.hint') }}</span>

View File

@ -1,6 +1,6 @@
<script setup lang="ts">
import { computed, inject, nextTick, onBeforeUnmount, onMounted, onUpdated, ref, watch } from 'vue';
import { computedAsync, useDebounceFn } from '@vueuse/core';
import { computedAsync, useDebounceFn, useElementSize } from '@vueuse/core';
import get from 'lodash/get';
@ -81,7 +81,6 @@ import { useWorkflowsListStore } from '@/app/stores/workflowsList.store';
import { useUIStore } from '@/app/stores/ui.store';
import type { EventBus } from '@n8n/utils/event-bus';
import { createEventBus } from '@n8n/utils/event-bus';
import { useElementSize } from '@vueuse/core';
import { captureMessage } from '@sentry/vue';
import { isCredentialOnlyNodeType } from '@/app/utils/credentialOnlyNodes';
import {
@ -189,8 +188,6 @@ const { isEnabled: isCollectionOverhaulEnabled } = useCollectionOverhaul();
const expressionLocalResolveCtx = inject(ExpressionLocalResolveContextSymbol, undefined);
// ESLint: false positive
// eslint-disable-next-line @typescript-eslint/no-redundant-type-constituents
const inputField = ref<InstanceType<typeof N8nInput | typeof N8nSelect> | HTMLElement>();
const wrapper = ref<HTMLDivElement>();
@ -1153,7 +1150,7 @@ function validateJsonPassword(value: string) {
return;
}
if (!value || !value.trim()) {
if (!value?.trim()) {
jsonValidationError.value = null;
return;
}
@ -1285,7 +1282,7 @@ onMounted(() => {
void externalHooks.run('parameterInput.mount', {
parameter: props.parameter,
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unnecessary-type-assertion
inputFieldRef: inputField.value as InstanceType<typeof N8nInput>,
});
});

View File

@ -56,6 +56,7 @@ export class TimeSaved implements INodeType {
noDataExpression: true,
typeOptions: {
minValue: 0,
numberPrecision: 0,
},
description: 'Number of minutes saved by this workflow execution',
},

View File

@ -1,28 +1,58 @@
import type { IExecuteFunctions } from 'n8n-workflow';
import { mock } from 'jest-mock-extended';
import type { IExecuteFunctions, INodeExecutionData } from 'n8n-workflow';
import { TimeSaved } from '../TimeSaved.node';
describe('TimeSaved node', () => {
function createExecuteFunctionsMock(options?: {
mode?: 'once' | 'perItem';
minutesSaved?: number;
items?: INodeExecutionData[];
continueOnFail?: boolean;
}) {
const {
items = [{ json: {} }],
mode = 'once',
minutesSaved = 10,
continueOnFail = false,
} = options ?? {};
const setMetadata = jest.fn();
const continueOnFailMock = jest.fn().mockReturnValue(continueOnFail);
return {
executeFunctions: mock<IExecuteFunctions>({
getInputData: jest.fn().mockReturnValue(items),
getNodeParameter: jest.fn().mockReturnValueOnce(mode).mockReturnValueOnce(minutesSaved),
continueOnFail: continueOnFailMock,
setMetadata,
getNode: jest.fn().mockReturnValue({ name: 'TimeSaved', type: 'timeSaved' }),
}),
setMetadata,
continueOnFail: continueOnFailMock,
};
}
it('should be defined', () => {
expect(TimeSaved).toBeDefined();
});
it('should set metadata with time saved for fixed option', async () => {
it('should expose minutesSaved as a whole number with correct typeOptions', () => {
const node = new TimeSaved();
const minutesSaved = node.description.properties.find((p) => p.name === 'minutesSaved');
expect(minutesSaved?.typeOptions).toEqual(
expect.objectContaining({ minValue: 0, numberPrecision: 0 }),
);
});
it('should set metadata with time saved for once mode', async () => {
const node = new TimeSaved();
const executionFunctions = {
getInputData: jest.fn().mockReturnValue([{ json: {} }]),
getNodeParameter: jest.fn().mockReturnValueOnce('fixed').mockReturnValueOnce(10),
continueOnFail: jest.fn().mockReturnValue(false),
setMetadata: jest.fn(),
getNode: jest.fn().mockReturnValue({
name: 'TimeSaved',
type: 'timeSaved',
}),
} as any as IExecuteFunctions;
const { executeFunctions, setMetadata } = createExecuteFunctionsMock();
const result = await node.execute.call(executionFunctions);
const result = await node.execute.call(executeFunctions);
expect(executionFunctions.setMetadata).toHaveBeenCalledWith({
expect(setMetadata).toHaveBeenCalledWith({
timeSaved: {
minutes: 10,
},
@ -34,20 +64,14 @@ describe('TimeSaved node', () => {
it('should set metadata with time saved for per item option', async () => {
const node = new TimeSaved();
const executionFunctions = {
getInputData: jest.fn().mockReturnValueOnce([{ json: {} }, { json: {} }]),
getNodeParameter: jest.fn().mockReturnValueOnce('perItem').mockReturnValueOnce(10),
continueOnFail: jest.fn().mockReturnValue(false),
setMetadata: jest.fn(),
getNode: jest.fn().mockReturnValue({
name: 'TimeSaved',
type: 'timeSaved',
}),
} as any as IExecuteFunctions;
const { executeFunctions, setMetadata } = createExecuteFunctionsMock({
items: [{ json: {} }, { json: {} }],
mode: 'perItem',
});
const result = await node.execute.call(executionFunctions);
const result = await node.execute.call(executeFunctions);
expect(executionFunctions.setMetadata).toHaveBeenCalledWith({
expect(setMetadata).toHaveBeenCalledWith({
timeSaved: {
minutes: 20,
},
@ -59,64 +83,45 @@ describe('TimeSaved node', () => {
it('should return an error if the minutes saved is negative', async () => {
const node = new TimeSaved();
const executionFunctions = {
getInputData: jest.fn().mockReturnValueOnce([{ json: {} }]),
getNodeParameter: jest.fn().mockReturnValueOnce('fixed').mockReturnValueOnce(-1),
continueOnFail: jest.fn().mockReturnValue(false),
setMetadata: jest.fn(),
getNode: jest.fn().mockReturnValue({
name: 'TimeSaved',
type: 'timeSaved',
}),
} as any as IExecuteFunctions;
const { executeFunctions, setMetadata } = createExecuteFunctionsMock({
minutesSaved: -1,
});
await expect(node.execute.call(executionFunctions)).rejects.toThrow(
await expect(node.execute.call(executeFunctions)).rejects.toThrow(
'Time saved cannot be negative, got: -1',
);
expect(executionFunctions.setMetadata).not.toHaveBeenCalled();
expect(setMetadata).not.toHaveBeenCalled();
});
it('should return an error if the minutes saved is not a number', async () => {
const node = new TimeSaved();
const executionFunctions = {
getInputData: jest.fn().mockReturnValueOnce([{ json: {} }]),
getNodeParameter: jest.fn().mockReturnValueOnce('fixed').mockReturnValueOnce('not a number'),
continueOnFail: jest.fn().mockReturnValue(false),
setMetadata: jest.fn(),
getNode: jest.fn().mockReturnValue({
name: 'TimeSaved',
type: 'timeSaved',
}),
} as any as IExecuteFunctions;
const { executeFunctions, setMetadata } = createExecuteFunctionsMock({
// @ts-expect-error - we want to test the error case
minutesSaved: 'not a number',
});
await expect(node.execute.call(executionFunctions)).rejects.toThrow(
await expect(node.execute.call(executeFunctions)).rejects.toThrow(
'Parameter "minutesSaved" is not number',
);
expect(executionFunctions.setMetadata).not.toHaveBeenCalled();
expect(setMetadata).not.toHaveBeenCalled();
});
it('should continue on fail if the minutes saved is not a number and the config is set to continue on fail', async () => {
it('should continue on fail when setMetadata throws and continue on fail is enabled', async () => {
const node = new TimeSaved();
const executionFunctions = {
getInputData: jest.fn().mockReturnValueOnce([{ json: {} }]),
getNodeParameter: jest.fn().mockReturnValueOnce('fixed').mockReturnValueOnce(10),
continueOnFail: jest.fn().mockReturnValue(true),
setMetadata: jest.fn().mockImplementationOnce(() => {
throw new Error('Test error');
}),
getNode: jest.fn().mockReturnValue({
name: 'TimeSaved',
type: 'timeSaved',
}),
} as any as IExecuteFunctions;
const { executeFunctions, setMetadata, continueOnFail } = createExecuteFunctionsMock({
continueOnFail: true,
});
setMetadata.mockImplementationOnce(() => {
throw new Error('Test error');
});
const result = await node.execute.call(executionFunctions);
const result = await node.execute.call(executeFunctions);
expect(result[0].length).toEqual(1);
expect(executionFunctions.continueOnFail).toHaveBeenCalled();
expect(continueOnFail).toHaveBeenCalled();
});
});