fix: error log on aborted uploads (#28806)
parent
911dde39c9
commit
e4352a7817
|
|
@ -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 { ApiTags } from '@nestjs/swagger';
|
||||||
import { Response } from 'express';
|
import { Request, Response } from 'express';
|
||||||
import { Endpoint, HistoryBuilder } from 'src/decorators';
|
import { Endpoint, HistoryBuilder } from 'src/decorators';
|
||||||
import { AuthDto } from 'src/dtos/auth.dto';
|
import { AuthDto } from 'src/dtos/auth.dto';
|
||||||
import { SyncAckDeleteDto, SyncAckDto, SyncAckSetDto, SyncStreamDto } from 'src/dtos/sync.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.',
|
'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'),
|
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 {
|
try {
|
||||||
await this.service.stream(auth, res, dto);
|
await this.service.stream(auth, res, dto);
|
||||||
} catch (error: Error | any) {
|
} catch (error: Error | any) {
|
||||||
res.setHeader('Content-Type', 'application/json');
|
res.setHeader('Content-Type', 'application/json');
|
||||||
this.errorService.handleError(res, error);
|
this.errorService.handleError(req, res, error);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -1,14 +1,14 @@
|
||||||
import {
|
import {
|
||||||
CallHandler,
|
CallHandler,
|
||||||
ExecutionContext,
|
ExecutionContext,
|
||||||
HttpException,
|
|
||||||
Injectable,
|
Injectable,
|
||||||
InternalServerErrorException,
|
InternalServerErrorException,
|
||||||
NestInterceptor,
|
NestInterceptor,
|
||||||
} from '@nestjs/common';
|
} from '@nestjs/common';
|
||||||
|
import { Request } from 'express';
|
||||||
import { Observable, catchError, throwError } from 'rxjs';
|
import { Observable, catchError, throwError } from 'rxjs';
|
||||||
import { LoggingRepository } from 'src/repositories/logging.repository';
|
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';
|
import { routeToErrorMessage } from 'src/utils/misc';
|
||||||
|
|
||||||
@Injectable()
|
@Injectable()
|
||||||
|
|
@ -18,14 +18,16 @@ export class ErrorInterceptor implements NestInterceptor {
|
||||||
}
|
}
|
||||||
|
|
||||||
intercept(context: ExecutionContext, next: CallHandler<any>): Observable<any> {
|
intercept(context: ExecutionContext, next: CallHandler<any>): Observable<any> {
|
||||||
|
const req = context.switchToHttp().getRequest<Request>();
|
||||||
|
|
||||||
return next.handle().pipe(
|
return next.handle().pipe(
|
||||||
catchError((error) =>
|
catchError((error) =>
|
||||||
throwError(() => {
|
throwError(() => {
|
||||||
if (error instanceof HttpException) {
|
if (isHttpException(error)) {
|
||||||
return error;
|
return error;
|
||||||
}
|
}
|
||||||
|
|
||||||
logGlobalError(this.logger, error);
|
onRequestError(req, error, this.logger);
|
||||||
|
|
||||||
const message = routeToErrorMessage(context.getHandler().name);
|
const message = routeToErrorMessage(context.getHandler().name);
|
||||||
return new InternalServerErrorException(message);
|
return new InternalServerErrorException(message);
|
||||||
|
|
|
||||||
|
|
@ -96,7 +96,11 @@ export class FileUploadInterceptor implements NestInterceptor {
|
||||||
|
|
||||||
private handleFile(request: AuthRequest, file: Express.Multer.File, callback: Callback<Partial<ImmichFile>>) {
|
private handleFile(request: AuthRequest, file: Express.Multer.File, callback: Callback<Partial<ImmichFile>>) {
|
||||||
request.on('error', (error) => {
|
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);
|
this.assetService.onUploadError(request, file).catch(this.logger.error);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -1,10 +1,10 @@
|
||||||
import { ArgumentsHost, Catch, ExceptionFilter, HttpException } from '@nestjs/common';
|
import { ArgumentsHost, Catch, ExceptionFilter, HttpException } from '@nestjs/common';
|
||||||
import { Response } from 'express';
|
import { Request, Response } from 'express';
|
||||||
import { ClsService } from 'nestjs-cls';
|
import { ClsService } from 'nestjs-cls';
|
||||||
import { ZodSerializationException, ZodValidationException } from 'nestjs-zod';
|
import { ZodSerializationException, ZodValidationException } from 'nestjs-zod';
|
||||||
import { ImmichHeader } from 'src/enum';
|
import { ImmichHeader } from 'src/enum';
|
||||||
import { LoggingRepository } from 'src/repositories/logging.repository';
|
import { LoggingRepository } from 'src/repositories/logging.repository';
|
||||||
import { logGlobalError } from 'src/utils/logger';
|
import { onRequestError } from 'src/utils/logger';
|
||||||
import { ZodError } from 'zod';
|
import { ZodError } from 'zod';
|
||||||
|
|
||||||
@Catch()
|
@Catch()
|
||||||
|
|
@ -17,10 +17,13 @@ export class GlobalExceptionFilter implements ExceptionFilter<Error> {
|
||||||
}
|
}
|
||||||
|
|
||||||
catch(error: Error, host: ArgumentsHost) {
|
catch(error: Error, host: ArgumentsHost) {
|
||||||
this.handleError(host.switchToHttp().getResponse<Response>(), error);
|
const http = host.switchToHttp();
|
||||||
|
this.handleError(http.getRequest<Request>(), http.getResponse<Response>(), error);
|
||||||
}
|
}
|
||||||
|
|
||||||
handleError(res: Response, error: Error) {
|
handleError(req: Request, res: Response, error: Error) {
|
||||||
|
onRequestError(req, error, this.logger);
|
||||||
|
|
||||||
const { status, body } = this.fromError(error);
|
const { status, body } = this.fromError(error);
|
||||||
if (!res.headersSent) {
|
if (!res.headersSent) {
|
||||||
res.header(ImmichHeader.CorrelationId, this.cls.getId()).status(status).json(body);
|
res.header(ImmichHeader.CorrelationId, this.cls.getId()).status(status).json(body);
|
||||||
|
|
@ -28,8 +31,6 @@ export class GlobalExceptionFilter implements ExceptionFilter<Error> {
|
||||||
}
|
}
|
||||||
|
|
||||||
private fromError(error: Error) {
|
private fromError(error: Error) {
|
||||||
logGlobalError(this.logger, error);
|
|
||||||
|
|
||||||
if (error instanceof HttpException) {
|
if (error instanceof HttpException) {
|
||||||
const status = error.getStatus();
|
const status = error.getStatus();
|
||||||
const response = error.getResponse();
|
const response = error.getResponse();
|
||||||
|
|
|
||||||
|
|
@ -1,14 +1,23 @@
|
||||||
import { HttpException } from '@nestjs/common';
|
import { HttpException } from '@nestjs/common';
|
||||||
|
import { Request } from 'express';
|
||||||
import { LoggingRepository } from 'src/repositories/logging.repository';
|
import { LoggingRepository } from 'src/repositories/logging.repository';
|
||||||
|
|
||||||
export const logGlobalError = (logger: LoggingRepository, error: Error) => {
|
const isRequestAborted = (request: Request) => request.destroyed === true && request.complete === false;
|
||||||
if (error instanceof HttpException) {
|
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 status = error.getStatus();
|
||||||
const response = error.getResponse();
|
const response = error.getResponse();
|
||||||
logger.debug(`HttpException(${status}): ${JSON.stringify(response)}`);
|
logger.debug(`HttpException(${status}): ${JSON.stringify(response)}`);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (isRequestAborted(req)) {
|
||||||
|
logger.debug(`Client aborted request: ${error}`);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
if (error instanceof Error) {
|
if (error instanceof Error) {
|
||||||
logger.error(`Unknown error: ${error}`, error?.stack);
|
logger.error(`Unknown error: ${error}`, error?.stack);
|
||||||
return;
|
return;
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue