Skip to content
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

커스텀 예외처리 리팩터링 #1365

Open
wonyongChoi05 opened this issue Jul 5, 2023 · 1 comment
Open

커스텀 예외처리 리팩터링 #1365

wonyongChoi05 opened this issue Jul 5, 2023 · 1 comment
Assignees
Labels
Milestone

Comments

@wonyongChoi05
Copy link
Member

wonyongChoi05 commented Jul 5, 2023

커스텀 예외 처리가 알 수 없는 흐름으로 동작하는 현상이 꽤나 자주 일어납니다. (특히 테스트 코드 작성할 때 등록 안된 커스텀 예외들이 자주 보임) 다들 아시다시피, 이는 BadRequestCode(Enum)에 해당 예외를 커스텀 예외로 등록을 하지 않는다면 NotFoundErrorCodeException 발생하기 때문입니다.

이를 해결하기 위해서 생각해본 여러가지 방법이 있는데, 다음과 같습니다.

  • BadRequestCode에 등록되지 않은 예외들을 등록한다.

    • 장점
      • 기존 코드 구조를 바꾸지 않고도 적용할 수 있다.
    • 단점
      • 모든 커스텀 예외들이 하나의 Enum에 모여있어, 유지보수에 좋지 않을 수 있다.(공통)
      • 무슨 예외가 등록되지 않았는지 일일이 찾아봐야 한다.
      • 추후에 똑같은 문제가 발생할 여지가 있다.
  • BadRequestCode를 Optional로 조회하지 않고 등록되어 있는 예외를 발생시킨다.

    • 장점
      • 등록되어 있지 않은 커스텀 예외를 사용할 수 없다.
    • 단점
      • 모든 커스텀 예외들이 하나의 Enum에 모여있어, 유지보수에 좋지 않을 수 있다.(공통)
      • 앞으로 NotFoundErrorCodeException가 발생하지 않을테지만, 커스텀 예외를 사용하기 위해선 역시나 무슨 예외가 등록되지 않았는지 일일이 찾아봐야 한다.
  • 커스텀 예외를 사용할 때 status, code, type을 입력하도록 강제한다.

    • 장점
      • 각 커스텀 예외 객체에서 status, code, type을 관리할 수 있다.
        • 별도의 커스텀 예외 집합이 필요하지 않다.
      • 등록되어 있지 않은 커스텀 예외를 사용할 수 없다.
      • BadRequest(400)가 아닌, 다른 번대의 커스텀 예외가 필요할 때, 기존 코드에 비해서 적용하기 쉬울 것이다.
    • 단점
      • (거의 없겠지만)같은 status와 code를 가졌음에도 다른 메시지가 필요한 경우에 활용성이 낮을 수 있다.
      • 현재 등록되어 있는 모든 커스텀 예외 클래스에 적용해야 하기 때문에 꽤 많은 시간이 필요하다.

호이와 조금 더 상의해보고 결정하는 방향으로 하겠습니다.
++ 위 의견들은 어디까지나 제 의견과 해결방안이기 때문에 더 좋은 방안이 있다면, 많은 의견 코멘트 해주시면 감사하겠습니다.

@wonyongChoi05 wonyongChoi05 added this to the 2023 레벨3 milestone Jul 5, 2023
@wonyongChoi05 wonyongChoi05 self-assigned this Jul 5, 2023
@This2sho
Copy link
Contributor

현재 진행 방향

현재의 문제: 커스텀 예외 처리가 알 수 없는 흐름으로 동작함(커스텀 예외로 등록을 안한다면 NotFoundErrorCodeException 발생

BadRequest에서 findByClass 메소드로 같은 타입의 에러코드를 조회하는데 없으면 NotFoundErrorCodeException 을 던짐..
즉, 예외등록을 안해놓음..

최대한, 현재 방식과 유사하면서 깔끔하게 수정하기로함..

BadRequestException

변경 전

@Getter
public class BadRequestException extends RuntimeException {

    private BadRequestCode codeAndMessage = BadRequestCode.findByClass(this.getClass());

    private String message;
    private int code;

    public BadRequestException() {
        this.message = codeAndMessage.getMessage();
        this.code = codeAndMessage.getCode();
    }
}

변경 후

@Getter
public class BadRequestException extends RuntimeException {

    private String message;
    private int code;

    public BadRequestException(BadRequestCode badRequestCode) {
        this.message = badRequestCode.getMessage();
        this.code = badRequestCode.getCode();
    }
}
  • 생성자에서 BadRequestCode를 받게해서 무조건 예외 등록을 하도록 수정

BadRequestCode

변경 전

@AllArgsConstructor
@Getter
public enum BadRequestCode {
    NOT_FOUND_ERROR_CODE(0000, "해당 에러의 에러코드를 찾을 수 없습니다.", NotFoundErrorCodeException.class),

    ...

    CURRICULUM_NOT_FOUND_EXCEPTION(8007, "해당하는 커리큘럼을 찾을 수 없습니다", CurriculumNotFoundException.class);

    private int code;
    private String message;
    private Class<? extends BadRequestException> type;

    public static BadRequestCode findByClass(Class<?> type) {
        return Arrays.stream(BadRequestCode.values())
            .filter(code -> code.type.equals(type))
            .findAny()
            .orElseThrow(NotFoundErrorCodeException::new);
    }

}
public class NotFoundErrorCodeException extends BadRequestException {

}
public class CurriculumNotFoundException extends BadRequestException {

}

변경 후

@AllArgsConstructor
@Getter
public enum BadRequestCode {
    NOT_FOUND_ERROR_CODE(0000, "해당 에러의 에러코드를 찾을 수 없습니다."),
		
		... 

    ROADMAP_KEYWORD_SEQUENCE_EXCEPTION(8007, "키워드 시퀀스가 유효하지 않습니다.");

    private int code;
    private String message;
}
  • 복잡한 Custom 예외 다 삭제 ⇒ BadRequestCode만 관리하도록 변경 (CustomException 만들 필요 없음)

실제 코드 작성에서 바뀐 부분

변경 전

public MemberTags(List<MemberTag> values) {
        final int originalSize = values.size();
        final long count = values.stream().distinct().count();
        if(originalSize != count) throw new DuplicateMemberTagException();
        this.values = values;
    }

변경 후

public MemberTags(List<MemberTag> values) {
        final int originalSize = values.size();
        final long count = values.stream().distinct().count();
        if (originalSize != count) {
            throw new BadRequestException(DUPLICATE_MEMBER_TAG);
        }

        this.values = values;
    }
  • 기존에는 CustomException을 알고 있었다면, 바뀐 후에는 BadRequestCode를 알고 있도록 변경

하면서 느낀점

  • 이렇게 해도 될까? (예외처리 관련된 부분은 프롤로그 사용하는 사람들 다 쓰는거라서 한번 같이 얘기할 필요가 있지 않을까라는 생각)

  • 바꿀게 너무 많음 (에러 던지는 부분, 예외 코드 정의 안되어있는 부분 등)

  • 모든 커스텀 예외의 HTTP 상태 코드가 모두 BadRequest 400을 던짐..

    • 이 부분은 좀 더 고려해봐야할 듯..
  • BadRequestException의 code(특정 예외 코드)의 에러목록 문서화가 3xxx번대에서 끊김..
    문서화 최신화도 필요..

  • Custom Exception이 너무 많다보니까 하나로 퉁 칠려고 했는데, Exception catch하기가 빡셈..

    private String getUserId(String token) {
          try {
              return jwtTokenProvider.extractSubject(token);
          } catch (TokenNotValidException e) {
              return "Guest";
          }
      }

    현재는 일단, 다음과 같이 바꿔놓음..

    private String getUserId(String token) {
          try {
              return jwtTokenProvider.extractSubject(token);
          } catch (BadRequestException e) {
              return "Guest";
          }
      }
  • Test에서 예외 체크하는 경우

    assertThrows(NotFoundErrorCodeException.class, () -> sessionService.findById(1L));

    assertThrows 대신 assertThatThrownBy 사용하기 바람.

    assertThatThrownBy(() -> sessionService.findById(1L))
                .isInstanceOf(BadRequest.class)
                .hasMessage(SESSION_NOT_FOUND_EXCEPTION.getMessage());

구구의 피드백

  • 한번에 변경하지 말고 기존 코드를 사용하면서 같이 변경할 수 있는 방법을 생각해보자.
  • 1xxx 번 부터 프로젝트만의 코드를 사용하면 관리(문서화)하기가 쉽지않다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants