diff --git a/server/src/controllers/integrity-admin.controller.ts b/server/src/controllers/integrity-admin.controller.ts index 10bec75a1..856130ad6 100644 --- a/server/src/controllers/integrity-admin.controller.ts +++ b/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.setHeader('Cache-Control', 'private, no-cache, no-transform'); - res.setHeader('Content-Disposition', `attachment; filename="${encodeURIComponent(`${Date.now()}-${type}.csv`)}"`); + res.header({ + 'Content-Type': 'text/csv', + 'Cache-Control': 'private, no-cache, no-transform', + 'Content-Disposition': `attachment; filename="${encodeURIComponent(`${Date.now()}-${type}.csv`)}"`, + }); this.service.getIntegrityReportCsv(type).pipe(res); } diff --git a/server/src/controllers/sync.controller.ts b/server/src/controllers/sync.controller.ts index 670104f52..922d48f7b 100644 --- a/server/src/controllers/sync.controller.ts +++ b/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); } } diff --git a/server/src/middleware/error.interceptor.ts b/server/src/middleware/error.interceptor.ts index 970db4ac0..22a5f88e7 100644 --- a/server/src/middleware/error.interceptor.ts +++ b/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): Observable { const req = context.switchToHttp().getRequest(); + const res = context.switchToHttp().getResponse(); 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); diff --git a/server/src/middleware/global-exception.filter.ts b/server/src/middleware/global-exception.filter.ts index f232136c1..3d5376845 100644 --- a/server/src/middleware/global-exception.filter.ts +++ b/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 { } 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 = diff --git a/server/src/utils/file.ts b/server/src/utils/file.ts index fd61c286d..abe9ea6ad 100644 --- a/server/src/utils/file.ts +++ b/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 - if (isConnectionAborted(error) || res.headersSent) { - return; + const { canWrite } = onRouteError(undefined, res, error, logger); + if (canWrite) { + next(new NotFoundException()); } - - // log non-http errors - if (!(error instanceof HttpException)) { - logger.error(`Unable to send file: ${error}`, error.stack); - } - - next(new NotFoundException()); } }; diff --git a/server/src/utils/logger.ts b/server/src/utils/logger.ts index d968e3cfa..0586b7451 100644 --- a/server/src/utils/logger.ts +++ b/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; - } - - if (isRequestAborted(req)) { - logger.debug(`Client aborted request: ${error}`); - return; + return { canWrite: true }; } if (error instanceof Error) { logger.error(`Unknown error: ${error}`, error?.stack); - return; + return { canWrite: true }; } + + return { canWrite: true }; }; diff --git a/server/src/utils/misc.ts b/server/src/utils/misc.ts index c9caedd0c..05923522e 100644 --- a/server/src/utils/misc.ts +++ b/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 = (promise: Promise, logger: LoggingRepository): void => { promise.catch((error: Error | any) => logger.error(`Promise error: ${error}`, error?.stack)); };