refactor(server)!: structured validation error responses (#28204)

* refactor(server)!: structured validation error responses

* refactor(server): clarify comment on removing duplicate HTTP response fields

* enhance validation error tests

* make path and message required

* fmt

* fix e2e test

* fmt

* feat: enhance error handling in getServerErrorMessage function
This commit is contained in:
Timon
2026-05-04 06:00:03 +02:00
committed by GitHub
parent eca0e60db8
commit 3decc864b5
33 changed files with 456 additions and 185 deletions
+4 -3
View File
@@ -28,6 +28,10 @@ export const errorDto = {
badRequest: (message: any = null) => ({
message: message ?? expect.anything(),
}),
validationError: (errors?: ReadonlyArray<{ path: ReadonlyArray<string | number>; message: string }>) => ({
message: 'Validation failed',
errors: errors ? expect.arrayContaining(errors.map((e) => expect.objectContaining(e))) : expect.any(Array),
}),
noPermission: {
message: expect.stringContaining('Not found or no'),
},
@@ -37,9 +41,6 @@ export const errorDto = {
alreadyHasAdmin: {
message: 'The server already has an admin',
},
invalidEmail: {
message: ['email must be an email'],
},
};
export const signupResponseDto = {
+21 -7
View File
@@ -110,7 +110,9 @@ describe('/libraries', () => {
});
expect(status).toBe(400);
expect(body).toEqual(errorDto.badRequest(['[importPaths] Array must have unique items']));
expect(body).toEqual(
errorDto.validationError([{ path: ['importPaths'], message: 'Array must have unique items' }]),
);
});
it('should not create an external library with duplicate exclusion patterns', async () => {
@@ -125,7 +127,9 @@ describe('/libraries', () => {
});
expect(status).toBe(400);
expect(body).toEqual(errorDto.badRequest(['[exclusionPatterns] Array must have unique items']));
expect(body).toEqual(
errorDto.validationError([{ path: ['exclusionPatterns'], message: 'Array must have unique items' }]),
);
});
});
@@ -157,7 +161,9 @@ describe('/libraries', () => {
.send({ name: '' });
expect(status).toBe(400);
expect(body).toEqual(errorDto.badRequest(['[name] Too small: expected string to have >=1 characters']));
expect(body).toEqual(
errorDto.validationError([{ path: ['name'], message: 'Too small: expected string to have >=1 characters' }]),
);
});
it('should change the import paths', async () => {
@@ -181,7 +187,9 @@ describe('/libraries', () => {
.send({ importPaths: [''] });
expect(status).toBe(400);
expect(body).toEqual(errorDto.badRequest(['[importPaths] Array items must not be empty']));
expect(body).toEqual(
errorDto.validationError([{ path: ['importPaths'], message: 'Array items must not be empty' }]),
);
});
it('should reject duplicate import paths', async () => {
@@ -191,7 +199,9 @@ describe('/libraries', () => {
.send({ importPaths: ['/path', '/path'] });
expect(status).toBe(400);
expect(body).toEqual(errorDto.badRequest(['[importPaths] Array must have unique items']));
expect(body).toEqual(
errorDto.validationError([{ path: ['importPaths'], message: 'Array must have unique items' }]),
);
});
it('should change the exclusion pattern', async () => {
@@ -215,7 +225,9 @@ describe('/libraries', () => {
.send({ exclusionPatterns: ['**/*.jpg', '**/*.jpg'] });
expect(status).toBe(400);
expect(body).toEqual(errorDto.badRequest(['[exclusionPatterns] Array must have unique items']));
expect(body).toEqual(
errorDto.validationError([{ path: ['exclusionPatterns'], message: 'Array must have unique items' }]),
);
});
it('should reject an empty exclusion pattern', async () => {
@@ -225,7 +237,9 @@ describe('/libraries', () => {
.send({ exclusionPatterns: [''] });
expect(status).toBe(400);
expect(body).toEqual(errorDto.badRequest(['[exclusionPatterns] Array items must not be empty']));
expect(body).toEqual(
errorDto.validationError([{ path: ['exclusionPatterns'], message: 'Array items must not be empty' }]),
);
});
});
+12 -4
View File
@@ -109,7 +109,9 @@ describe('/map', () => {
.get('/map/reverse-geocode?lon=123')
.set('Authorization', `Bearer ${admin.accessToken}`);
expect(status).toBe(400);
expect(body).toEqual(errorDto.badRequest(['[lat] Invalid input: expected number, received NaN']));
expect(body).toEqual(
errorDto.validationError([{ path: ['lat'], message: 'Invalid input: expected number, received NaN' }]),
);
});
it('should throw an error if a lat is not a number', async () => {
@@ -117,7 +119,9 @@ describe('/map', () => {
.get('/map/reverse-geocode?lat=abc&lon=123.456')
.set('Authorization', `Bearer ${admin.accessToken}`);
expect(status).toBe(400);
expect(body).toEqual(errorDto.badRequest(['[lat] Invalid input: expected number, received NaN']));
expect(body).toEqual(
errorDto.validationError([{ path: ['lat'], message: 'Invalid input: expected number, received NaN' }]),
);
});
it('should throw an error if a lat is out of range', async () => {
@@ -125,7 +129,9 @@ describe('/map', () => {
.get('/map/reverse-geocode?lat=91&lon=123.456')
.set('Authorization', `Bearer ${admin.accessToken}`);
expect(status).toBe(400);
expect(body).toEqual(errorDto.badRequest(['[lat] Too big: expected number to be <=90']));
expect(body).toEqual(
errorDto.validationError([{ path: ['lat'], message: 'Too big: expected number to be <=90' }]),
);
});
it('should throw an error if a lon is not provided', async () => {
@@ -133,7 +139,9 @@ describe('/map', () => {
.get('/map/reverse-geocode?lat=75')
.set('Authorization', `Bearer ${admin.accessToken}`);
expect(status).toBe(400);
expect(body).toEqual(errorDto.badRequest(['[lon] Invalid input: expected number, received NaN']));
expect(body).toEqual(
errorDto.validationError([{ path: ['lon'], message: 'Invalid input: expected number, received NaN' }]),
);
});
const reverseGeocodeTestCases = [
+16 -4
View File
@@ -105,7 +105,11 @@ describe(`/oauth`, () => {
it(`should throw an error if a redirect uri is not provided`, async () => {
const { status, body } = await request(app).post('/oauth/authorize').send({});
expect(status).toBe(400);
expect(body).toEqual(errorDto.badRequest(['[redirectUri] Invalid input: expected string, received undefined']));
expect(body).toEqual(
errorDto.validationError([
{ path: ['redirectUri'], message: 'Invalid input: expected string, received undefined' },
]),
);
});
it('should return a redirect uri', async () => {
@@ -164,13 +168,17 @@ describe(`/oauth`, () => {
it(`should throw an error if a url is not provided`, async () => {
const { status, body } = await request(app).post('/oauth/callback').send({});
expect(status).toBe(400);
expect(body).toEqual(errorDto.badRequest(['[url] Invalid input: expected string, received undefined']));
expect(body).toEqual(
errorDto.validationError([{ path: ['url'], message: 'Invalid input: expected string, received undefined' }]),
);
});
it(`should throw an error if the url is empty`, async () => {
const { status, body } = await request(app).post('/oauth/callback').send({ url: '' });
expect(status).toBe(400);
expect(body).toEqual(errorDto.badRequest(['[url] Too small: expected string to have >=1 characters']));
expect(body).toEqual(
errorDto.validationError([{ path: ['url'], message: 'Too small: expected string to have >=1 characters' }]),
);
});
it(`should throw an error if the state is not provided`, async () => {
@@ -375,7 +383,11 @@ describe(`/oauth`, () => {
it(`should throw an error if the logout_token is not provided`, async () => {
const { status, body } = await request(app).post('/oauth/backchannel-logout').send({});
expect(status).toBe(400);
expect(body).toEqual(errorDto.badRequest(['[logout_token] Invalid input: expected string, received undefined']));
expect(body).toEqual(
errorDto.validationError([
{ path: ['logout_token'], message: 'Invalid input: expected string, received undefined' },
]),
);
});
it(`should throw an error if an invalid logout token is provided`, async () => {
@@ -341,7 +341,9 @@ describe('/shared-links', () => {
.set('Authorization', `Bearer ${user1.accessToken}`);
expect(status).toBe(400);
expect(body).toEqual(errorDto.badRequest());
expect(body).toEqual(
errorDto.validationError([{ path: [], message: 'Invalid input: expected object, received undefined' }]),
);
});
it('should require an asset/album id', async () => {
+9 -2
View File
@@ -41,7 +41,9 @@ describe('/stacks', () => {
.send({ assetIds: [asset.id] });
expect(status).toBe(400);
expect(body).toEqual(errorDto.badRequest());
expect(body).toEqual(
errorDto.validationError([{ path: ['assetIds'], message: 'Too small: expected array to have >=2 items' }]),
);
});
it('should require a valid id', async () => {
@@ -51,7 +53,12 @@ describe('/stacks', () => {
.send({ assetIds: [uuidDto.invalid, uuidDto.invalid] });
expect(status).toBe(400);
expect(body).toEqual(errorDto.badRequest());
expect(body).toEqual(
errorDto.validationError([
{ path: ['assetIds', 0], message: 'Invalid UUID' },
{ path: ['assetIds', 1], message: 'Invalid UUID' },
]),
);
});
it('should require access', async () => {
+2 -2
View File
@@ -309,7 +309,7 @@ describe('/tags', () => {
.get(`/tags/${uuidDto.invalid}`)
.set('Authorization', `Bearer ${admin.accessToken}`);
expect(status).toBe(400);
expect(body).toEqual(errorDto.badRequest(['[id] Invalid UUID']));
expect(body).toEqual(errorDto.validationError([{ path: ['id'], message: 'Invalid UUID' }]));
});
it('should get tag details', async () => {
@@ -427,7 +427,7 @@ describe('/tags', () => {
.delete(`/tags/${uuidDto.invalid}`)
.set('Authorization', `Bearer ${admin.accessToken}`);
expect(status).toBe(400);
expect(body).toEqual(errorDto.badRequest(['[id] Invalid UUID']));
expect(body).toEqual(errorDto.validationError([{ path: ['id'], message: 'Invalid UUID' }]));
});
it('should delete a tag', async () => {
@@ -108,14 +108,20 @@ describe('/admin/users', () => {
expect(body).toEqual(errorDto.forbidden);
});
for (const key of ['password', 'email', 'name', 'quotaSizeInBytes', 'shouldChangePassword', 'notify']) {
for (const [key, message] of [
['password', 'Invalid input: expected string, received null'],
['email', 'Invalid input: expected email, received object'],
['name', 'Invalid input: expected string, received null'],
['shouldChangePassword', 'Invalid input: expected boolean, received null'],
['notify', 'Invalid input: expected boolean, received null'],
] as const) {
it(`should not allow null ${key}`, async () => {
const { status, body } = await request(app)
.post(`/admin/users`)
.set('Authorization', `Bearer ${admin.accessToken}`)
.send({ ...createUserDto.user1, [key]: null });
expect(status).toBe(400);
expect(body).toEqual(errorDto.badRequest());
expect(body).toEqual(errorDto.validationError([{ path: [key], message }]));
});
}
@@ -153,14 +159,19 @@ describe('/admin/users', () => {
expect(body).toEqual(errorDto.forbidden);
});
for (const key of ['password', 'email', 'name', 'shouldChangePassword']) {
for (const [key, message] of [
['password', 'Invalid input: expected string, received null'],
['email', 'Invalid input: expected email, received object'],
['name', 'Invalid input: expected string, received null'],
['shouldChangePassword', 'Invalid input: expected boolean, received null'],
] as const) {
it(`should not allow null ${key}`, async () => {
const { status, body } = await request(app)
.put(`/admin/users/${uuidDto.notFound}`)
.set('Authorization', `Bearer ${admin.accessToken}`)
.send({ [key]: null });
expect(status).toBe(400);
expect(body).toEqual(errorDto.badRequest());
expect(body).toEqual(errorDto.validationError([{ path: [key], message }]));
});
}
+6 -2
View File
@@ -179,7 +179,9 @@ describe('/users', () => {
expect(status).toBe(400);
expect(body).toEqual(
errorDto.badRequest(['[download.archiveSize] Invalid input: expected int, received number']),
errorDto.validationError([
{ path: ['download', 'archiveSize'], message: 'Invalid input: expected int, received number' },
]),
);
});
@@ -207,7 +209,9 @@ describe('/users', () => {
expect(status).toBe(400);
expect(body).toEqual(
errorDto.badRequest(['[download.includeEmbeddedVideos] Invalid input: expected boolean, received number']),
errorDto.validationError([
{ path: ['download', 'includeEmbeddedVideos'], message: 'Invalid input: expected boolean, received number' },
]),
);
});