refactor(core): Add Data store specific errors use them at service / repository (no-changelog) (#18380)

This commit is contained in:
Jaakko Husso
2025-08-15 12:44:10 +03:00
committed by GitHub
parent 9cc1b11f7f
commit e1d8eaa170
11 changed files with 273 additions and 119 deletions

View File

@@ -2,5 +2,5 @@ import { z } from 'zod';
import { Z } from 'zod-class';
export class MoveDataStoreColumnDto extends Z.class({
targetIndex: z.number(),
targetIndex: z.number().int().nonnegative(),
}) {}

View File

@@ -1933,7 +1933,7 @@ describe('POST /projects/:projectId/data-stores/:dataStoreId/insert', () => {
const response = await authMemberAgent
.post(`/projects/${memberProject.id}/data-stores/${dataStore.id}/insert`)
.send(payload)
.expect(500);
.expect(400);
expect(response.body.message).toContain('unknown column');
const rowsInDb = await dataStoreRowsRepository.getManyAndCount(toTableName(dataStore.id), {});
@@ -2174,7 +2174,7 @@ describe('POST /projects/:projectId/data-stores/:dataStoreId/upsert', () => {
const response = await authMemberAgent
.post(`/projects/${memberProject.id}/data-stores/${dataStore.id}/upsert`)
.send(payload)
.expect(500);
.expect(400);
expect(response.body.message).toContain('unknown column');
const rowsInDb = await dataStoreRowsRepository.getManyAndCount(toTableName(dataStore.id), {});

View File

@@ -6,6 +6,11 @@ import { Container } from '@n8n/di';
import { DataStoreRowsRepository } from '../data-store-rows.repository';
import { DataStoreRepository } from '../data-store.repository';
import { DataStoreService } from '../data-store.service';
import { DataStoreColumnNameConflictError } from '../errors/data-store-column-name-conflict.error';
import { DataStoreColumnNotFoundError } from '../errors/data-store-column-not-found.error';
import { DataStoreNameConflictError } from '../errors/data-store-name-conflict.error';
import { DataStoreNotFoundError } from '../errors/data-store-not-found.error';
import { DataStoreValidationError } from '../errors/data-store-validation.error';
import { toTableName } from '../utils/sql-utils';
beforeAll(async () => {
@@ -172,9 +177,7 @@ describe('dataStore', () => {
});
// ASSERT
await expect(result).rejects.toThrow(
`Data store with name '${name}' already exists in this project`,
);
await expect(result).rejects.toThrow(DataStoreNameConflictError);
});
});
@@ -209,7 +212,7 @@ describe('dataStore', () => {
});
// ASSERT
await expect(result).rejects.toThrow("Data Store 'this is not an id' does not exist.");
await expect(result).rejects.toThrow(DataStoreNotFoundError);
});
it('should fail when renaming to a taken name', async () => {
@@ -229,9 +232,7 @@ describe('dataStore', () => {
const result = dataStoreService.updateDataStore(dataStoreNewId, project1.id, { name });
// ASSERT
await expect(result).rejects.toThrow(
`Data store with name '${name}' already exists in this project`,
);
await expect(result).rejects.toThrow(DataStoreNameConflictError);
});
});
@@ -266,9 +267,7 @@ describe('dataStore', () => {
const result = dataStoreService.deleteDataStore('this is not an id', project1.id);
// ASSERT
await expect(result).rejects.toThrow(
"Tried to delete non-existent data store 'this is not an id'",
);
await expect(result).rejects.toThrow(DataStoreNotFoundError);
});
});
@@ -404,9 +403,7 @@ describe('dataStore', () => {
});
// ASSERT
await expect(result).rejects.toThrow(
`column name 'myColumn1' already taken in data store '${dataStoreId}'`,
);
await expect(result).rejects.toThrow(DataStoreColumnNameConflictError);
});
it('should fail with adding column of non-existent table', async () => {
@@ -417,9 +414,7 @@ describe('dataStore', () => {
});
// ASSERT
await expect(result).rejects.toThrow(
"Tried to add column to non-existent data store 'this is not an id'",
);
await expect(result).rejects.toThrow(DataStoreNotFoundError);
});
});
@@ -477,9 +472,7 @@ describe('dataStore', () => {
const result = dataStoreService.deleteColumn(dataStoreId, project1.id, 'thisIsNotAnId');
// ASSERT
await expect(result).rejects.toThrow(
`Tried to delete column with name not present in data store '${dataStoreId}'`,
);
await expect(result).rejects.toThrow(DataStoreColumnNotFoundError);
});
it('should fail when deleting column from unknown table', async () => {
@@ -497,9 +490,7 @@ describe('dataStore', () => {
const result = dataStoreService.deleteColumn('this is not an id', project1.id, c1.id);
// ASSERT
await expect(result).rejects.toThrow(
"Tried to delete column from non-existent data store 'this is not an id'",
);
await expect(result).rejects.toThrow(DataStoreNotFoundError);
});
});
@@ -960,7 +951,7 @@ describe('dataStore', () => {
]);
// ASSERT
await expect(result).rejects.toThrow('mismatched key count');
await expect(result).rejects.toThrow(new DataStoreValidationError('mismatched key count'));
});
it('rejects a mismatched row with missing column', async () => {
@@ -982,7 +973,7 @@ describe('dataStore', () => {
]);
// ASSERT
await expect(result).rejects.toThrow('mismatched key count');
await expect(result).rejects.toThrow(new DataStoreValidationError('mismatched key count'));
});
it('rejects a mismatched row with replaced column', async () => {
@@ -1004,7 +995,7 @@ describe('dataStore', () => {
]);
// ASSERT
await expect(result).rejects.toThrow('unknown column name');
await expect(result).rejects.toThrow(new DataStoreValidationError('unknown column name'));
});
it('rejects unknown data store id', async () => {
@@ -1026,7 +1017,7 @@ describe('dataStore', () => {
]);
// ASSERT
await expect(result).rejects.toThrow("Data Store 'this is not an id' does not exist.");
await expect(result).rejects.toThrow(DataStoreNotFoundError);
});
it('rejects on empty column list', async () => {
@@ -1041,7 +1032,9 @@ describe('dataStore', () => {
// ASSERT
await expect(result).rejects.toThrow(
'No columns found for this data store or data store not found',
new DataStoreValidationError(
'No columns found for this data store or data store not found',
),
);
});
@@ -1059,7 +1052,9 @@ describe('dataStore', () => {
]);
// ASSERT
await expect(result).rejects.toThrow("value 'true' does not match column type 'number'");
await expect(result).rejects.toThrow(
new DataStoreValidationError("value 'true' does not match column type 'number'"),
);
});
});

View File

@@ -1,12 +1,12 @@
import { DataStoreCreateColumnSchema } from '@n8n/api-types';
import { Service } from '@n8n/di';
import { DataSource, EntityManager, Repository } from '@n8n/typeorm';
import { UnexpectedError, UserError } from 'n8n-workflow';
import { UnexpectedError } from 'n8n-workflow';
import { DataStoreColumn } from './data-store-column.entity';
import { DataStoreRowsRepository } from './data-store-rows.repository';
import { NotFoundError } from '@/errors/response-errors/not-found.error';
import { BadRequestError } from '@/errors/response-errors/bad-request.error';
import { DataStoreColumnNameConflictError } from './errors/data-store-column-name-conflict.error';
import { DataStoreValidationError } from './errors/data-store-validation.error';
@Service()
export class DataStoreColumnRepository extends Repository<DataStoreColumn> {
@@ -40,9 +40,7 @@ export class DataStoreColumnRepository extends Repository<DataStoreColumn> {
});
if (existingColumnMatch) {
throw new UserError(
`column name '${schema.name}' already taken in data store '${dataStoreId}'`,
);
throw new DataStoreColumnNameConflictError(schema.name, dataStoreId);
}
if (schema.index === undefined) {
@@ -88,32 +86,23 @@ export class DataStoreColumnRepository extends Repository<DataStoreColumn> {
});
}
async moveColumn(rawDataStoreId: string, columnId: string, targetIndex: number) {
async moveColumn(dataStoreId: string, column: DataStoreColumn, targetIndex: number) {
await this.manager.transaction(async (em) => {
const existingColumn = await em.findOneBy(DataStoreColumn, {
id: columnId,
dataStoreId: rawDataStoreId,
});
const columnCount = await em.countBy(DataStoreColumn, { dataStoreId });
if (existingColumn === null) {
throw new NotFoundError(
`tried to move column not present in data store '${rawDataStoreId}'`,
if (targetIndex < 0) {
throw new DataStoreValidationError('tried to move column to negative index');
}
if (targetIndex >= columnCount) {
throw new DataStoreValidationError(
'tried to move column to an index larger than column count',
);
}
const columnCount = await em.countBy(DataStoreColumn, { dataStoreId: rawDataStoreId });
if (targetIndex >= columnCount) {
throw new BadRequestError('tried to move column to index larger than column count');
}
if (targetIndex < 0) {
throw new BadRequestError('tried to move column to negative index');
}
await this.shiftColumns(rawDataStoreId, existingColumn.index, -1, em);
await this.shiftColumns(rawDataStoreId, targetIndex, 1, em);
await em.update(DataStoreColumn, { id: existingColumn.id }, { index: targetIndex });
await this.shiftColumns(dataStoreId, column.index, -1, em);
await this.shiftColumns(dataStoreId, targetIndex, 1, em);
await em.update(DataStoreColumn, { id: column.id }, { index: targetIndex });
});
}

View File

@@ -22,6 +22,16 @@ import {
} from '@n8n/decorators';
import { DataStoreService } from './data-store.service';
import { DataStoreColumnNameConflictError } from './errors/data-store-column-name-conflict.error';
import { DataStoreColumnNotFoundError } from './errors/data-store-column-not-found.error';
import { DataStoreNameConflictError } from './errors/data-store-name-conflict.error';
import { DataStoreNotFoundError } from './errors/data-store-not-found.error';
import { DataStoreValidationError } from './errors/data-store-validation.error';
import { BadRequestError } from '@/errors/response-errors/bad-request.error';
import { ConflictError } from '@/errors/response-errors/conflict.error';
import { InternalServerError } from '@/errors/response-errors/internal-server.error';
import { NotFoundError } from '@/errors/response-errors/not-found.error';
@RestController('/projects/:projectId/data-stores')
export class DataStoreController {
@@ -34,7 +44,17 @@ export class DataStoreController {
_res: Response,
@Body dto: CreateDataStoreDto,
) {
return await this.dataStoreService.createDataStore(req.params.projectId, dto);
try {
return await this.dataStoreService.createDataStore(req.params.projectId, dto);
} catch (e: unknown) {
if (!(e instanceof Error)) {
throw e;
} else if (e instanceof DataStoreNameConflictError) {
throw new ConflictError(e.message);
} else {
throw new InternalServerError(e.message, e);
}
}
}
@Get('/')
@@ -59,7 +79,19 @@ export class DataStoreController {
@Param('dataStoreId') dataStoreId: string,
@Body dto: UpdateDataStoreDto,
) {
return await this.dataStoreService.updateDataStore(dataStoreId, req.params.projectId, dto);
try {
return await this.dataStoreService.updateDataStore(dataStoreId, req.params.projectId, dto);
} catch (e: unknown) {
if (e instanceof DataStoreNotFoundError) {
throw new NotFoundError(e.message);
} else if (e instanceof DataStoreNameConflictError) {
throw new ConflictError(e.message);
} else if (e instanceof Error) {
throw new InternalServerError(e.message, e);
} else {
throw e;
}
}
}
@Delete('/:dataStoreId')
@@ -69,7 +101,17 @@ export class DataStoreController {
_res: Response,
@Param('dataStoreId') dataStoreId: string,
) {
return await this.dataStoreService.deleteDataStore(dataStoreId, req.params.projectId);
try {
return await this.dataStoreService.deleteDataStore(dataStoreId, req.params.projectId);
} catch (e: unknown) {
if (e instanceof DataStoreNotFoundError) {
throw new NotFoundError(e.message);
} else if (e instanceof Error) {
throw new InternalServerError(e.message, e);
} else {
throw e;
}
}
}
@Get('/:dataStoreId/columns')
@@ -79,7 +121,17 @@ export class DataStoreController {
_res: Response,
@Param('dataStoreId') dataStoreId: string,
) {
return await this.dataStoreService.getColumns(dataStoreId, req.params.projectId);
try {
return await this.dataStoreService.getColumns(dataStoreId, req.params.projectId);
} catch (e: unknown) {
if (e instanceof DataStoreNotFoundError) {
throw new NotFoundError(e.message);
} else if (e instanceof Error) {
throw new InternalServerError(e.message, e);
} else {
throw e;
}
}
}
@Post('/:dataStoreId/columns')
@@ -90,7 +142,19 @@ export class DataStoreController {
@Param('dataStoreId') dataStoreId: string,
@Body dto: AddDataStoreColumnDto,
) {
return await this.dataStoreService.addColumn(dataStoreId, req.params.projectId, dto);
try {
return await this.dataStoreService.addColumn(dataStoreId, req.params.projectId, dto);
} catch (e: unknown) {
if (e instanceof DataStoreNotFoundError) {
throw new NotFoundError(e.message);
} else if (e instanceof DataStoreColumnNameConflictError) {
throw new ConflictError(e.message);
} else if (e instanceof Error) {
throw new InternalServerError(e.message, e);
} else {
throw e;
}
}
}
@Delete('/:dataStoreId/columns/:columnId')
@@ -101,7 +165,17 @@ export class DataStoreController {
@Param('dataStoreId') dataStoreId: string,
@Param('columnId') columnId: string,
) {
return await this.dataStoreService.deleteColumn(dataStoreId, req.params.projectId, columnId);
try {
return await this.dataStoreService.deleteColumn(dataStoreId, req.params.projectId, columnId);
} catch (e: unknown) {
if (e instanceof DataStoreNotFoundError || e instanceof DataStoreColumnNotFoundError) {
throw new NotFoundError(e.message);
} else if (e instanceof Error) {
throw new InternalServerError(e.message, e);
} else {
throw e;
}
}
}
@Patch('/:dataStoreId/columns/:columnId/move')
@@ -113,7 +187,24 @@ export class DataStoreController {
@Param('columnId') columnId: string,
@Body dto: MoveDataStoreColumnDto,
) {
return await this.dataStoreService.moveColumn(dataStoreId, req.params.projectId, columnId, dto);
try {
return await this.dataStoreService.moveColumn(
dataStoreId,
req.params.projectId,
columnId,
dto,
);
} catch (e: unknown) {
if (e instanceof DataStoreNotFoundError || e instanceof DataStoreColumnNotFoundError) {
throw new NotFoundError(e.message);
} else if (e instanceof DataStoreValidationError) {
throw new BadRequestError(e.message);
} else if (e instanceof Error) {
throw new InternalServerError(e.message, e);
} else {
throw e;
}
}
}
@Get('/:dataStoreId/rows')
@@ -124,7 +215,21 @@ export class DataStoreController {
@Param('dataStoreId') dataStoreId: string,
@Query dto: ListDataStoreContentQueryDto,
) {
return await this.dataStoreService.getManyRowsAndCount(dataStoreId, req.params.projectId, dto);
try {
return await this.dataStoreService.getManyRowsAndCount(
dataStoreId,
req.params.projectId,
dto,
);
} catch (e: unknown) {
if (e instanceof DataStoreNotFoundError) {
throw new NotFoundError(e.message);
} else if (e instanceof Error) {
throw new InternalServerError(e.message, e);
} else {
throw e;
}
}
}
@Post('/:dataStoreId/insert')
@@ -135,7 +240,19 @@ export class DataStoreController {
@Param('dataStoreId') dataStoreId: string,
@Body dto: AddDataStoreRowsDto,
) {
return await this.dataStoreService.insertRows(dataStoreId, req.params.projectId, dto.data);
try {
return await this.dataStoreService.insertRows(dataStoreId, req.params.projectId, dto.data);
} catch (e: unknown) {
if (e instanceof DataStoreNotFoundError) {
throw new NotFoundError(e.message);
} else if (e instanceof DataStoreValidationError) {
throw new BadRequestError(e.message);
} else if (e instanceof Error) {
throw new InternalServerError(e.message, e);
} else {
throw e;
}
}
}
@Post('/:dataStoreId/upsert')
@@ -146,6 +263,18 @@ export class DataStoreController {
@Param('dataStoreId') dataStoreId: string,
@Body dto: UpsertDataStoreRowsDto,
) {
return await this.dataStoreService.upsertRows(dataStoreId, req.params.projectId, dto);
try {
return await this.dataStoreService.upsertRows(dataStoreId, req.params.projectId, dto);
} catch (e: unknown) {
if (e instanceof DataStoreNotFoundError) {
throw new NotFoundError(e.message);
} else if (e instanceof DataStoreValidationError) {
throw new BadRequestError(e.message);
} else if (e instanceof Error) {
throw new InternalServerError(e.message, e);
} else {
throw e;
}
}
}
}

View File

@@ -10,14 +10,14 @@ import type {
import { UpdateDataStoreDto } from '@n8n/api-types/src/dto/data-store/update-data-store.dto';
import { Logger } from '@n8n/backend-common';
import { Service } from '@n8n/di';
import { UserError } from 'n8n-workflow';
import { ConflictError } from '@/errors/response-errors/conflict.error';
import { NotFoundError } from '@/errors/response-errors/not-found.error';
import { DataStoreColumnRepository } from './data-store-column.repository';
import { DataStoreRowsRepository } from './data-store-rows.repository';
import { DataStoreRepository } from './data-store.repository';
import { DataStoreColumnNotFoundError } from './errors/data-store-column-not-found.error';
import { DataStoreNameConflictError } from './errors/data-store-name-conflict.error';
import { DataStoreNotFoundError } from './errors/data-store-not-found.error';
import { DataStoreValidationError } from './errors/data-store-validation.error';
import { toTableName, normalizeRows } from './utils/sql-utils';
@Service()
@@ -59,11 +59,7 @@ export class DataStoreService {
}
async deleteDataStore(dataStoreId: string, projectId: string) {
await this.validateDataStoreExists(
dataStoreId,
projectId,
`Tried to delete non-existent data store '${dataStoreId}'`,
);
await this.validateDataStoreExists(dataStoreId, projectId);
await this.dataStoreRepository.deleteDataStore(dataStoreId);
@@ -71,11 +67,7 @@ export class DataStoreService {
}
async addColumn(dataStoreId: string, projectId: string, dto: AddDataStoreColumnDto) {
await this.validateDataStoreExists(
dataStoreId,
projectId,
`Tried to add column to non-existent data store '${dataStoreId}'`,
);
await this.validateDataStoreExists(dataStoreId, projectId);
return await this.dataStoreColumnRepository.addColumn(dataStoreId, dto);
}
@@ -86,36 +78,19 @@ export class DataStoreService {
columnId: string,
dto: MoveDataStoreColumnDto,
) {
await this.validateDataStoreExists(
dataStoreId,
projectId,
`Tried to move column from non-existent data store '${dataStoreId}'`,
);
await this.validateDataStoreExists(dataStoreId, projectId);
const existingColumn = await this.validateColumnExists(dataStoreId, columnId);
await this.dataStoreColumnRepository.moveColumn(dataStoreId, columnId, dto.targetIndex);
await this.dataStoreColumnRepository.moveColumn(dataStoreId, existingColumn, dto.targetIndex);
return true;
}
async deleteColumn(dataStoreId: string, projectId: string, columnId: string) {
await this.validateDataStoreExists(
dataStoreId,
projectId,
`Tried to delete column from non-existent data store '${dataStoreId}'`,
);
await this.validateDataStoreExists(dataStoreId, projectId);
const existingColumn = await this.validateColumnExists(dataStoreId, columnId);
const existingColumnMatch = await this.dataStoreColumnRepository.findOneBy({
id: columnId,
dataStoreId,
});
if (existingColumnMatch === null) {
throw new NotFoundError(
`Tried to delete column with name not present in data store '${dataStoreId}'`,
);
}
await this.dataStoreColumnRepository.deleteColumn(dataStoreId, existingColumnMatch);
await this.dataStoreColumnRepository.deleteColumn(dataStoreId, existingColumn);
return true;
}
@@ -156,7 +131,6 @@ export class DataStoreService {
await this.validateRows(dataStoreId, rows);
const columns = await this.dataStoreColumnRepository.getColumns(dataStoreId);
return await this.dataStoreRowsRepository.insertRows(toTableName(dataStoreId), rows, columns);
}
@@ -172,7 +146,9 @@ export class DataStoreService {
private async validateRows(dataStoreId: string, rows: DataStoreRows): Promise<void> {
const columns = await this.dataStoreColumnRepository.getColumns(dataStoreId);
if (columns.length === 0) {
throw new UserError('No columns found for this data store or data store not found');
throw new DataStoreValidationError(
'No columns found for this data store or data store not found',
);
}
const columnNames = new Set(columns.map((x) => x.name));
@@ -180,40 +156,46 @@ export class DataStoreService {
for (const row of rows) {
const keys = Object.keys(row);
if (columns.length !== keys.length) {
throw new UserError('mismatched key count');
throw new DataStoreValidationError('mismatched key count');
}
for (const key of keys) {
if (!columnNames.has(key)) {
throw new UserError('unknown column name');
throw new DataStoreValidationError('unknown column name');
}
const cell = row[key];
if (cell === null) continue;
switch (columnTypeMap.get(key)) {
case 'boolean':
if (typeof cell !== 'boolean')
throw new UserError(
throw new DataStoreValidationError(
`value '${cell.toString()}' does not match column type 'boolean'`,
);
break;
case 'date':
if (!(cell instanceof Date))
throw new UserError(`value '${cell}' does not match column type 'date'`);
throw new DataStoreValidationError(
`value '${cell}' does not match column type 'date'`,
);
row[key] = cell.toISOString();
break;
case 'string':
if (typeof cell !== 'string')
throw new UserError(`value '${cell.toString()}' does not match column type 'string'`);
throw new DataStoreValidationError(
`value '${cell.toString()}' does not match column type 'string'`,
);
break;
case 'number':
if (typeof cell !== 'number')
throw new UserError(`value '${cell.toString()}' does not match column type 'number'`);
throw new DataStoreValidationError(
`value '${cell.toString()}' does not match column type 'number'`,
);
break;
}
}
}
}
private async validateDataStoreExists(dataStoreId: string, projectId: string, msg?: string) {
private async validateDataStoreExists(dataStoreId: string, projectId: string) {
const existingTable = await this.dataStoreRepository.findOneBy({
id: dataStoreId,
project: {
@@ -222,22 +204,33 @@ export class DataStoreService {
});
if (!existingTable) {
throw new NotFoundError(msg ?? `Data Store '${dataStoreId}' does not exist.`);
throw new DataStoreNotFoundError(dataStoreId);
}
return existingTable;
}
private async validateUniqueName(name: string, projectId: string, msg?: string) {
private async validateColumnExists(dataStoreId: string, columnId: string) {
const existingColumn = await this.dataStoreColumnRepository.findOneBy({
id: columnId,
dataStoreId,
});
if (existingColumn === null) {
throw new DataStoreColumnNotFoundError(dataStoreId, columnId);
}
return existingColumn;
}
private async validateUniqueName(name: string, projectId: string) {
const hasNameClash = await this.dataStoreRepository.existsBy({
name,
projectId,
});
if (hasNameClash) {
throw new ConflictError(
msg ?? `Data store with name '${name}' already exists in this project`,
);
throw new DataStoreNameConflictError(name);
}
}
}

View File

@@ -0,0 +1,12 @@
import { UserError } from 'n8n-workflow';
export class DataStoreColumnNameConflictError extends UserError {
constructor(columnName: string, dataStoreId: string) {
super(
`Data store column with name '${columnName}' already exists in data store '${dataStoreId}'`,
{
level: 'warning',
},
);
}
}

View File

@@ -0,0 +1,9 @@
import { UserError } from 'n8n-workflow';
export class DataStoreColumnNotFoundError extends UserError {
constructor(dataStoreId: string, columnId: string) {
super(`Could not find the column '${columnId}' in the data store: ${dataStoreId}`, {
level: 'warning',
});
}
}

View File

@@ -0,0 +1,9 @@
import { UserError } from 'n8n-workflow';
export class DataStoreNameConflictError extends UserError {
constructor(name: string) {
super(`Data store with name '${name}' already exists in this project`, {
level: 'warning',
});
}
}

View File

@@ -0,0 +1,9 @@
import { UserError } from 'n8n-workflow';
export class DataStoreNotFoundError extends UserError {
constructor(dataStoreId: string) {
super(`Could not find the data store: '${dataStoreId}'`, {
level: 'warning',
});
}
}

View File

@@ -0,0 +1,9 @@
import { UserError } from 'n8n-workflow';
export class DataStoreValidationError extends UserError {
constructor(msg: string) {
super(`Validation error with data store request: ${msg}`, {
level: 'warning',
});
}
}