From 8c6e4e9019049d8b38d46ec934b6d04a7064cc66 Mon Sep 17 00:00:00 2001 From: Jason Rasmussen Date: Wed, 3 Jun 2026 12:23:06 -0400 Subject: [PATCH] fix: error log on aborted uploads --- server/src/controllers/sync.controller.ts | 8 ++++---- server/src/middleware/error.interceptor.ts | 10 ++++++---- server/src/middleware/file-upload.interceptor.ts | 6 +++++- server/src/middleware/global-exception.filter.ts | 13 +++++++------ server/src/utils/logger.ts | 13 +++++++++++-- 5 files changed, 33 insertions(+), 17 deletions(-) diff --git a/server/src/controllers/sync.controller.ts b/server/src/controllers/sync.controller.ts index c9f3fa7825..b696db8212 100644 --- a/server/src/controllers/sync.controller.ts +++ b/server/src/controllers/sync.controller.ts @@ -1,6 +1,6 @@ -import { Body, Controller, Delete, Get, Header, HttpCode, HttpStatus, Post, Res } from '@nestjs/common'; +import { Body, Controller, Delete, Get, Header, HttpCode, HttpStatus, Post, Req, Res } from '@nestjs/common'; import { ApiTags } from '@nestjs/swagger'; -import { Response } from 'express'; +import { Request, Response } from 'express'; import { Endpoint, HistoryBuilder } from 'src/decorators'; import { AuthDto } from 'src/dtos/auth.dto'; import { SyncAckDeleteDto, SyncAckDto, SyncAckSetDto, SyncStreamDto } from 'src/dtos/sync.dto'; @@ -27,12 +27,12 @@ export class SyncController { 'Retrieve a JSON lines streamed response of changes for synchronization. This endpoint is used by the mobile app to efficiently stay up to date with changes.', history: new HistoryBuilder().added('v1').beta('v1').stable('v2'), }) - async getSyncStream(@Auth() auth: AuthDto, @Res() res: Response, @Body() dto: SyncStreamDto) { + async getSyncStream(@Auth() auth: AuthDto, @Req() req: Request, @Res() res: Response, @Body() dto: SyncStreamDto) { try { await this.service.stream(auth, res, dto); } catch (error: Error | any) { res.setHeader('Content-Type', 'application/json'); - this.errorService.handleError(res, error); + this.errorService.handleError(req, res, error); } } diff --git a/server/src/middleware/error.interceptor.ts b/server/src/middleware/error.interceptor.ts index 3c0c09aa54..2cf5369e98 100644 --- a/server/src/middleware/error.interceptor.ts +++ b/server/src/middleware/error.interceptor.ts @@ -1,14 +1,14 @@ import { CallHandler, ExecutionContext, - HttpException, Injectable, InternalServerErrorException, NestInterceptor, } from '@nestjs/common'; +import { Request } from 'express'; import { Observable, catchError, throwError } from 'rxjs'; import { LoggingRepository } from 'src/repositories/logging.repository'; -import { logGlobalError } from 'src/utils/logger'; +import { isHttpException, onRequestError } from 'src/utils/logger'; import { routeToErrorMessage } from 'src/utils/misc'; @Injectable() @@ -18,14 +18,16 @@ export class ErrorInterceptor implements NestInterceptor { } intercept(context: ExecutionContext, next: CallHandler): Observable { + const req = context.switchToHttp().getRequest(); + return next.handle().pipe( catchError((error) => throwError(() => { - if (error instanceof HttpException) { + if (isHttpException(error)) { return error; } - logGlobalError(this.logger, error); + onRequestError(req, error, this.logger); const message = routeToErrorMessage(context.getHandler().name); return new InternalServerErrorException(message); diff --git a/server/src/middleware/file-upload.interceptor.ts b/server/src/middleware/file-upload.interceptor.ts index 63acb13789..1bbab36a87 100644 --- a/server/src/middleware/file-upload.interceptor.ts +++ b/server/src/middleware/file-upload.interceptor.ts @@ -96,7 +96,11 @@ export class FileUploadInterceptor implements NestInterceptor { private handleFile(request: AuthRequest, file: Express.Multer.File, callback: Callback>) { request.on('error', (error) => { - this.logger.warn('Request error while uploading file, cleaning up', error); + if ('code' in error && error.code === 'ECONNRESET') { + this.logger.debug('Upload was cancelled'); + } else { + this.logger.error(`Upload failed with: ${error}`); + } this.assetService.onUploadError(request, file).catch(this.logger.error); }); diff --git a/server/src/middleware/global-exception.filter.ts b/server/src/middleware/global-exception.filter.ts index 7572274d15..67bba5c358 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 { Response } from 'express'; +import { Request, 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 { onRequestError } from 'src/utils/logger'; import { ZodError } from 'zod'; @Catch() @@ -17,10 +17,13 @@ export class GlobalExceptionFilter implements ExceptionFilter { } catch(error: Error, host: ArgumentsHost) { - this.handleError(host.switchToHttp().getResponse(), error); + const http = host.switchToHttp(); + this.handleError(http.getRequest(), http.getResponse(), error); } - handleError(res: Response, error: Error) { + handleError(req: Request, res: Response, error: Error) { + onRequestError(req, error, this.logger); + const { status, body } = this.fromError(error); if (!res.headersSent) { res.header(ImmichHeader.CorrelationId, this.cls.getId()).status(status).json(body); @@ -28,8 +31,6 @@ export class GlobalExceptionFilter implements ExceptionFilter { } private fromError(error: Error) { - logGlobalError(this.logger, error); - if (error instanceof HttpException) { const status = error.getStatus(); const response = error.getResponse(); diff --git a/server/src/utils/logger.ts b/server/src/utils/logger.ts index ecc8847043..df321f24c2 100644 --- a/server/src/utils/logger.ts +++ b/server/src/utils/logger.ts @@ -1,14 +1,23 @@ import { HttpException } from '@nestjs/common'; +import { Request } from 'express'; import { LoggingRepository } from 'src/repositories/logging.repository'; -export const logGlobalError = (logger: LoggingRepository, error: Error) => { - if (error instanceof HttpException) { +const isRequestAborted = (request: Request) => request.destroyed === true && request.complete === false; +export const isHttpException = (error: Error): error is HttpException => error instanceof HttpException; + +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; + } + if (error instanceof Error) { logger.error(`Unknown error: ${error}`, error?.stack); return;