Default + Step 9: 에러코드 정의, 글로벌 에러 핸들러, 토큰 검증 인터셉터 등록 #24

kilhyeonjun · 2024-10-23T07:13:15Z

요구사항

비즈니스 별 발생할 수 있는 에러 코드 정의 및 관리 체계 구축
프레임워크별 글로벌 에러 핸들러를 통해 예외 로깅 및 응답 처리 핸들러 구현
시스템 성격에 적합하게 Filter, Interceptor 를 활용해 기능의 관점을 분리하여 개선
모든 API 가 정상적으로 기능을 제공하도록 완성

작업 내용

BaseEntity의 패키지 위치를 변경했습니다. (domain.support.model 로 이동)
- chore: BaseEntity 패키지 위치 이동
CoreException 생성 (비지니스 예외 처리용 Custom Exception), ErrorCode 생성, ErrorType 생성
- feat: CoreException 생성
- 이전의 BusinessException을 대체할 용도입니다. 도메인 로직이 HttpStatus를 가지고 있는 문제가 있었습니다. 도메인은 client와 상관없이 동작해야합니다.
공통 에러코드 마이그레이션 [domian]ErrorCode -> ErrorType
- refactor: WaitingQueueErrorCode 제거 및 ErrorType.WaitingQueue 로 전환
- refactor: UserErrorCode 제거 및 ErrorType.User 로 전환
- refactor: PaymentErrorCode 제거 및 ErrorType.Payment 로 전환
- refactor: ConcertErrorCode 제거 및 ErrorType.Concert 로 전환
- CoreException에 맞는 type으로 변경했습니다.
API와 관련된 파일을 interfaces.api.support 패키지로 이동했습니다.
- chore: api support 패키지 생성 및 이동
CoreException의 @ExceptionHandler를 등록하였습니다.
- feat: 글로벌 에러핸들러에 CoreException.class 추가
- 로그 레벨 별로 로깅처리를 했습니다.
- ErrorCode 별로 reponse할 HttpStatus를 관리했습니다.
이전 메소드명 규칙에 안맞는 부분을 수정했습니다. (lock 획득할 경우 xxxWithLock)
- chore: uuid로 토큰 조회시 lock 제거 (메소드 네이밍 규칙)
토큰 활성화. 토큰 만료. 임시예약 해제를 스케줄러에 등록했습니다.
- feat: 스케줄러 등록
대기열 검증을 interceptor로 등록했습니다.
- feat: 대기열 검증 interceptor 등록
- controller에서 별도의 대기열 검증 처리를 제거했습니다.

리뷰포인트

궁금한 사항은 코멘트 남겨놨습니다. 확인 부탁드립니다!
리뷰어님들이 생각했을 때 더 좋은 패턴이 보일 경우 공유해주시면 좋을 것 같습니다.

kilhyeonjun · 2024-10-23T07:22:08Z

src/main/java/com/example/hhplus/concert/domain/support/error/ErrorType.java

+
+  private final ErrorCode code;
+  private final String message;
+  private final LogLevel logLevel;


ErrorType에서 logLevel을 관리할경우
동일한 에러지만 비지니스에 따라 log level을 유연하게 관리하지 못할수도 있다는 의견이 있었습니다.
이런 부분에 대한 의견을 듣고 싶습니다.

저는 지금 현상에서 유연하게 관리하는 방법으론 낮은 로그레벨을 공통으로 관리하고 더 높은 레벨의 로깅이 필요할 경우 해당 비지니스 코드에 직접 로깅하는 방법이 있을 것 같다는 생각이 드는 것 같아요

로그레벨을 Exception을 선언하는 쪽에서 주입하는 방법도 있을 것 같습니다!

kilhyeonjun · 2024-10-23T07:26:06Z

...com/example/hhplus/concert/interfaces/api/interceptor/WaitingQueueValidationInterceptor.java

+    Map<String, String> pathVariables = (Map<String, String>) request.getAttribute(
+        HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE);


콘서트별로 관리하기 위해 대기열에 concertId를 담고 있습니다.
그래서 검증 usecase를 위해선 콘서트 정보가 필요한 상황입니다.

콘서트를 알 수 있는 정보는 현재 concertId, concertScheduleId, concertSeatId 3개가 있고 이는 pathVariable로 넘오는 형태이기에 HandlerMapping에 담겨있는 메타정보를 이용해서 콘서트 정보를 가져올 있는 key값을 가져왔습니다.

이러한 방법이 많이 쓰이는지 아니면 정규식 또는 /로 split해서 index로 가져오는 방식을 사용하는 지 궁금합니다.

kilhyeonjun · 2024-10-23T07:27:34Z

...com/example/hhplus/concert/interfaces/api/interceptor/WaitingQueueValidationInterceptor.java

+    if (pathVariables.containsKey("concertId")) {
+      Long concertId = Long.parseLong(pathVariables.get("concertId"));
+      waitingQueueFacade.validateWaitingQueueProcessingAndConcertId(waitingQueueTokenUuid,
+          concertId);
+    } else if (pathVariables.containsKey("concertScheduleId")) {
+      Long concertScheduleId = Long.parseLong(pathVariables.get("concertScheduleId"));
+      waitingQueueFacade.validateWaitingQueueProcessingAndScheduleId(waitingQueueTokenUuid,
+          concertScheduleId);
+    } else if (pathVariables.containsKey("concertSeatId")) {
+      Long concertSeatId = Long.parseLong(pathVariables.get("concertSeatId"));
+      waitingQueueFacade.validateWaitingQueueProcessingAndSeatId(waitingQueueTokenUuid,
+          concertSeatId);
+    } else {
+      throw new CoreException(ErrorType.INVALID_REQUEST);
+    }


concertId, concertScheduleId, concertSeatId 별 별도의 usecase를 두었는데
해당 방법이 아닌 인터셉터에서 직접적으로 repository에 접근해서 가져오는 방식도 있을 것 같습니다.
이 방식은 하나의 usecase (e.g. validateWaitingQueueProcessingAndConcertId) 만 사용될 것 같은데 어느 방식을 더 선호하는지 그리고 이유도 듣고 싶습니다.

if ~ else 문을 쓰는 것보다 우선은 PathVariables에 서 containsKey를 판단하는 것을 따로 빼고,
그 다음에 로직을 타는 건 어떠실까요

kilhyeonjun · 2024-10-23T07:30:08Z

src/test/java/com/example/hhplus/concert/application/UserFacadeTest.java

+      final Exception result = assertThrows(Exception.class,
          () -> userFacade.chargeUserWalletAmount(userId, walletId, amount));

      // then
-      assertThat(result.getMessage()).isEqualTo(UserErrorCode.USER_NOT_FOUND.getMessage());
+      assertThat(result.getMessage()).isEqualTo(ErrorType.User.USER_NOT_FOUND.getMessage());


then 부분에서 메시지 내용만 비교했을 때
메시지내용은 동일하지만 log level이나 code가 다른 값에 대한 고려는 없어지는 것 같습니다.
메시지말고 error type 으로 검증하는 게 더 좋았을까요?

assertThrows에서 Exception 타입만 구체화하면 되지 않을까 생각했는데 어떠실까요?

Lexyyaa · 2024-10-26T06:35:18Z

src/main/java/com/example/hhplus/concert/domain/support/error/ErrorType.java

ErrorType 안에 클래스를 생성해서 도메인별로 타입 분리한게 인상적입니다! 저도 이렇게 변경해야겠어요!!

maiorem · 2024-10-26T06:56:39Z

작업 내용과 리뷰 포인트를 잘 정리해주셔서 보기가 너무 편합니다.
반성이 많이 되네요....ㅎㅎㅎ ㅜㅜ

Lexyyaa · 2024-10-26T07:01:47Z

src/main/java/com/example/hhplus/concert/interfaces/api/support/advice/ApiControllerAdvice.java

@@ -12,6 +13,27 @@
 @RestControllerAdvice
 class ApiControllerAdvice extends ResponseEntityExceptionHandler {

+  @ExceptionHandler(CoreException.class)


개인적으로 DomainException이나 BizException이라는 네이밍이 더 적합해보입니다! 왜냐하면 일단 첫번째로 해당 Exception이 domain 하위에 위치해 있고, 해당 Exception으로 throw된 상황에 뱉어내는 에러메시지들이 비즈니스 로직과 더 유관해보이고 또한 log level 이 모두 warn이기 때문입니다!

maiorem · 2024-10-26T07:06:22Z

src/main/java/com/example/hhplus/concert/domain/support/error/ErrorType.java

+    WALLET_ID_MUST_NOT_BE_NULL(ErrorCode.BAD_REQUEST, "지갑 ID는 null일 수 없습니다.", LogLevel.WARN),
+    AMOUNT_MUST_NOT_BE_NULL(ErrorCode.BAD_REQUEST, "금액은 null일 수 없습니다.", LogLevel.WARN),
+    AMOUNT_MUST_BE_POSITIVE(ErrorCode.BAD_REQUEST, "금액은 0보다 커야 합니다.", LogLevel.WARN),
+    ;


ErrorType 내부로 도메인별 에러코드를 따로 관리하는 점이 인상적입니다.
굉장히 내용이 많은데 관리에 대한 이슈는 없을지 궁금해집니다. 혹시 생각해두신 방법이 있을까요?

우선 여러 비지니스에 따라 exception을 throw 해주는 상황에서 비지니스에서 별도로 처리하는 방향보단 에러 코드 모아서 관리할려고 했습니다.
에러 코드 관리하는 방법 중엔 도메인 별로 파일을 나누거나 현 PR 처럼 한 파일에 하위 관계로 하는 방법이 있을 것 같아요.
후자로 선택한 이유는 개인적인 생각이지만 도메인 별로 파일로 나누게 되면 중복된 에러 코드를 작성할 수 도 있고 그걸 찾는 번거로움이 있을 것 같았습니다.

maiorem · 2024-10-26T07:08:14Z

src/main/java/com/example/hhplus/concert/domain/user/dto/UserCommand.java

-import com.example.hhplus.concert.domain.common.exception.BusinessException;
-import com.example.hhplus.concert.domain.user.UserErrorCode;
+import com.example.hhplus.concert.domain.support.error.CoreException;
+import com.example.hhplus.concert.domain.support.error.ErrorType;



Command와 Query 패턴을 실제로 수정/조회 적절한 시점에 적극적으로 활용하시는군요!
위 패턴을 사용한다면 어떻게 사용하는게 효과적일지 고민이 있었는데 참고가 되었습니다.

kilhyeonjun added 12 commits October 22, 2024 17:33

chore: BaseEntity 패키지 위치 이동

20c4aba

feat: CoreException 생성

5c6c10d

refactor: WaitingQueueErrorCode 제거 및 ErrorType.WaitingQueue 로 전환

63b8839

refactor: UserErrorCode 제거 및 ErrorType.User 로 전환

cf948b4

refactor: PaymentErrorCode 제거 및 ErrorType.Payment 로 전환

47b1213

refactor: ConcertErrorCode 제거 및 ErrorType.Concert 로 전환

b9026f8

chore: 이전 Exception 패키지 제거 (BusinessException)

2cc2dc4

chore: api support 패키지 생성 및 이동

beceab1

feat: 글로벌 에러핸들러에 CoreException.class 추가

3e62d17

chore: uuid로 토큰 조회시 lock 제거 (메소드 네이밍 규칙)

c8ea489

feat: 스케줄러 등록

1b01779

feat: 대기열 검증 interceptor 등록

16d3936

kilhyeonjun self-assigned this Oct 23, 2024

kilhyeonjun linked an issue Oct 23, 2024 that may be closed by this pull request

대기열 검증 interceptor 구현 #23

Closed

pull-request-size bot added the size/XXL label Oct 23, 2024

kilhyeonjun linked an issue Oct 23, 2024 that may be closed by this pull request

글로벌 에러 핸들러를 통해 예외 로깅 및 응답 처리 핸들러 구현 #22

Closed

kilhyeonjun added this to the 프로젝트 고도화 milestone Oct 23, 2024

kilhyeonjun linked an issue Oct 23, 2024 that may be closed by this pull request

비즈니스 별 발생할 수 있는 에러 코드 정의 및 관리 체계 구축 #20

Closed

kilhyeonjun added default step9 코드리뷰 labels Oct 23, 2024

kilhyeonjun commented Oct 23, 2024

View reviewed changes

kilhyeonjun marked this pull request as ready for review October 23, 2024 07:30

kilhyeonjun merged commit 1232796 into master Oct 23, 2024

kilhyeonjun deleted the feature/step-9 branch October 23, 2024 08:09

Lexyyaa reviewed Oct 26, 2024

View reviewed changes

maiorem reviewed Oct 26, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Default + Step 9: 에러코드 정의, 글로벌 에러 핸들러, 토큰 검증 인터셉터 등록 #24

Default + Step 9: 에러코드 정의, 글로벌 에러 핸들러, 토큰 검증 인터셉터 등록 #24

Uh oh!

kilhyeonjun commented Oct 23, 2024 •

edited

Loading

Uh oh!

kilhyeonjun Oct 23, 2024

Uh oh!

LeeJaeYun7 Oct 26, 2024

Uh oh!

kilhyeonjun Oct 23, 2024

Uh oh!

kilhyeonjun Oct 23, 2024

Uh oh!

LeeJaeYun7 Oct 26, 2024

Uh oh!

kilhyeonjun Oct 23, 2024

Uh oh!

LeeJaeYun7 Oct 26, 2024

Uh oh!

Lexyyaa Oct 26, 2024

Uh oh!

maiorem commented Oct 26, 2024

Uh oh!

Lexyyaa Oct 26, 2024

Uh oh!

maiorem Oct 26, 2024

Uh oh!

kilhyeonjun Oct 27, 2024

Uh oh!

maiorem Oct 26, 2024

Uh oh!

Uh oh!

		Map<String, String> pathVariables = (Map<String, String>) request.getAttribute(
		HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE);

Default + Step 9: 에러코드 정의, 글로벌 에러 핸들러, 토큰 검증 인터셉터 등록 #24

Default + Step 9: 에러코드 정의, 글로벌 에러 핸들러, 토큰 검증 인터셉터 등록 #24

Uh oh!

Conversation

kilhyeonjun commented Oct 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

요구사항

작업 내용

리뷰포인트

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maiorem commented Oct 26, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kilhyeonjun commented Oct 23, 2024 •

edited

Loading