refactor: error handling (#31455)

server/src/controllers/integrity-admin.controller.ts

@@ -83,9 +83,11 @@ export class IntegrityAdminController {
   @FileResponse()
   @Authenticated({ permission: Permission.Maintenance, admin: true })
   getIntegrityReportCsv(@Param() { type }: IntegrityReportTypeParamDto, @Res() res: Response): void {
-    res.setHeader('Content-Type', 'text/csv');
+    res.header({
-    res.setHeader('Cache-Control', 'private, no-cache, no-transform');
+      'Content-Type': 'text/csv',
-    res.setHeader('Content-Disposition', `attachment; filename="${encodeURIComponent(`${Date.now()}-${type}.csv`)}"`);
+      'Cache-Control': 'private, no-cache, no-transform',
+      'Content-Disposition': `attachment; filename="${encodeURIComponent(`${Date.now()}-${type}.csv`)}"`,
+    });
 
     this.service.getIntegrityReportCsv(type).pipe(res);
   }

server/src/controllers/sync.controller.ts

@@ -31,7 +31,6 @@ export class SyncController {
     try {
       await this.service.stream(auth, res, dto);
     } catch (error: Error | any) {
-      res.setHeader('Content-Type', 'application/json');
       this.errorService.handleError(req, res, error);
     }
   }

server/src/middleware/error.interceptor.ts

@@ -5,10 +5,10 @@ import {
   InternalServerErrorException,
   NestInterceptor,
 } from '@nestjs/common';
-import { Request } from 'express';
+import { Request, Response } from 'express';
 import { Observable, catchError, throwError } from 'rxjs';
 import { LoggingRepository } from 'src/repositories/logging.repository.js';
-import { isHttpException, onRequestError } from 'src/utils/logger.js';
+import { isHttpException, onRouteError } from 'src/utils/logger.js';
 import { routeToErrorMessage } from 'src/utils/misc.js';
 
 @Injectable()
@@ -19,6 +19,7 @@ export class ErrorInterceptor implements NestInterceptor {
 
   intercept(context: ExecutionContext, next: CallHandler<any>): Observable<any> {
     const req = context.switchToHttp().getRequest<Request>();
+    const res = context.switchToHttp().getResponse<Response>();
 
     return next.handle().pipe(
       catchError((error) =>
@@ -27,7 +28,7 @@ export class ErrorInterceptor implements NestInterceptor {
             return error;
           }
 
-          onRequestError(req, error, this.logger);
+          onRouteError(req, res, error, this.logger);
 
           const message = routeToErrorMessage(context.getHandler().name);
           return new InternalServerErrorException(message);

server/src/middleware/global-exception.filter.ts

@@ -1,10 +1,10 @@
-import { ArgumentsHost, Catch, ExceptionFilter, HttpException } from '@nestjs/common';
+import { ArgumentsHost, Catch, ExceptionFilter } from '@nestjs/common';
 import { Request, Response } from 'express';
 import { ClsService } from 'nestjs-cls';
 import { ZodSerializationException, ZodValidationException } from 'nestjs-zod';
 import { ImmichHeader } from 'src/enum.js';
 import { LoggingRepository } from 'src/repositories/logging.repository.js';
-import { onRequestError } from 'src/utils/logger.js';
+import { isHttpException, onRouteError } from 'src/utils/logger.js';
 import { ZodError } from 'zod';
 
 @Catch()
@@ -22,16 +22,24 @@ export class GlobalExceptionFilter implements ExceptionFilter<Error> {
   }
 
   handleError(req: Request, res: Response, error: Error) {
-    onRequestError(req, error, this.logger);
+    const { canWrite } = onRouteError(req, res, error, this.logger);
+    if (!canWrite) {
+      return;
+    }
 
     const { status, body } = this.fromError(error);
-    if (!res.headersSent) {
+
-      res.header(ImmichHeader.CorrelationId, this.cls.getId()).status(status).json(body);
+    res
-    }
+      .header({
+        [ImmichHeader.CorrelationId]: this.cls.getId(),
+        'Content-Type': 'application/json',
+      })
+      .status(status)
+      .json(body);
   }
 
   private fromError(error: Error) {
-    if (error instanceof HttpException) {
+    if (isHttpException(error)) {
       const status = error.getStatus();
       const response = error.getResponse();
       const body: Record<string, unknown> =

server/src/utils/file.ts

@@ -1,4 +1,4 @@
-import { HttpException, NotFoundException, StreamableFile } from '@nestjs/common';
+import { NotFoundException, StreamableFile } from '@nestjs/common';
 import { NextFunction, Response } from 'express';
 import { access, constants } from 'node:fs/promises';
 import { basename, extname } from 'node:path';
@@ -6,7 +6,7 @@ import { promisify } from 'node:util';
 import { CacheControl } from 'src/enum.js';
 import { LoggingRepository } from 'src/repositories/logging.repository.js';
 import { ImmichReadStream } from 'src/repositories/storage.repository.js';
-import { isConnectionAborted } from 'src/utils/misc.js';
+import { onRouteError } from 'src/utils/logger.js';
 
 export function getFileNameWithoutExtension(path: string): string {
   return basename(path, getFilenameExtension(path));
@@ -62,7 +62,7 @@ export const sendFile = async (
     const cacheControlHeader = cacheControlHeaders[file.cacheControl];
     if (cacheControlHeader) {
       // set the header to Cache-Control
-      res.set('Cache-Control', cacheControlHeader);
+      res.header('Cache-Control', cacheControlHeader);
     }
 
     res.header('Content-Type', file.contentType);
@@ -72,17 +72,10 @@ export const sendFile = async (
 
     return await _sendFile(file.path, { dotfiles: 'allow' });
   } catch (error: Error | any) {
-    // ignore client-closed connection
+    const { canWrite } = onRouteError(undefined, res, error, logger);
-    if (isConnectionAborted(error) || res.headersSent) {
+    if (canWrite) {
-      return;
+      next(new NotFoundException());
     }
-
-    // log non-http errors
-    if (!(error instanceof HttpException)) {
-      logger.error(`Unable to send file: ${error}`, error.stack);
-    }
-
-    next(new NotFoundException());
   }
 };
 

server/src/utils/logger.ts

@@ -1,25 +1,29 @@
 import { HttpException } from '@nestjs/common';
-import { Request } from 'express';
+import { Request, Response } from 'express';
 import { LoggingRepository } from 'src/repositories/logging.repository.js';
 
 const isRequestAborted = (request: Request) => request.destroyed && !request.complete;
 export const isHttpException = (error: Error): error is HttpException => error instanceof HttpException;
+export const isConnectionAbortedError = (error: Error | any) => error.code === 'ECONNABORTED';
+
+export const onRouteError = (req: Request | undefined, res: Response, error: Error, logger: LoggingRepository) => {
+  // ignore client-closed connection
+  if (res.headersSent || isConnectionAbortedError(error) || (req && isRequestAborted(req))) {
+    logger.debug(`Client aborted request: ${error}`);
+    return { canWrite: false };
+  }
 
-export const onRequestError = (req: Request, error: Error, logger: LoggingRepository) => {
   if (isHttpException(error)) {
     const status = error.getStatus();
     const response = error.getResponse();
     logger.debug(`HttpException(${status}): ${JSON.stringify(response)}`);
-    return;
+    return { canWrite: true };
-  }
-
-  if (isRequestAborted(req)) {
-    logger.debug(`Client aborted request: ${error}`);
-    return;
   }
 
   if (error instanceof Error) {
     logger.error(`Unknown error: ${error}`, error?.stack);
-    return;
+    return { canWrite: true };
   }
+
+  return { canWrite: true };
 };

server/src/utils/misc.ts

@@ -104,8 +104,6 @@ export const isDuplicateDetectionEnabled = (machineLearning: SystemConfig['machi
   isSmartSearchEnabled(machineLearning) && machineLearning.duplicateDetection.enabled;
 export const isFaceImportEnabled = (metadata: SystemConfig['metadata']) => metadata.faces.import;
 
-export const isConnectionAborted = (error: Error | any) => error.code === 'ECONNABORTED';
-
 export const handlePromiseError = <T>(promise: Promise<T>, logger: LoggingRepository): void => {
   promise.catch((error: Error | any) => logger.error(`Promise error: ${error}`, error?.stack));
 };