refactor(server)!: remove redundant error and statusCode fields from error responses (#28140)

* refactor(server)!: remove redundant error and statusCode fields from error responses

* use enum

* enhance response management

* chore: clean up header

* fix: chaining

* refactor: handle error

* fix e2e tests

---------

Co-authored-by: Jason Rasmussen <jason@rasm.me>

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'],
   },
 };

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 });
     });
   });
 });

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 {

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<Error> {
   }
 
   catch(error: Error, host: ArgumentsHost) {
-    const ctx = host.switchToHttp();
+    this.handleError(host.switchToHttp().getResponse<Response>(), error);
-    const response = ctx.getResponse<Response>();
-    const { status, body } = this.fromError(error);
-    if (!response.headersSent) {
-      response.header('X-Correlation-ID', this.cls.getId());
-      response.status(status).json({ ...body, statusCode: status });
-    }
   }
 
   handleError(res: Response, error: Error) {
     const { status, body } = this.fromError(error);
     if (!res.headersSent) {
-      res.header('X-Correlation-ID', this.cls.getId());
+      res.header(ImmichHeader.CorrelationId, this.cls.getId()).status(status).json(body);
-      res.status(status).json({ ...body, statusCode: status });
     }
   }
 
@@ -38,26 +32,24 @@ export class GlobalExceptionFilter implements ExceptionFilter<Error> {
 
     if (error instanceof HttpException) {
       const status = error.getStatus();
-      let body = error.getResponse();
+      const response = error.getResponse();
-
+      const body: Record<string, unknown> =
-      // unclear what circumstances would return a string
+        typeof response === 'string' ? { message: response } : { ...(response as object) };
-      if (typeof body === 'string') {
-        body = { message: body };
-      }
 
       // 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 = {
+          body['message'] = zodError.issues.map((issue) =>
-            message: zodError.issues.map((issue) =>
             issue.path.length > 0 ? `[${issue.path.join('.')}] ${issue.message}` : issue.message,
-            ),
+          );
-            error: 'Bad Request',
-          };
         }
       }
 
+      // 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 };
     }
 

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 cid = req.header(ImmichHeader.CorrelationId) || cls.get(CLS_ID);
-            const headerValue = Array.isArray(headerValues) ? headerValues[0] : headerValues;
-            const cid = headerValue || cls.get(CLS_ID);
             cls.set(CLS_ID, cid);
-            res.header('X-Correlation-ID', cid);
+            res.header(ImmichHeader.CorrelationId, cid);
           },
         },
       },

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',
   },
 };

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(),
     }),
   },