diff --git a/e2e/src/responses.ts b/e2e/src/responses.ts index fcb300828e..2ec7aecb0e 100644 --- a/e2e/src/responses.ts +++ b/e2e/src/responses.ts @@ -2,68 +2,42 @@ import { expect } from 'vitest'; export const errorDto = { unauthorized: { - error: 'Unauthorized', - statusCode: 401, message: 'Authentication required', }, unauthorizedWithMessage: (message: string) => ({ - error: 'Unauthorized', - statusCode: 401, message, }), forbidden: { - error: 'Forbidden', - statusCode: 403, message: expect.any(String), }, missingPermission: (permission: string) => ({ - error: 'Forbidden', - statusCode: 403, message: `Missing required permission: ${permission}`, }), wrongPassword: { - error: 'Bad Request', - statusCode: 400, message: 'Wrong password', }, invalidToken: { - error: 'Unauthorized', - statusCode: 401, message: 'Invalid user token', }, invalidShareKey: { - error: 'Unauthorized', - statusCode: 401, message: 'Invalid share key', }, passwordRequired: { - error: 'Unauthorized', - statusCode: 401, message: 'Password required', }, badRequest: (message: any = null) => ({ - error: 'Bad Request', - statusCode: 400, message: message ?? expect.anything(), }), noPermission: { - error: 'Bad Request', - statusCode: 400, message: expect.stringContaining('Not found or no'), }, incorrectLogin: { - error: 'Unauthorized', - statusCode: 401, message: 'Incorrect email or password', }, alreadyHasAdmin: { - error: 'Bad Request', - statusCode: 400, message: 'The server already has an admin', }, invalidEmail: { - error: 'Bad Request', - statusCode: 400, message: ['email must be an email'], }, }; diff --git a/e2e/src/specs/server/api/oauth.e2e-spec.ts b/e2e/src/specs/server/api/oauth.e2e-spec.ts index 9dcb431a4b..157fdfc84c 100644 --- a/e2e/src/specs/server/api/oauth.e2e-spec.ts +++ b/e2e/src/specs/server/api/oauth.e2e-spec.ts @@ -332,9 +332,7 @@ describe(`/oauth`, () => { const { status, body } = await request(app).post('/oauth/callback').send(callbackParams); expect(status).toBe(500); expect(body).toMatchObject({ - error: 'Internal Server Error', message: 'Failed to finish oauth', - statusCode: 500, }); }); @@ -495,11 +493,10 @@ describe(`/oauth`, () => { }); it('should reject OAuth discovery over HTTP', async () => { - const { status, body } = await request(app) + const { status } = await request(app) .post('/oauth/authorize') .send({ redirectUri: 'http://127.0.0.1:2285/auth/login' }); expect(status).toBe(500); - expect(body).toMatchObject({ statusCode: 500 }); }); }); }); diff --git a/server/src/enum.ts b/server/src/enum.ts index fc43f54db1..9ba66145bb 100644 --- a/server/src/enum.ts +++ b/server/src/enum.ts @@ -22,6 +22,7 @@ export enum ImmichHeader { SharedLinkKey = 'x-immich-share-key', SharedLinkSlug = 'x-immich-share-slug', Checksum = 'x-immich-checksum', + CorrelationId = 'X-Correlation-ID', } export enum ImmichQuery { diff --git a/server/src/middleware/global-exception.filter.ts b/server/src/middleware/global-exception.filter.ts index e83ee18fea..f331df9147 100644 --- a/server/src/middleware/global-exception.filter.ts +++ b/server/src/middleware/global-exception.filter.ts @@ -2,6 +2,7 @@ import { ArgumentsHost, Catch, ExceptionFilter, HttpException } from '@nestjs/co import { Response } from 'express'; import { ClsService } from 'nestjs-cls'; import { ZodSerializationException, ZodValidationException } from 'nestjs-zod'; +import { ImmichHeader } from 'src/enum'; import { LoggingRepository } from 'src/repositories/logging.repository'; import { logGlobalError } from 'src/utils/logger'; import { ZodError } from 'zod'; @@ -16,20 +17,13 @@ export class GlobalExceptionFilter implements ExceptionFilter { } catch(error: Error, host: ArgumentsHost) { - const ctx = host.switchToHttp(); - const response = ctx.getResponse(); - const { status, body } = this.fromError(error); - if (!response.headersSent) { - response.header('X-Correlation-ID', this.cls.getId()); - response.status(status).json({ ...body, statusCode: status }); - } + this.handleError(host.switchToHttp().getResponse(), error); } handleError(res: Response, error: Error) { const { status, body } = this.fromError(error); if (!res.headersSent) { - res.header('X-Correlation-ID', this.cls.getId()); - res.status(status).json({ ...body, statusCode: status }); + res.header(ImmichHeader.CorrelationId, this.cls.getId()).status(status).json(body); } } @@ -38,26 +32,24 @@ export class GlobalExceptionFilter implements ExceptionFilter { if (error instanceof HttpException) { const status = error.getStatus(); - let body = error.getResponse(); - - // unclear what circumstances would return a string - if (typeof body === 'string') { - body = { message: body }; - } + const response = error.getResponse(); + const body: Record = + typeof response === 'string' ? { message: response } : { ...(response as object) }; // handle both request and response validation errors if (error instanceof ZodValidationException || error instanceof ZodSerializationException) { const zodError = error.getZodError(); if (zodError instanceof ZodError && zodError.issues.length > 0) { - body = { - message: zodError.issues.map((issue) => - issue.path.length > 0 ? `[${issue.path.join('.')}] ${issue.message}` : issue.message, - ), - error: 'Bad Request', - }; + body['message'] = zodError.issues.map((issue) => + issue.path.length > 0 ? `[${issue.path.join('.')}] ${issue.message}` : issue.message, + ); } } + // remove fields that duplicate the HTTP response line or will be reformatted in a later step + delete body['error']; + delete body['statusCode']; + delete body['errors']; return { status, body }; } diff --git a/server/src/repositories/config.repository.ts b/server/src/repositories/config.repository.ts index b77f9fc8c3..f6bae39897 100644 --- a/server/src/repositories/config.repository.ts +++ b/server/src/repositories/config.repository.ts @@ -15,6 +15,7 @@ import { EnvSchema } from 'src/dtos/env.dto'; import { DatabaseExtension, ImmichEnvironment, + ImmichHeader, ImmichTelemetry, ImmichWorker, LogFormat, @@ -300,11 +301,9 @@ const getEnv = (): EnvData => { mount: true, generateId: true, setup: (cls, req: Request, res: Response) => { - const headerValues = req.headers['x-correlation-id']; - const headerValue = Array.isArray(headerValues) ? headerValues[0] : headerValues; - const cid = headerValue || cls.get(CLS_ID); + const cid = req.header(ImmichHeader.CorrelationId) || cls.get(CLS_ID); cls.set(CLS_ID, cid); - res.header('X-Correlation-ID', cid); + res.header(ImmichHeader.CorrelationId, cid); }, }, }, diff --git a/server/test/medium/responses.ts b/server/test/medium/responses.ts index adff5703c1..2fcab5b2dc 100644 --- a/server/test/medium/responses.ts +++ b/server/test/medium/responses.ts @@ -2,58 +2,36 @@ import { expect } from 'vitest'; export const errorDto = { unauthorized: { - error: 'Unauthorized', - statusCode: 401, message: 'Authentication required', }, forbidden: { - error: 'Forbidden', - statusCode: 403, message: expect.any(String), }, missingPermission: (permission: string) => ({ - error: 'Forbidden', - statusCode: 403, message: `Missing required permission: ${permission}`, }), wrongPassword: { - error: 'Bad Request', - statusCode: 400, message: 'Wrong password', }, invalidToken: { - error: 'Unauthorized', - statusCode: 401, message: 'Invalid user token', }, invalidShareKey: { - error: 'Unauthorized', - statusCode: 401, message: 'Invalid share key', }, invalidSharePassword: { - error: 'Unauthorized', - statusCode: 401, message: 'Invalid password', }, badRequest: (message: any = null) => ({ - error: 'Bad Request', - statusCode: 400, message: message ?? expect.anything(), }), noPermission: { - error: 'Bad Request', - statusCode: 400, message: expect.stringContaining('Not found or no'), }, incorrectLogin: { - error: 'Unauthorized', - statusCode: 401, message: 'Incorrect email or password', }, alreadyHasAdmin: { - error: 'Bad Request', - statusCode: 400, message: 'The server already has an admin', }, }; diff --git a/server/test/small.factory.ts b/server/test/small.factory.ts index e4001d18ab..ad4dbf7524 100644 --- a/server/test/small.factory.ts +++ b/server/test/small.factory.ts @@ -246,8 +246,6 @@ export const factory = { date: newDate, responses: { badRequest: (message: any = null) => ({ - error: 'Bad Request', - statusCode: 400, message: message ?? expect.anything(), }), },