github-mirror/n8n
1
0
Fork 0
mirror of https://github.com/n8n-io/n8n.git synced 2026-06-04 02:37:46 +02:00
Code Issues Actions 18 Packages Projects Releases Wiki Activity Graph

refactor(core): Reject combining requireStatus and requireNotCanceled (#31501)

Browse Source
This commit is contained in:
Iván Ovejero 2026-06-02 10:21:15 +02:00 committed by GitHub
parent 7bed5ec9a7
commit c33a772cc0
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 22 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
packages/cli/src/executions/__tests__/execution-persistence.test.ts
View File

@ -12,7 +12,7 @@ import { QueryFailedError } from '@n8n/typeorm';
import { mock } from 'jest-mock-extended';
import type { BinaryDataService, StorageConfig } from 'n8n-core';
import type { IWorkflowBase } from 'n8n-workflow';
import { createEmptyRunExecutionData } from 'n8n-workflow';
import { createEmptyRunExecutionData, UnexpectedError } from 'n8n-workflow';
import { DuplicateExecutionError } from '@/errors/duplicate-execution.error';
import type { DbStore } from '@/executions/execution-data/db-store';
@ -328,6 +328,22 @@ describe('ExecutionPersistence', () => {
} as unknown as Awaited<ReturnType<ExecutionRepository['findOne']>>);
};
describe('condition guards', () => {
it('throws when requireStatus and requireNotCanceled are combined (both constrain status)', async () => {
const executionPersistence = createPersistenceService('db');
await expect(
executionPersistence.updateExistingExecution(
executionId,
{ status: 'running' },
{ requireStatus: 'waiting', requireNotCanceled: true },
),
).rejects.toThrow(UnexpectedError);
expect(executionRepository.update).not.toHaveBeenCalled();
});
});
describe('metadata-only updates', () => {
it('should update the entity directly without touching either data store', async () => {
const executionPersistence = createPersistenceService('fs');

9
packages/cli/src/executions/execution-persistence.ts
View File

@ -12,6 +12,7 @@ import { ExecutionEntity, ExecutionRepository, Not } from '@n8n/db';
import { Service } from '@n8n/di';
import { stringify } from 'flatted';
import { BinaryDataService, StorageConfig } from 'n8n-core';
import { UnexpectedError } from 'n8n-workflow';
import { DbStore } from './execution-data/db-store';
import { FsStore } from './execution-data/fs-store';
@ -246,16 +247,16 @@ export class ExecutionPersistence {
executionId: string,
conditions?: UpdateExecutionConditions,
): FindOptionsWhere<ExecutionEntity> {
if (conditions?.requireStatus && conditions?.requireNotCanceled) {
throw new UnexpectedError('`requireStatus` and `requireNotCanceled` cannot be combined');
}
const where: FindOptionsWhere<ExecutionEntity> = { id: executionId };
if (conditions?.requireStatus) where.status = conditions.requireStatus;
// TODO(CAT-3214): `ExecutionEntity.finished` is deprecated and we should rely on statuses
// only, but for now we still use it to filter out finished executions for parity with
// ExecutionRepository.
if (conditions?.requireNotFinished) where.finished = false;
// TODO(CAT-3215): `requireStatus` and `requireNotCanceled` both write to `where.status`,
// so if both are supplied the `Not('canceled')` clause silently overwrites the specific
// status check. In practice callers never combine them, so once we drop strict parity with
// ExecutionRepository we should assert their mutual exclusivity (or combine them somehow).
if (conditions?.requireNotCanceled) where.status = Not('canceled');
return where;
}