mirror of
https://github.com/n8n-io/n8n.git
synced 2026-06-04 10:39:23 +02:00
fix(core): Prevent isUniqueConstraintError false positives (#31284)
Co-authored-by: SAAKSHI GUPTA <saakshigupta2002@gmail.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
7efcc311b5
commit
a3e37fcd12
|
|
@ -1,8 +1,9 @@
|
|||
import { QueryFailedError } from '@n8n/db';
|
||||
import type { Response } from 'express';
|
||||
import { mock } from 'jest-mock-extended';
|
||||
|
||||
import { LicenseEulaRequiredError } from '@/errors/response-errors/license-eula-required.error';
|
||||
import { sendErrorResponse } from '@/response-helper';
|
||||
import { isUniqueConstraintError, sendErrorResponse } from '@/response-helper';
|
||||
|
||||
describe('sendErrorResponse', () => {
|
||||
let mockResponse: Response;
|
||||
|
|
@ -51,3 +52,77 @@ describe('sendErrorResponse', () => {
|
|||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('isUniqueConstraintError', () => {
|
||||
const makeQueryFailedError = (
|
||||
message: string,
|
||||
driverError: { code?: string } = {},
|
||||
): QueryFailedError => {
|
||||
return new QueryFailedError('Query', [], Object.assign(new Error(message), driverError));
|
||||
};
|
||||
|
||||
describe('returns true for actual database unique-constraint violations', () => {
|
||||
it('SQLite extended code SQLITE_CONSTRAINT_UNIQUE', () => {
|
||||
const error = makeQueryFailedError('UNIQUE constraint failed: workflow_entity.name', {
|
||||
code: 'SQLITE_CONSTRAINT_UNIQUE',
|
||||
});
|
||||
expect(isUniqueConstraintError(error)).toBe(true);
|
||||
});
|
||||
|
||||
it('SQLite base code SQLITE_CONSTRAINT when message mentions UNIQUE constraint', () => {
|
||||
const error = makeQueryFailedError(
|
||||
'SQLITE_CONSTRAINT: UNIQUE constraint failed: workflow_entity.name',
|
||||
{ code: 'SQLITE_CONSTRAINT' },
|
||||
);
|
||||
expect(isUniqueConstraintError(error)).toBe(true);
|
||||
});
|
||||
|
||||
it('PostgreSQL unique_violation (code 23505)', () => {
|
||||
const error = makeQueryFailedError(
|
||||
'duplicate key value violates unique constraint "users_email_key"',
|
||||
{ code: '23505' },
|
||||
);
|
||||
expect(isUniqueConstraintError(error)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('returns false for errors that previously triggered false positives (#25012)', () => {
|
||||
it('plain Error whose message mentions a workflow named "Duplicate Detection"', () => {
|
||||
const error = new Error(
|
||||
'Cannot publish workflow: Node "Execute Workflow" references workflow "Duplicate Detection" which is not published',
|
||||
);
|
||||
expect(isUniqueConstraintError(error)).toBe(false);
|
||||
});
|
||||
|
||||
it('plain Error whose message mentions "unique"', () => {
|
||||
const error = new Error('The value must be unique across all entries');
|
||||
expect(isUniqueConstraintError(error)).toBe(false);
|
||||
});
|
||||
|
||||
it('plain Error mentioning both trigger words', () => {
|
||||
const error = new Error('Unique identifier duplicate found in configuration');
|
||||
expect(isUniqueConstraintError(error)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('returns false for unrelated database errors', () => {
|
||||
it('SQLite base code SQLITE_CONSTRAINT for a NOT NULL violation', () => {
|
||||
const error = makeQueryFailedError('NOT NULL constraint failed: workflow_entity.name', {
|
||||
code: 'SQLITE_CONSTRAINT',
|
||||
});
|
||||
expect(isUniqueConstraintError(error)).toBe(false);
|
||||
});
|
||||
|
||||
it('PostgreSQL not_null_violation (code 23502)', () => {
|
||||
const error = makeQueryFailedError('null value in column violates not-null constraint', {
|
||||
code: '23502',
|
||||
});
|
||||
expect(isUniqueConstraintError(error)).toBe(false);
|
||||
});
|
||||
|
||||
it('QueryFailedError with no driver code', () => {
|
||||
const error = makeQueryFailedError('Connection timeout after 30000ms');
|
||||
expect(isUniqueConstraintError(error)).toBe(false);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
import { inDevelopment, Logger } from '@n8n/backend-common';
|
||||
import type { User } from '@n8n/db';
|
||||
import { QueryFailedError, type User } from '@n8n/db';
|
||||
import { Container } from '@n8n/di';
|
||||
import type { ReportingOptions } from '@n8n/errors';
|
||||
import type { Request, Response } from 'express';
|
||||
|
|
@ -92,8 +92,27 @@ export function sendErrorResponse(res: Response, error: Error) {
|
|||
res.status(status).json(response);
|
||||
}
|
||||
|
||||
export const isUniqueConstraintError = (error: Error) =>
|
||||
['unique', 'duplicate'].some((s) => error.message.toLowerCase().includes(s));
|
||||
export const isUniqueConstraintError = (error: Error): 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: 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;
|
||||
};
|
||||
|
||||
export function reportError(error: Error, options?: ReportingOptions) {
|
||||
if (!(error instanceof ResponseError) || error.httpStatusCode > 404) {
|
||||
|
|
|
|||
|
|
@ -204,7 +204,7 @@ describe('Secret Providers Connections API', () => {
|
|||
.send(payload)
|
||||
.expect(400);
|
||||
|
||||
expect(response.body.message).toBe('There is already an entry with this name');
|
||||
expect(response.body.message).toBe('Connection with key "duplicateTest" already exists');
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user