diff --git a/packages/@n8n/db/src/index.ts b/packages/@n8n/db/src/index.ts index a6523f8de01..c07f64a293f 100644 --- a/packages/@n8n/db/src/index.ts +++ b/packages/@n8n/db/src/index.ts @@ -15,6 +15,8 @@ export { chunkIds } from './utils/chunk-ids'; export { dbNowLiteral, dbNowPlusMsLiteral, parseDbTime } from './utils/dialect-time'; export { escapeLike, LIKE_ESCAPE_CLAUSE } from './utils/escape-like'; export { generateHostInstanceId } from './utils/generators'; +export { isEntityNotFoundError } from './utils/is-entity-not-found-error'; +export { isForeignKeyConstraintError } from './utils/is-foreign-key-constraint-error'; export { isStringArray } from './utils/is-string-array'; export { isUniqueConstraintError } from './utils/is-unique-constraint-error'; export { isValidEmail } from './utils/is-valid-email'; diff --git a/packages/@n8n/db/src/repositories/workflow-history.repository.ts b/packages/@n8n/db/src/repositories/workflow-history.repository.ts index fafdf4ac3ec..fdd33d94b33 100644 --- a/packages/@n8n/db/src/repositories/workflow-history.repository.ts +++ b/packages/@n8n/db/src/repositories/workflow-history.repository.ts @@ -24,6 +24,17 @@ export class WorkflowHistoryRepository extends BaseRepository { return await this.delete({ createdAt: LessThan(date) }); } + async findVersionSummaries( + workflowId: string, + versionIds: string[], + ): Promise> { + return await this.find({ + where: { workflowId, versionId: In(versionIds) }, + select: ['versionId', 'name', 'createdAt'], + order: { createdAt: 'DESC' }, + }); + } + /** * Name and optionally describe a single version. Scoped by `workflowId` too * so a version of another workflow can never be touched, and returns the diff --git a/packages/@n8n/db/src/utils/__tests__/is-entity-not-found-error.test.ts b/packages/@n8n/db/src/utils/__tests__/is-entity-not-found-error.test.ts new file mode 100644 index 00000000000..5db822125e4 --- /dev/null +++ b/packages/@n8n/db/src/utils/__tests__/is-entity-not-found-error.test.ts @@ -0,0 +1,23 @@ +import { EntityNotFoundError } from '@n8n/typeorm'; + +import { isEntityNotFoundError } from '../is-entity-not-found-error'; + +class DummyEntity {} + +describe('isEntityNotFoundError', () => { + it('returns true for an EntityNotFoundError', () => { + const error = new EntityNotFoundError(DummyEntity, { id: 'missing' }); + expect(isEntityNotFoundError(error)).toBe(true); + }); + + it('returns false for an unrelated error', () => { + const error = new Error('Connection terminated due to connection timeout'); + expect(isEntityNotFoundError(error)).toBe(false); + }); + + it('returns false for non-error values', () => { + expect(isEntityNotFoundError(null)).toBe(false); + expect(isEntityNotFoundError(undefined)).toBe(false); + expect(isEntityNotFoundError('not found')).toBe(false); + }); +}); diff --git a/packages/@n8n/db/src/utils/__tests__/is-foreign-key-constraint-error.test.ts b/packages/@n8n/db/src/utils/__tests__/is-foreign-key-constraint-error.test.ts new file mode 100644 index 00000000000..fe6c5cddebf --- /dev/null +++ b/packages/@n8n/db/src/utils/__tests__/is-foreign-key-constraint-error.test.ts @@ -0,0 +1,71 @@ +import { QueryFailedError } from '@n8n/typeorm'; + +import { isForeignKeyConstraintError } from '../is-foreign-key-constraint-error'; + +describe('isForeignKeyConstraintError', () => { + const makeQueryFailedError = ( + message: string, + driverError: { code?: string } = {}, + ): QueryFailedError => { + return new QueryFailedError('Query', [], Object.assign(new Error(message), driverError)); + }; + + describe('returns true for actual database foreign-key-constraint violations', () => { + it('SQLite extended code SQLITE_CONSTRAINT_FOREIGNKEY', () => { + const error = makeQueryFailedError('FOREIGN KEY constraint failed', { + code: 'SQLITE_CONSTRAINT_FOREIGNKEY', + }); + expect(isForeignKeyConstraintError(error)).toBe(true); + }); + + it('SQLite base code SQLITE_CONSTRAINT when message mentions FOREIGN KEY constraint', () => { + const error = makeQueryFailedError('SQLITE_CONSTRAINT: FOREIGN KEY constraint failed', { + code: 'SQLITE_CONSTRAINT', + }); + expect(isForeignKeyConstraintError(error)).toBe(true); + }); + + it('PostgreSQL foreign_key_violation (code 23503)', () => { + const error = makeQueryFailedError( + 'insert or update on table "annotation_tag_mapping" violates foreign key constraint', + { code: '23503' }, + ); + expect(isForeignKeyConstraintError(error)).toBe(true); + }); + }); + + describe('returns false for non-QueryFailedError values', () => { + it('plain Error whose message mentions "foreign key"', () => { + const error = new Error('foreign key lookup failed upstream'); + expect(isForeignKeyConstraintError(error)).toBe(false); + }); + + it('non-error values', () => { + expect(isForeignKeyConstraintError(null)).toBe(false); + expect(isForeignKeyConstraintError(undefined)).toBe(false); + expect(isForeignKeyConstraintError('FOREIGN KEY constraint failed')).toBe(false); + }); + }); + + describe('returns false for unrelated database errors', () => { + it('SQLite base code SQLITE_CONSTRAINT for a UNIQUE violation', () => { + const error = makeQueryFailedError('UNIQUE constraint failed: workflow_entity.name', { + code: 'SQLITE_CONSTRAINT', + }); + expect(isForeignKeyConstraintError(error)).toBe(false); + }); + + it('PostgreSQL unique_violation (code 23505)', () => { + const error = makeQueryFailedError( + 'duplicate key value violates unique constraint "users_email_key"', + { code: '23505' }, + ); + expect(isForeignKeyConstraintError(error)).toBe(false); + }); + + it('QueryFailedError with no driver code', () => { + const error = makeQueryFailedError('Connection timeout after 30000ms'); + expect(isForeignKeyConstraintError(error)).toBe(false); + }); + }); +}); diff --git a/packages/@n8n/db/src/utils/is-entity-not-found-error.ts b/packages/@n8n/db/src/utils/is-entity-not-found-error.ts new file mode 100644 index 00000000000..c64907d49ef --- /dev/null +++ b/packages/@n8n/db/src/utils/is-entity-not-found-error.ts @@ -0,0 +1,8 @@ +import { EntityNotFoundError } from '@n8n/typeorm'; + +/** + * Whether `error` is a TypeORM "entity not found" error, e.g. from `findOneOrFail`. + */ +export function isEntityNotFoundError(error: unknown): boolean { + return error instanceof EntityNotFoundError; +} diff --git a/packages/@n8n/db/src/utils/is-foreign-key-constraint-error.ts b/packages/@n8n/db/src/utils/is-foreign-key-constraint-error.ts new file mode 100644 index 00000000000..23f8b42db22 --- /dev/null +++ b/packages/@n8n/db/src/utils/is-foreign-key-constraint-error.ts @@ -0,0 +1,13 @@ +import { isQueryFailedErrorOfKind } from './is-query-failed-error-of-kind'; + +/** + * Whether `error` is a foreign-key-constraint violation, across PostgreSQL and SQLite. + */ +export function isForeignKeyConstraintError(error: unknown): boolean { + return isQueryFailedErrorOfKind(error, { + // PostgreSQL: 23503 = foreign_key_violation. + postgresCode: '23503', + sqliteExtendedCode: 'SQLITE_CONSTRAINT_FOREIGNKEY', + sqliteMessagePattern: /FOREIGN KEY constraint/i, + }); +} diff --git a/packages/@n8n/db/src/utils/is-query-failed-error-of-kind.ts b/packages/@n8n/db/src/utils/is-query-failed-error-of-kind.ts new file mode 100644 index 00000000000..d65a185175d --- /dev/null +++ b/packages/@n8n/db/src/utils/is-query-failed-error-of-kind.ts @@ -0,0 +1,34 @@ +import { QueryFailedError } from '@n8n/typeorm'; + +/** + * Whether `error` is a `QueryFailedError` violating the constraint kind identified + * by `postgresCode`/`sqliteExtendedCode`/`sqliteMessagePattern`, across PostgreSQL + * and SQLite. Shared by the per-kind predicates (`isUniqueConstraintError`, + * `isForeignKeyConstraintError`, …) so the driver-error narrowing and SQLite + * disambiguation logic have one implementation instead of drifting copies. + */ +export function isQueryFailedErrorOfKind( + error: unknown, + { + postgresCode, + sqliteExtendedCode, + sqliteMessagePattern, + }: { postgresCode: string; sqliteExtendedCode: string; sqliteMessagePattern: RegExp }, +): boolean { + if (!(error instanceof QueryFailedError)) return false; + + // TypeORM types `driverError` as `any`; narrow it via `unknown` so the + // property checks below stay type-safe without an `as` cast. + const driverError: unknown = error.driverError; + if (typeof driverError !== 'object' || driverError === null) return false; + + const code = + 'code' in driverError && typeof driverError.code === 'string' ? driverError.code : undefined; + + if (code === postgresCode) return true; + + // SQLite: the extended code is unambiguous; the base code covers all constraint + // kinds (NOT NULL, FK, CHECK, UNIQUE), so disambiguate via the message. + if (code === sqliteExtendedCode) return true; + return code === 'SQLITE_CONSTRAINT' && sqliteMessagePattern.test(error.message); +} diff --git a/packages/@n8n/db/src/utils/is-unique-constraint-error.ts b/packages/@n8n/db/src/utils/is-unique-constraint-error.ts index 4147bdce714..74a14cd1c40 100644 --- a/packages/@n8n/db/src/utils/is-unique-constraint-error.ts +++ b/packages/@n8n/db/src/utils/is-unique-constraint-error.ts @@ -1,26 +1,13 @@ -import { QueryFailedError } from '@n8n/typeorm'; +import { isQueryFailedErrorOfKind } from './is-query-failed-error-of-kind'; /** * Whether `error` is a unique-constraint violation, across PostgreSQL and SQLite. */ export function isUniqueConstraintError(error: unknown): boolean { - if (!(error instanceof QueryFailedError)) return false; - - // TypeORM types `driverError` as `any`; narrow it via `unknown` so the - // property checks below stay type-safe without an `as` cast. - const driverError: unknown = error.driverError; - if (typeof driverError !== 'object' || driverError === null) return false; - - const code = - 'code' in driverError && typeof driverError.code === 'string' ? driverError.code : undefined; - - // PostgreSQL: 23505 = unique_violation. - if (code === '23505') return true; - - // SQLite: the extended code is unambiguous; the base code covers all constraint - // kinds (NOT NULL, FK, CHECK, UNIQUE), so disambiguate via the message. - if (code === 'SQLITE_CONSTRAINT_UNIQUE') return true; - if (code === 'SQLITE_CONSTRAINT' && /UNIQUE constraint/i.test(error.message)) return true; - - return false; + return isQueryFailedErrorOfKind(error, { + // PostgreSQL: 23505 = unique_violation. + postgresCode: '23505', + sqliteExtendedCode: 'SQLITE_CONSTRAINT_UNIQUE', + sqliteMessagePattern: /UNIQUE constraint/i, + }); } diff --git a/packages/cli/eslint.config.mjs b/packages/cli/eslint.config.mjs index e50fc32ad76..cc6dc6102cf 100644 --- a/packages/cli/eslint.config.mjs +++ b/packages/cli/eslint.config.mjs @@ -240,7 +240,6 @@ export default defineConfig( // NEVER add to this list — a new leak must fail CI. Entries are removed as each file migrates. files: [ // credentials/ - './src/credentials-helper.ts', './src/credentials/credential-connection-status-provider.interface.ts', './src/credentials/credential-connection-status-proxy.ts', './src/credentials/credential-dependency.service.ts', @@ -331,7 +330,6 @@ export default defineConfig( './src/executions/execution-data/db-store.ts', './src/executions/execution-persistence.ts', './src/executions/execution-recovery.service.ts', - './src/executions/execution.service.ts', './src/instance-settings-loader/loaders/log-streaming.instance-settings-loader.ts', './src/modules/agents/agents.service.ts', './src/modules/chat-hub/chat-hub-agent.service.ts', diff --git a/packages/cli/src/credentials-helper.ts b/packages/cli/src/credentials-helper.ts index 491172da99c..c89e4b34176 100644 --- a/packages/cli/src/credentials-helper.ts +++ b/packages/cli/src/credentials-helper.ts @@ -1,12 +1,12 @@ -/* eslint-disable @typescript-eslint/no-unsafe-argument */ -/* eslint-disable @typescript-eslint/no-unsafe-member-access */ - import { SYSTEM_RESOLVER_ID } from '@n8n/api-types'; import { LicenseState } from '@n8n/backend-common'; import type { CredentialsEntity, ICredentialsDb } from '@n8n/db'; -import { CredentialsRepository, SecretsProviderConnectionRepository } from '@n8n/db'; +import { + CredentialsRepository, + isEntityNotFoundError, + SecretsProviderConnectionRepository, +} from '@n8n/db'; import { Service } from '@n8n/di'; -import { EntityNotFoundError } from '@n8n/typeorm'; import { Credentials, getAdditionalKeys } from 'n8n-core'; import type { CredentialInformation, @@ -343,7 +343,7 @@ export class CredentialsHelper extends ICredentialsHelper { type, }); } catch (error) { - if (error instanceof EntityNotFoundError) { + if (isEntityNotFoundError(error)) { throw new CredentialNotFoundError(nodeCredential.id, type); } diff --git a/packages/cli/src/executions/__tests__/execution.service.test.ts b/packages/cli/src/executions/__tests__/execution.service.test.ts index ae65f5ca20a..03e79364266 100644 --- a/packages/cli/src/executions/__tests__/execution.service.test.ts +++ b/packages/cli/src/executions/__tests__/execution.service.test.ts @@ -11,7 +11,6 @@ import type { Project, User, WorkflowHistoryRepository, - WorkflowHistory, } from '@n8n/db'; import { Container } from '@n8n/di'; import { QueryFailedError } from '@n8n/typeorm'; @@ -865,31 +864,26 @@ describe('ExecutionService', () => { const result = await executionService.getExecutedVersions(workflowId); expect(result).toEqual([]); - expect(workflowHistoryRepository.find).not.toHaveBeenCalled(); + expect(workflowHistoryRepository.findVersionSummaries).not.toHaveBeenCalled(); }); it('should return versions with metadata from workflow history', async () => { const versionIds = ['v1', 'v2']; executionRepository.getDistinctVersionIds.mockResolvedValue(versionIds); - const historyVersions = [ - mock({ versionId: 'v2', name: null, createdAt: new Date('2025-01-02') }), - mock({ - versionId: 'v1', - name: 'Release 1', - createdAt: new Date('2025-01-01'), - }), + const historySummaries = [ + { versionId: 'v2', name: null, createdAt: new Date('2025-01-02') }, + { versionId: 'v1', name: 'Release 1', createdAt: new Date('2025-01-01') }, ]; - workflowHistoryRepository.find.mockResolvedValue(historyVersions); + workflowHistoryRepository.findVersionSummaries.mockResolvedValue(historySummaries); const result = await executionService.getExecutedVersions(workflowId); expect(executionRepository.getDistinctVersionIds).toHaveBeenCalledWith(workflowId); - expect(workflowHistoryRepository.find).toHaveBeenCalledWith({ - where: { workflowId, versionId: expect.anything() }, - select: ['versionId', 'name', 'createdAt'], - order: { createdAt: 'DESC' }, - }); + expect(workflowHistoryRepository.findVersionSummaries).toHaveBeenCalledWith( + workflowId, + versionIds, + ); expect(result).toHaveLength(2); expect(result[0].versionId).toBe('v2'); expect(result[0].name).toBeNull(); @@ -899,7 +893,7 @@ describe('ExecutionService', () => { it('should return empty array when version IDs have no matching history', async () => { executionRepository.getDistinctVersionIds.mockResolvedValue(['orphan-v1']); - workflowHistoryRepository.find.mockResolvedValue([]); + workflowHistoryRepository.findVersionSummaries.mockResolvedValue([]); const result = await executionService.getExecutedVersions(workflowId); @@ -1107,16 +1101,37 @@ describe('ExecutionService', () => { ]); }); - it('should map QueryFailedError to NotFoundError for missing tags', async () => { + it('should map a foreign-key-constraint violation to NotFoundError for missing tags', async () => { executionPersistence.findOneInWorkflows.mockResolvedValue(mock({ id: '1' })); executionAnnotationRepository.findOneOrFail.mockResolvedValue({ id: 42 } as never); annotationTagMappingRepository.overwriteTags.mockRejectedValue( - new QueryFailedError('INSERT', [], new Error('FK')), + new QueryFailedError( + 'INSERT', + [], + Object.assign(new Error('FOREIGN KEY constraint failed'), { + code: 'SQLITE_CONSTRAINT_FOREIGNKEY', + }), + ), ); await expect( executionService.updateExecutionTags('1', ['missing'], ['wf-1']), ).rejects.toThrow('Some tags not found'); }); + + it('should let a non-foreign-key QueryFailedError propagate instead of mapping it to a 404', async () => { + executionPersistence.findOneInWorkflows.mockResolvedValue(mock({ id: '1' })); + executionAnnotationRepository.findOneOrFail.mockResolvedValue({ id: 42 } as never); + const dbError = new QueryFailedError( + 'INSERT', + [], + Object.assign(new Error('Connection timeout after 30000ms'), {}), + ); + annotationTagMappingRepository.overwriteTags.mockRejectedValue(dbError); + + await expect(executionService.updateExecutionTags('1', ['tag-1'], ['wf-1'])).rejects.toThrow( + dbError, + ); + }); }); }); diff --git a/packages/cli/src/executions/execution.service.ts b/packages/cli/src/executions/execution.service.ts index e18ab15b48e..ecb8e1aaccb 100644 --- a/packages/cli/src/executions/execution.service.ts +++ b/packages/cli/src/executions/execution.service.ts @@ -14,12 +14,11 @@ import { AnnotationTagMappingRepository, ExecutionAnnotationRepository, ExecutionRepository, - In, + isForeignKeyConstraintError, WorkflowHistoryRepository, WorkflowRepository, } from '@n8n/db'; import { Service } from '@n8n/di'; -import { QueryFailedError } from '@n8n/typeorm'; import { ensureError } from '@n8n/utils/errors/ensure-error'; import { stringify } from 'flatted'; import { validate as jsonSchemaValidate } from 'jsonschema'; @@ -811,7 +810,7 @@ export class ExecutionService { try { await this.annotationTagMappingRepository.overwriteTags(annotation.id, tagIds); } catch (error) { - if (error instanceof QueryFailedError) { + if (isForeignKeyConstraintError(error)) { throw new NotFoundError('Some tags not found'); } throw error; @@ -836,11 +835,10 @@ export class ExecutionService { const versionIds = await this.executionRepository.getDistinctVersionIds(workflowId); if (versionIds.length === 0) return []; - const versions = await this.workflowHistoryRepository.find({ - where: { workflowId, versionId: In(versionIds) }, - select: ['versionId', 'name', 'createdAt'], - order: { createdAt: 'DESC' }, - }); + const versions = await this.workflowHistoryRepository.findVersionSummaries( + workflowId, + versionIds, + ); return versions.map((v) => ({ versionId: v.versionId, diff --git a/packages/cli/src/modules/promotions.ee/database/repositories/promotion-provider.repository.ts b/packages/cli/src/modules/promotions.ee/database/repositories/promotion-provider.repository.ts index 76583614f62..f4473ef0d7e 100644 --- a/packages/cli/src/modules/promotions.ee/database/repositories/promotion-provider.repository.ts +++ b/packages/cli/src/modules/promotions.ee/database/repositories/promotion-provider.repository.ts @@ -1,6 +1,11 @@ -import { BaseRepository, type OperationContext, TransactionRunner } from '@n8n/db'; +import { + BaseRepository, + isForeignKeyConstraintError, + type OperationContext, + TransactionRunner, +} from '@n8n/db'; import { Service } from '@n8n/di'; -import { DataSource, QueryFailedError } from '@n8n/typeorm'; +import { DataSource } from '@n8n/typeorm'; import { PromotionProvider } from '../entities/promotion-provider.entity'; import { PromotionConflictError } from '../promotion-conflict.error'; @@ -63,23 +68,3 @@ export class PromotionProviderRepository extends BaseRepository { } }); }); + + describe('findVersionSummaries', () => { + it('scopes by workflowId, filters by versionId, selects versionId/name/createdAt, and orders by createdAt desc', async () => { + const workflowA = await createWorkflow({ + versionId: uuid(), + nodes: [testNode1], + }); + const workflowB = await createWorkflow({ + versionId: uuid(), + nodes: [testNode1], + }); + + const vOldest = uuid(); + const vMiddle = uuid(); + const vNewest = uuid(); + const vNotRequested = uuid(); + const vOtherWorkflow = uuid(); + + const threeDaysAgo = new Date(); + threeDaysAgo.setDate(threeDaysAgo.getDate() - 3); + const twoDaysAgo = new Date(); + twoDaysAgo.setDate(twoDaysAgo.getDate() - 2); + const oneDayAgo = new Date(); + oneDayAgo.setDate(oneDayAgo.getDate() - 1); + + await createWorkflowHistory(workflowA, undefined, undefined, { + versionId: vOldest, + name: null, + createdAt: threeDaysAgo, + }); + await createWorkflowHistory(workflowA, undefined, undefined, { + versionId: vMiddle, + name: 'Release 1', + createdAt: twoDaysAgo, + }); + await createWorkflowHistory(workflowA, undefined, undefined, { + versionId: vNewest, + name: null, + createdAt: oneDayAgo, + }); + // Belongs to workflowA but not passed in versionIds — must be excluded + await createWorkflowHistory(workflowA, undefined, undefined, { + versionId: vNotRequested, + name: 'Should not appear', + createdAt: oneDayAgo, + }); + // Passed in versionIds but belongs to workflowB — must be excluded by workflowId scoping + await createWorkflowHistory(workflowB, undefined, undefined, { + versionId: vOtherWorkflow, + name: 'Wrong workflow', + createdAt: oneDayAgo, + }); + + const repository = Container.get(WorkflowHistoryRepository); + const result = await repository.findVersionSummaries(workflowA.id, [ + vOldest, + vMiddle, + vNewest, + vOtherWorkflow, + uuid(), // not present in the DB at all + ]); + + expect(result).toHaveLength(3); + expect(result.map((v) => v.versionId)).toEqual([vNewest, vMiddle, vOldest]); + expect(result[0]).toMatchObject({ versionId: vNewest, name: null }); + expect(result[1]).toMatchObject({ versionId: vMiddle, name: 'Release 1' }); + expect(result[2]).toMatchObject({ versionId: vOldest, name: null }); + // `select` should be applied: only versionId/name/createdAt are populated, + // not the full entity (e.g. nodes/authors). + expect(result[0]).not.toHaveProperty('nodes'); + expect(result[0]).not.toHaveProperty('authors'); + }); + + it('returns an empty array when no versionIds match', async () => { + const workflow = await createWorkflow({ versionId: uuid(), nodes: [testNode1] }); + + const repository = Container.get(WorkflowHistoryRepository); + const result = await repository.findVersionSummaries(workflow.id, [uuid()]); + + expect(result).toEqual([]); + }); + }); });