-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add: logging middleware #135
base: dev
Are you sure you want to change the base?
Conversation
src/app.module.ts
Outdated
configure(consumer: MiddlewareConsumer): any { | ||
consumer.apply(AppLoggerMiddleware).forRoutes('*'); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드 리뷰 결과는 다음과 같습니다:
-
버그 위험:
AppLoggerMiddleware
의 구현에 따라 오류가 발생할 수 있습니다. 미들웨어가 요청과 응답을 처리하는 로직에서 예외 처리가 필요합니다.- 모든 경로(
*
)에 대해 미들웨어를 적용하는 것은 부하를 증가시킬 수 있으므로, 특정 경로에만 적용하는 것이 좋습니다.
-
개선 제안:
AppLoggerMiddleware
의 설정 및 사용을 명시적으로 문서화하면 유지보수에 도움이 될 수 있습니다.- 만약 미들웨어에서 상태나 다른 서비스와 상호작용을 한다면, 의존성 주입 방식으로 초기화하는 것이 더 좋습니다.
- 가능하다면 TypeScript 타입을 명확히 하여 코드의 가독성을 높이고 에러를 방지하십시오.
-
일반적인 권장 사항:
- 단위 테스트를 추가하여 미들웨어의 동작을 검증하고, 이상적인 작동을 보장하는 것이 좋습니다.
이러한 점들을 고려하여 코드를 개선하면 더욱 안정적이고 효율적인 애플리케이션을 만들 수 있을 것입니다.
|
||
next(); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드 리뷰 결과는 다음과 같습니다:
장점:
- 명확한 역할: Middleware가 HTTP 요청 로그를 기록하는 역할을 명확히 하고 있습니다.
- Logger 사용: NestJS의 Logger 클래스를 활용하여 일관된 로그 형식을 제공합니다.
잠재적 문제:
-
Content-Length 처리:
content-length
헤더가 비어있을 경우 로그에 null 또는 undefined가 나타날 수 있습니다. 이를 기본값으로 처리하면 좋습니다.const contentLength = response.get('content-length') || '0';
-
비동기 로깅: 현재 로그를 기록하는 과정이 비동기적으로 요청이 끝날 때까지 기다립니다. 응답이 완료되는 시점에서 메시지를 기록하기 때문에, 간혹 동시성이 문제가 될 수 있는 고부하 환경에서는 로그가 유실될 가능성이 있습니다. 일반적으로, 로그는
finish
이벤트로 처리하는 것이 더 안전합니다.
개선 제안:
- 포맷팅 개선: 로그 출력을 좀 더 읽기 쉽게 포맷팅할 수 있습니다. 예를 들어, JSON 형식으로 묶어서 출력하는 방법을 고려할 수 있습니다.
this.logger.log(JSON.stringify({
method,
url,
statusCode,
contentLength,
userAgent,
ip,
}));
- 에러 핸들링: 가능하면 에러 상황도 로그에 포함시키는 것으로 확장하면 유용할 것입니다. 예를 들어 유효하지 않은 요청 시 로그를 남기는 것도 좋은 접근입니다.
이와 같은 점들을 개선하면 더욱 안정적이고 사용자 친화적인 로그 시스템이 될 것입니다.
@@ -88,6 +91,7 @@ | |||
"@types/jest": "28.1.4", | |||
"@types/node": "^16.18.23", | |||
"@types/supertest": "^6.0.2", | |||
"@types/uuid": "^10.0.0", | |||
"@typescript-eslint/eslint-plugin": "^5.0.0", | |||
"@typescript-eslint/parser": "^5.0.0", | |||
"cross-env": "^7.0.3", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드 패치에 대한 간략한 리뷰를 하겠습니다.
-
버전 호환성: 새로운 패키지(
uuid
,winston
,winston-daily-rotate-file
)의 버전이 기존 프로젝트 의존성과 잘 맞는지 확인해야 합니다. 특히winston
과 관련된 패키지는 서로 호환성을 검토하는 것이 좋습니다. -
@types
추가:@types/uuid
를 추가한 것은 좋지만, 나머지 새로 추가된 패키지에도 타입 정의가 필요한 경우, 해당 패키지의 타입을 추가하는지 확인하십시오. -
사용하지 않는 의존성: 추가된 패키지가 실제 코드에서 사용되는지 점검해야 합니다. 필요 없는 의존성이 포함되지 않도록 주의하세요.
-
패키지 보안:
uuid
,winston
등 새로 추가된 패키지의 보안 취약점 여부를 확인할 필요가 있습니다. 정기적으로 보안 스캐닝 도구를 활용하는 것을 추천합니다. -
문서화: 새로 추가된 패키지의 사용법에 대한 문서를 업데이트해야 다른 개발자들이 이해하는 데 도움이 됩니다.
이 외에도 전반적인 테스트 케이스가 잘 작성되어 있는지 점검하는 것도 중요합니다.
@@ -29,6 +29,7 @@ import { ClsModule } from 'nestjs-cls'; | |||
import { ClsPluginTransactional } from '@nestjs-cls/transactional'; | |||
import { PrismaService } from '@src/prisma/prisma.service'; | |||
import { TransactionalAdapterPrisma } from '@nestjs-cls/transactional-adapter-prisma'; | |||
// import { LoggingMiddleware } from "@src/common/middleware/http.logging.middleware"; | |||
|
|||
@Module({ | |||
imports: [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드 패치에 대한 간단한 검토를 하겠습니다.
개선 제안 및 버그 위험
-
미사용 코드 주석 처리:
LoggingMiddleware
임포트 문이 주석 처리되어 있는데, 현재 코드에서 사용되지 않는 경우 지워버리는 것이 깔끔합니다. 불필요한 코드가 프로젝트 관리에 혼란을 줄 수 있습니다.
-
NestModule 구현:
NestModule
인터페이스가 추가되었으나, 실제로 이를 구현하는 코드가 없습니다. 필요하다면configure()
메서드를 추가하여 미들웨어를 설정할 수 있습니다. 그렇지 않으면 불필요한 의존성이 될 수 있습니다.
-
주석의 용도:
- 주석으로 남겨진 코드는 나중에 사용할 계획이 없는 것이라면 삭제하는 게 좋습니다. 주석은 종종 혼란을 초래할 수 있기 때문입니다.
-
모듈 구조:
- 모듈 내 의존성을 명확히 하고, 필요하지 않은 모듈은 제거하도록 검토해야 합니다. 예를 들어,
ClsModule
이나PrismaService
의 목적이 명확하지 않다면 검토해보는 것이 좋습니다.
- 모듈 내 의존성을 명확히 하고, 필요하지 않은 모듈은 제거하도록 검토해야 합니다. 예를 들어,
-
코드 일관성:
- 전체적인 코드 스타일과 일관성을 유지하는 것이 중요합니다. 다른 파일들과 비교하여 변화를 적용하는 것이 좋습니다.
이러한 점들을 고려하여 코드를 개선하면 더 견고하고 이해하기 쉬운 모듈을 만들 수 있을 것입니다.
@@ -53,6 +55,8 @@ async function bootstrap() { | |||
}), | |||
); | |||
|
|||
app.useGlobalInterceptors(new LoggingInterceptor()); | |||
app.useGlobalFilters(new HttpExceptionFilter()); | |||
app.enableShutdownHooks(); | |||
return app.listen(8000); | |||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드 리뷰를 위해 제공된 패치 내용을 살펴보았습니다. 다음은 몇 가지 버그 위험 요소와 개선 제안입니다:
-
버그 위험:
LoggingInterceptor
및HttpExceptionFilter
가 제대로 작동하기 위해서는 이들이 올바르게 구현되어 있어야 합니다. 해당 클래스들이 의존하는 외부 모듈이나 설정이 올바르게 구성되어 있는지 확인해야 합니다.
-
성능:
- 글로벌 인터셉터 및 필터를 추가함에 따라 애플리케이션의 성능에 영향을 줄 수 있습니다. 실제로 로그를 남기거나 예외를 처리할 때 성능 저하가 발생하지 않도록 주의해야 합니다.
-
예외 처리 개선:
HttpExceptionFilter
가 전역적으로 설정되었으므로, 필터 내부에서 모든 에러를 카운트하거나 로깅하는 로직을 추가하여 문제 해결에 도움이 될 수 있습니다.
-
테스트:
- 새로 추가된 인터셉터와 필터에 대한 유닛 테스트를 작성하여 그 동작을 검증하는 것이 좋습니다. 실행 중인 애플리케이션에서의 예상치 못한 동작을 방지할 수 있습니다.
-
주석 추가:
- 코드의 의도를 명확히 하기 위해 주요 변경 사항에 대한 주석을 추가하면 향후 유지보수에 도움이 될 것입니다.
-
환경 변수 관리:
- 포트 번호(8000)가 하드 코드되어 있는데, 환경 변수를 사용해 더 유연하게 처리하는 방법을 고려해 볼 수 있습니다.
이러한 점들을 참고하여 코드를 보완하면 보다 안정적이고 효율적인 애플리케이션이 될 것입니다.
path: request.url, | ||
}); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드 리뷰 결과는 다음과 같습니다:
-
예외 처리:
requestData
를 처리할 때,method
가 'PUT' 또는 'DELETE'와 같은 다른 HTTP 메소드에 대한 처리가 누락되었습니다. 해당 경우도 고려하여 로직을 추가하는 것이 좋습니다.
-
로깅 정보:
response.body
를 사용하고 있으나, Express의 응답 객체에는 body 프로퍼티가 존재하지 않습니다. 제대로 된 오류 메시지를 기록하기 위해서는 예외에서 발생한 내용을 대신 로깅해야 합니다.
-
UUID 생성:
- 요청 헤더에서 UUID를 가져오고 없는 경우 새로 생성하는 방식은 괜찮지만, 실패한 예외에 대해서만 이런 UUID를 사용하는 것도 한 방법입니다. 더 명확한 트래킹을 위해 이 방식을 고민해볼 수 있습니다.
-
변수명 일관성:
requestLog
와errorResponseLog
와 같이 변수명을 좀 더 직관적으로 직무에 맞게 지정하면 가독성이 향상될 것입니다. 예를 들면,errorResponseLog
는exceptionLog
로 변경할 수 있습니다.
-
불필요한 주석:
// meta
와 같은 불필요한 주석은 삭제하세요. 이는 코드 가독성을 떨어뜨립니다.
-
로그 형식:
- JSON으로 로그를 작성하고 있지만,
customFormat
에서 단순히 문자열 변환을 통해 출력하는 것은 가독성을 낮출 수 있습니다. 필요한 필드를 명시적으로 추출하여 logs 쿼리를 개선할 수 있습니다.
- JSON으로 로그를 작성하고 있지만,
-
성능:
- 매 호출마다 UUID를 생성하는 것은 성능 저하를 초래할 수 있습니다. Kubernetes 등 환경에서도 고유한 로그를 위한 ID 생성 전략을 신중히 고려하세요.
-
타입 안전:
request?.user?.sid ?? ''
와 같은 옵셔널 체이닝은 자바스크립트의 버전을 고려하여 잘 사용하세요. 모든 서버 환경에서 지원되지 않을 수 있습니다.
이 외에도 전반적으로 구조가 잘 잡혀있고, 기본적인 에러 핸들링 로직은 적절합니다. 그러나 위 사항들을 고려하여 다듬으면 더욱 안정성과 가독성을 높일 수 있을 것입니다.
}), | ||
); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드 리뷰 결과는 다음과 같습니다:
버그 리스크
- req.body 처리:
POST
외의 메소드에 대한 처리가 모두req.body
로 설정되어 있습니다. 이 부분은 다른 HTTP 메소드에 대해 적절한 로그를 남기지 못할 수 있습니다. - requestId 처리: 기본값으로 UUID 생성이 되어 있지만, 만약 클라이언트가 유효한
uuid
를 제공하지 않을 경우 여전히 UUID가 생략될 수 있습니다. - res.on('finish') 사용: 모든 파이프라인에서
finish
이벤트를 등록하고 있으며, 이는 여러 번 호출될 위험이 있습니다. 단일 호출로 제한하는 로직이 필요합니다.
개선 제안
- 로깅 포맷 통일화: 요청 및 응답 로깅에서 사용하는 필드들을 일관되게 구성하면 나중에 분석하기 용이합니다.
- 예외 처리 향상: 예외가 발생했을 때, 클라이언트에게 더 구체적인 정보를 보내기 위해
res.status().json()
을 활용할 수 있습니다. - 메타데이터 포함: 요청의 메타데이터를 더 추가하여 나중에 로그를 통해 유용한 정보(예: 사용자 IP)에 접근하기 쉽게 할 수 있습니다.
- 리듀서 추가: 요청 데이터를 간단하게 정리해서 로그에 남길 수 있도록 리듀서를 구현하면 가독성이 개선됩니다.
총평
로깅 인터셉터로써 기능은 잘 유지되고 있으나, 약간의 예외 처리와 코드 정리가 필요합니다. 전반적으로 안정성과 가독성을 높이는 방향으로 개선할 수 있을 것입니다.
|
||
@Controller('api/semesters') | ||
export class SemestersController { | ||
constructor(private readonly semestersService: SemestersService) {} | ||
|
||
@Get() | ||
@Public() | ||
async getSemesters(@Query() query: ISemester.QueryDto) { | ||
const semesters = await this.semestersService.getSemesters(query); | ||
return semesters.map((semester) => toJsonSemester(semester)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 코드 패치는 전반적으로 잘 작성되어 있지만, 몇 가지 점검할 만한 사항과 개선 제안을 드립니다.
버그 리스크
-
Query DTO 검증:
@Query()
데코레이터로 전달된ISemester.QueryDto
에 대한 검증이 필요합니다. 입력 값이 유효하지 않을 경우 오류를 처리하는 로직을 추가해야 합니다. -
비동기 오류 처리: 현재
async/await
블록에서getSemesters
호출 시 오류가 발생하면 이를 처리할 수 있는 로직이 없습니다. 적절한 예외 처리가 필요합니다.
개선 제안
-
TypeScript 타입 명시: 반환 타입을 명시하여 가독성을 높일 수 있습니다. 예를 들면,
async getSemesters(@Query() query: ISemester.QueryDto): Promise<ISemester[]>
와 같이 타입을 지정해주면 좋습니다. -
불필요한 import 줄이기: 만약
Public
데코레이터가 이 컨트롤러에서만 사용하는 것이 아니라면, 사용 여부를 재검토하고 불필요한 import는 제거하는 것이 좋습니다. -
세미스터 데이터 형식 변환:
map
함수 안의toJsonSemester
가 어떤 형태로 데이터를 변환하는지 명확히 하기 위해 주석이나 문서화를 고려하세요.
이러한 사항들을 기억하고 반영하시면 더욱 안정적이고 가독성 좋은 코드를 만들 수 있을 것입니다.
@@ -34,7 +36,7 @@ export class AuthConfig { | |||
return this.authChain | |||
.register(this.isPublicCommand) | |||
.register(this.sidCommand) | |||
.register(this.jwtCommand); | |||
.register(this.sessionCommand); | |||
}; | |||
|
|||
private getProdGuardConfig = () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드 리뷰를 다음과 같이 진행하겠습니다.
-
버그 리스크:
sessionCommand
가 추가됐는데, 다른 부분에서 이 커맨드를 사용하는 로직이 정의되어 있는지 확인해야 합니다. 만약sessionCommand
에 필요한 의존성이 제대로 설정되지 않았다면 런타임 오류가 발생할 수 있습니다..register(this.sessionCommand);
로 인해 기존의 JWT 커맨드 등록이 누락되었습니다. 이 코드가 의도된 것인지 검토가 필요합니다. 모든 인증 관련 로직을 제대로 처리하는지 확인하세요.
-
개선 제안:
- 변수와 메소드에 대해 일관된 네이밍 규칙을 유지하는 것이 좋습니다. 예를 들어
getProdGuardConfig
같은 메소드는 명명 규칙을 통일하면 더 읽기 쉬울 것입니다. - 에러 핸들링 로직이 필요할 수 있습니다. 특히 비동기 작업에서는 실패 상황에 대한 처리가 중요합니다.
SessionCommand
클래스에서 어떤 기능을 제공하는지를 명확히 문서화하거나 코멘트 추가를 추천합니다. 이를 통해 팀원들이 이해하기 쉬워집니다.
- 변수와 메소드에 대해 일관된 네이밍 규칙을 유지하는 것이 좋습니다. 예를 들어
전반적으로 잘 구성된 코드이나, 몇 가지 확인해야 할 사항들이 존재합니다.
@@ -29,6 +30,7 @@ import { AuthConfig } from './auth.config'; | |||
IsPublicCommand, | |||
JwtCommand, | |||
SidCommand, | |||
SessionCommand, | |||
AuthConfig, | |||
], | |||
exports: [AuthService, AuthConfig, AuthChain], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드 패치에 대해 간단히 리뷰하겠습니다.
-
모듈 가져오기:
SessionCommand
를 추가하는 것은 해당 모듈에서 필요한 기능을 확장하기 위한 좋은 접근입니다. 관련성이 높고 의존성이 관리되지 않으면 문제를 일으킬 수 있으니, 이 모듈이 정말로 필요한지 확인해야 합니다.
-
주석 부족:
- 코드 변경의 목적이나 이유가 주석으로 설명되어 있지 않습니다. 다른 개발자가 이해할 수 있도록 주석을 추가하는 것이 좋습니다.
-
내보내기 리스트:
- 새로운
SessionCommand
가exports
배열에 추가되지 않았습니다. 이를 통해 외부에서 사용할 수 있게 해줘야 할 필요가 있을 수 있습니다. 적절한 경우 추가하는 것을 고려하세요.
- 새로운
-
테스트 케이스:
- 새로운 기능이 추가되었으므로, 이와 관련된 테스트 케이스가 있는지 확인하세요. 단위 테스트 및 통합 테스트가 잘 작성되어 있어야 합니다.
-
의존성 관리:
@src/modules/auth/command/session.command
경로가 유효한지 점검하여 올바른 경로에서 모듈이 불러와지는지 확인해야 합니다.
이 외에는 큰 문제점은 없어 보입니다. 하지만 항상 충분한 테스트와 코드 리뷰를 거치는 것이 중요합니다.
RUN apt-get clean | ||
RUN apt-get install -y openssl | ||
|
||
# RUN apt-get update -y && apt-get install -y openssl | ||
|
||
COPY --from=build /var/www/otlplus-server/ /var/www/otlplus-server/ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드 패치에 대한 간단한 리뷰를 제공하겠습니다.
잠재적인 버그 위험
-
패키지 파일 복사:
COPY package*.json ./
에서COPY package.json ./
으로 변경하였는데, 만약package-lock.json
이 없다면 의도된 대로 작동하지 않을 수 있습니다. 둘 다 복사하도록 유지하는 것이 좋습니다. -
apt-get 명령어 순서:
apt-get update
가 누락되어 있습니다. 아마 모든 레포지토리 정보를 최신 상태로 보장해야 하기 때문에 이 명령어가 필요할 것입니다.apt-get install
전에apt-get update
를 해야 합니다. 코드의 커멘트 처리된 부분에서 이미 존재하던 부분을 주석 해제하면 문제가 해결될 수 있습니다.
개선 제안
-
최적화:
- Dockerfile에서 여러 RUN 명령을 하나로 결합하여 이미지의 레이어 수를 줄이는 것이 좋습니다. 예를 들어,
apt-get update
,apt-get install
, 그리고apt-get clean
을 하나의 RUN 명령으로 묶는 것이 권장됩니다.
RUN apt-get update -y && \ apt-get install -y openssl && \ apt-get clean && \ rm -rf /var/lib/apt/lists/*
- Dockerfile에서 여러 RUN 명령을 하나로 결합하여 이미지의 레이어 수를 줄이는 것이 좋습니다. 예를 들어,
-
헬스체크 추가: 서버가 정상적으로 실행되고 있는지를 확인하기 위해 헬스체크를 추가하는 것을 고려해볼 수 있습니다.
-
환경 변수 설정: 필요한 환경 변수를 선언하는 것이 좋습니다. 예를 들어, NODE_ENV 등.
이러한 점들을 고려해서 코드를 수정하면 더 안정적이고 최적화된 컨테이너 이미지를 만들 수 있을 것입니다.
app.enableShutdownHooks(); | ||
return app.listen(8000); | ||
return app.listen(8080); | ||
} | ||
|
||
bootstrap() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드 패치에 대한 간단한 리뷰를 제공하겠습니다.
버그 리스크
-
포트 변경: 서버가 8000에서 8080으로 포트를 변경했습니다. 이로 인해 현재까지 8000번 포트에서 연결된 클라이언트에 문제가 발생할 수 있습니다. 기존 클라이언트와의 연결성을 고려해야 합니다.
-
인터셉터 및 필터 추가:
LoggingInterceptor
와HttpExceptionFilter
가 글로벌로 설정되었습니다. 이들이 올바르게 작동하는지, 성능 문제나 예상치 못한 부작용이 없는지 확인해야 합니다.
개선 제안
-
환경 설정: 포트를 하드코딩하기보다는 환경 변수(예:
process.env.PORT
)를 사용하여 유연성을 높이는 것이 좋습니다. -
주석 추가: 새로운 인터셉터와 필터에 대한 설명 주석을 추가하면 코드 유지보수성이 향상됩니다. 특히, 이들이 어떤 기능을 하는지 명시하는 것이 유익할 것입니다.
-
예외 처리 강화:
HttpExceptionFilter
의 구현이나 설정이 적절한지 검토하여, 모든 예외 상황에서 사용자에게 유용한 피드백을 제공할 수 있도록 해야 합니다. -
모듈 정리: 필요 없는 모듈이 들어오지 않도록 imports를 정리하고, 실제 사용하지 않는 코드를 제거하면 가독성이 향상됩니다.
이와 같은 사항들을 고려하여 코드 품질과 안정성을 높일 수 있을 것입니다.
path: request.url, | ||
}); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 코드 패치는 NestJS의 예외 필터를 사용하여 HTTP 예외를 처리하는 기능을 추가합니다. 전반적으로 잘 구성되어 있지만 몇 가지 주의해야 할 점과 개선 사항이 있습니다.
잠재적인 버그 리스크
-
UUID 처리:
requestId
는request.headers['uuid']
에서 가져오는데, 이 값을 확인하지 않고 바로 사용합니다. 존재하지 않을 경우undefined
가 될 수 있으므로, 로그에 명시적으로"not provided"
와 같은 기본값을 설정하는 것이 좋습니다. -
요청 데이터 처리: 메서드별로 요청 데이터를 처리하긴 하지만, PUT이나 DELETE 등의 메서드를 다루지 않고 있습니다. 이러한 메서드도 로그에 포함할 필요가 있을 수 있습니다.
-
응답 데이터:
response.body
에서 예외 발생 시 응답 데이터를 가져오고 있는데, 이는 일반적으로 존재하지 않습니다. 대신 예외 객체에서 메시지를 추출하는 것이 좋습니다.
개선 제안
-
일관된 데이터 포맷: 모든 요청 메서드(GET, POST 등)에서 요청 본문을 처리하는 부분을 통합할 수 있습니다. 함수로 분리하여 유지보수를 쉽게 할 수 있습니다.
-
로그 레벨 조정: 현재 오류 로그(
logger.error
)만 출력하고 있는데, 다양한 상황에 따라 정보 로그(logger.info
)나 경고 로그(logger.warn
)도 사용할 수 있도록 더 세분화할 수 있습니다. -
타입 정의:
requestData
와 같은 변수에 대한 타입을 명시하면 코드 가독성이 향상될 수 있습니다. TypeScript의 장점을 활용하여 더욱 엄격한 타입 체크를 적용하세요. -
예외 처리 추가: 예외 필터 내에서 예외가 발생했을 때(예: JSON 파싱 오류 등)를 처리할 방법을 추가하십시오. 이를 통해 안정성을 높일 수 있습니다.
-
환경별 로깅 설정: 프로덕션 환경에서는 로그 레벨이나 저장 방식 등을 변경할 수 있도록 환경에 따라 다르게 설정할 수 있는 구조로 만드는 것도 고려해 볼 만합니다.
위 사항들을 반영하면 코드의 안정성과 가독성을 높이는 데 도움이 될 것입니다.
}), | ||
); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 코드 패치에 대한 간단한 코드 리뷰를 제공하겠습니다.
버그 위험 요소:
- UUID가 없는 경우:
requestId
가undefined
일 수 있지만, 로깅 데이터에서는 항상 UUID가 존재해야 한다고 가정하고 사용합니다. 기본값을 제공하는 것이 좋습니다. - POST와 PUT 요청 처리 중복: POST와 기타 모든 메서드에 대해 동일한 조건 처리하는 부분(
else
대신else if (method === 'PUT')
)이 있으며, 코드를 더 명확하게 하기 위해 두 개의 분기를 나누는 것이 좋습니다.
개선 제안:
- 리퀘스트 및 레스폰스 로깅 구조화: 로그 데이터를 더 구조적으로 만들기 위해 인터페이스를 사용하면 명확성 증가.
- 로깅 포맷: 특정 속성을 JSON 형태로 가지도록 포맷을 커스터마이징 하지만, 이 부분을 좀 더 직관적으로 개선할 수 있습니다. 예를 들어, 필요한 정보만 선택적으로 로그에 포함시킬 수 있습니다.
- 수동 에러 핸들링 제거:
req.body
를 읽을 때 발생할 수 있는 오류를 자동으로 처리하도록 NestJS의 유효성 검사 기능을 사용하는 것도 고려해 보세요. - 응답 시간 계산:
finish
이벤트 리스너 내에서 응답 시간이 매우 정확하게 계산될 수 있지만, 비동기 실행 흐름에서 로그가 누락될 가능성은 다소 존재합니다. 즉, 모든 로깅이 완벽히 기록되도록 처리할지 검토하는 것이 좋습니다. - 시간대 관리: 서버 머신의 지역적 시간대를 고려할 경우, 로그 기록 시 고정된 시간대 설정이 필요할 수 있습니다.
이러한 점들을 고려하여 코드의 안정성과 가독성을 높일 수 있습니다.
}), | ||
); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드 리뷰를 진행하겠습니다.
코드 리뷰 요약
-
로깅 구현:
- Winston을 이용하여 요청 및 응답 로깅을 잘 설정했습니다.
DailyRotateFile
사용으로 인해 로그 파일 관리도 용이합니다.
- Winston을 이용하여 요청 및 응답 로깅을 잘 설정했습니다.
-
예외 처리:
- 예외 발생 시 빈 객체를 기본으로 설정하는 부분은 좋지만, 구체적인 로그를 남기는 것이 더 유용할 수 있습니다.
console.error()
같은 추가 로깅을 고려해 보세요.
- 예외 발생 시 빈 객체를 기본으로 설정하는 부분은 좋지만, 구체적인 로그를 남기는 것이 더 유용할 수 있습니다.
-
요청 데이터 처리:
- POST 방식의 요청에서 조건을 잘 나누고 처리는 적절하지만, PUT, PATCH와 같은 다른 HTTP 메서드를 통일감 있게 처리해야 할 수도 있습니다.
-
타임존 처리:
- 타임존을 서울로 하여 좋은 선택입니다. 하지만 UTC로 스탬프를 찍고 클라이언트 쪽에서 변환하는 방법도 고려해보세요.
-
requestId
처리:- UUID를 헤더에서 가져오도록 했는데, 값이 없을 경우의 비즈니스 요구사항에 맞춰 기본값을 설정하는 것도 좋습니다.
-
로그 포맷 일관성:
- 요청 및 응답의 로그 포맷이 통일성을 가지지 않으면 나중에 로그 분석이 어려울 수 있습니다. 필요하다면 포맷팅을 개선해보세요.
-
내부 서버 오류 핸들링:
HttpException
외에도 다른 종류의 오류 처리를 추가해서 더 많은 세부정보를 로그에 남길 수 있습니다.
-
주석 처리된 코드 제거:
- 주석 처리된 부분은 불필요한 코드를 깔끔하게 정리하여 가독성을 높이는 것이 좋습니다. 필요하다면 명확한 의도를 가진 주석만 유지하세요.
버그 위험
req.user.sid
가 존재하지 않을 경우undefined
를 빈 문자열로 대체하는 방식은 생각보다 안전하지만 경우에 따라서 값이 더 복잡할 수 있으므로 확인이 필요합니다.- 로거의 동시성 문제도 고려하세요. 멀티스레드 컨텍스트에서 여러 요청이 동시에 로깅될 수 있기 때문에 경합 상태(race condition) 발생 가능성이 있습니다.
개선 사항
- API 요청/응답 구조에 대해서 스키마 검증을 추가하여 로그하기 전에 데이터를 검증하는 방법을 생각해볼 수 있습니다.
- 테스트 가능한 단위로 로깅 기능을 나누어 각 컴포넌트가 독립적으로 테스트 가능한 구조로 개발하는 것도 도움이 됩니다.
요약하자면, 매우 잘 작성된 코드이며 몇 가지 세부적인 개선을 통해 더욱 견고한 시스템으로 발전할 수 있을 거라 생각됩니다.
...auth_user, | ||
}; | ||
} | ||
|
||
async createUser( | ||
user: Prisma.session_userprofileCreateInput, | ||
): Promise<session_userprofile> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 코드 패치는 전반적으로 잘 작성되었으나, 몇 가지 버그 위험 요소와 개선 사항이 있습니다.
버그 위험 요소
-
데이터 형식 검증 부족:
decodedSessionData
에서 추출한 JSON 문자열이 반드시 올바른 구조를 가진다는 보장이 없습니다. 추가적인 데이터 유효성 검사가 필요합니다.userId
가 NaN일 경우 이후 DB 쿼리가 실패할 수 있습니다. 이를 방지하기 위해Number()
변환 후 체크가 필요합니다.
-
예외 처리 미비:
JSON.parse(jsonString);
는 잘못된 JSON 문자열이 들어올 경우 예외를 던질 수 있습니다. 이 부분에 대한 오류 처리를 추가해야 합니다.
-
퍼포먼스 문제:
- 두 개의 DB 쿼리 (
session_userprofile
와auth_user
)를 사용하고 있는데, 이는 성능 저하를 초래할 수 있습니다. 가능하다면 조인 쿼리로 수정하거나 배치 요청을 고려하십시오.
- 두 개의 DB 쿼리 (
개선 사항
-
타입 정의 추가:
- 반환 객체의 타입을 명시하여 유지보수성을 높일 수 있습니다.
-
상수 정의:
- 에러 메세지 등을 상수로 정의하면 추후 유지보수가 용이해집니다.
-
정규 표현식 개선:
- 정규 표현식을 사용하는 부분에서 좀 더 명확한 조건을 부여하여 신뢰성을 높일 수 있습니다.
-
Async/Await 처리 최적화:
await
키워드를 사용하는 경우, 병렬 실행이 가능한 쿼리는Promise.all([...])
을 사용하여 효율성을 개선할 수 있습니다.
결론
버그를 예방하고 코드의 견고함을 높이기 위해 위의 제안 사항들을 고려하는 것이 좋습니다. 추가적인 검증과 예외 처리를 통해 안정성을 더욱 강화할 수 있습니다.
// { | ||
// emit: 'stdout', | ||
// level: 'warn', | ||
// }, | ||
], | ||
}; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드 패치에 대한 간단한 리뷰입니다.
문제점 및 개선 사항
-
주석 처리:
log
배열에서 모든 로깅 레벨을 주석 처리했습니다. 이는 로깅이 필요한 경우 정보를 잃게 되며, 디버깅이나 오류 추적이 어려워질 수 있습니다. 주석을 제거하고 필요에 따라 로깅 설정을 조정하는 것이 좋습니다.
-
코드 가독성:
- 불필요한 주석으로 인해 코드가 번잡해진 느낌이 있습니다. 만약 특정 로깅 레벨이 필요 없다면 완전히 제거하거나, 주석을 남기더라도 필수적인 내용만 포함하는 것을 추천합니다.
-
유지 관리:
- 향후 로깅 설정을 수정해야 할 경우 주석 처리된 코드로 인해 혼란이 생길 수 있습니다. 설정 값을 환경 변수나 설정 파일로 분리하여 관리하면 더 유연하게 대처할 수 있습니다.
-
기능 검증:
- 로그 레벨 변경이 시스템의 기능에 어떤 영향을 미칠지 충분히 검토해야 합니다. 예를 들어,
info
와warn
로그가 없어지는 것으로 인해 중요한 경고 메시지를 놓치게 되는 경우가 발생할 수 있습니다.
- 로그 레벨 변경이 시스템의 기능에 어떤 영향을 미칠지 충분히 검토해야 합니다. 예를 들어,
위의 피드백을 참고하여 코드를 개선하면 더 안정적이고 유지 보수가 쉬운 소스 코드를 만들 수 있을 것입니다.
No description provided.