feat: error code interface 정의 및 exception, exception handler 추가 #32

soeunnPark · 2024-09-19T08:02:59Z

PR에 대한 설명 🔍

custom error code, exception, exception handler 추가

변경된 사항 📝

custom error code를 유연하게 추가하기 위해 인터페이스를 정의했습니다.
이 인터페이스를 구현하여 자유롭게 도메인별로 에러 코드를 만들어 사용하시면 될 것 같습니다.

혹시 예외 처리나 에러코드 관련하여 다른 방법이 있거나 더 좋은 코드가 있다면 꼭 말씀해주세요 😄

궁금한 점 ❓

지난번 e2e 프로젝트에서 custom exception을 여러 개 정의하여 사용했습니다. 다만 각각의 exception 클래스들이 별다른 기능을 하지 않고 아래와 같이 에러 메시지만 다르게 정의하고 있었습니다.

public class DuplicateEmailException extends CatchLineException {

    public DuplicateEmailException(String email) {
        super("이미 존재하는 이메일입니다. : " + email);
    }
}

커스텀 exception 클래스의 수는 아주 많은데 다른 점이 오직 에러 메시지밖에 없다면, 굳이 상황마다 새로 exception 클래스를 생성할 필요가 없지 않나? 라는 의문이 들었습니다.

일단 커스텀 error code를 정의하고 description 을 포함하였습니다. Badminton Exception은 error code와 description 을 포함하고 있습니다.

지금은 Runtime Exception을 상속받은 최상위 badminton exception 만 존재합니다.
이 예외 클래스를 상속받아 더 구체적인 예외를 정의한다고 하면, 어떤 내용이 추가되어야 하는지 궁금합니다.

PR에서 중점적으로 확인되어야 하는 사항

커스텀 예외를 처리하고 싶을 때 아래와 같이 던지면 됩니다 !

Custom Error Code를 추가한다.

이름 예시: ClubErrorCode, LeagueErrorCode, MemberErrorCode

ErrorCodeIfs (에러코드 인터페이스) 구현하여 만들어주세요.

코드 예시:

@Getter
@AllArgsConstructor
public enum ClubErrorCode implements ErrorCodeIfs {

	CLUB_NOT_FOUND(400, 1404, "존재하지 않는 동호회입니다."),
	CLUB_ALREADY_EXISTS(400, 1402, "이미 존재하는 동호회입니다."),
	;

	private final Integer httpStatusCode;
	private final Integer errorCode;
	private final String description;

}

첫 번째 파라미터 : http 상태 코드
두 번째 파라미터 : 에러 코드
세 번째 파라미터 : 에러에 대한 상세 설명

Validator에서 에러를 던질 때

public void checkIfClubPresent(String clubName) {
		clubRepository.findByClubName(clubName).ifPresent(club -> {
			throw new BadmintonException(CLUB_ALREADY_EXISTS);
		});
	}

BadmintonException을 상속받아 새로운 예외 클래스를 만들어 사용해도 됩니다.
새로 생성한 ClubErrorCode를 넣어서 던지면 됩니다.

km2535

Exception 관련 Swagger 문서 방법을 찾아 보면 좋을 것 같습니다.

LichKing-lee · 2024-09-19T12:11:57Z

badminton-api/src/main/java/org/badminton/api/common/error/ClubErrorCode.java

+	CLUB_ALREADY_EXISTS(400, 1402, "이미 존재하는 동호회입니다."),
+	;
+
+	private final Integer httpStatusCode;


요거 굳이 래퍼타입으로 갈 필요없지않나요?
int 로 가면됩니다~

@LichKing-lee 앗 맞습니다.. ! 수정하겠습니다 😄

LichKing-lee · 2024-09-19T12:13:15Z

badminton-api/src/main/java/org/badminton/api/common/error/ClubErrorCode.java

+	;
+
+	private final Integer httpStatusCode;
+	private final Integer errorCode;


이부분 굳이 숫자로 할필요가 없어요
String 으로 관리하는게 코드만 보고 사람이 확인하기에 훨씬 편할거에요. String 타입의 에러코드로 변경하시거나 아니면 enum 의 name 을 에러코드로 그냥 쓰셔도 좋을 것 같네요
혹시 숫자로 하신 이유가 있으면 말씀해주세요

@LichKing-lee 에러 코드 자체에 어떤 에러인지가 드러나면 좋을 것 같습니다 !
지금은 4401, 4402 등으로 errorCode를 정의했는데 C_4401, C_4402이나 CLUB_ALREADY_EXISTS 등로 바꾸면 더 좋다고 이해했는데 맞을까요?
숫자로 한 이유는 Http 상태 코드가 200, 400, 500 인 것에 익숙해져 있어서 그랬던 것 같습니다. 😅

네 맞아요~ 그렇게 문자로 가는게 개발자들이 로그를 보거나 문제를 파악하기에 훨씬 쉬울거에요

네! 감사합니다 ㅎㅎ 좋은 이름을 고민해서 수정하도록 하겠습니다 !!

LichKing-lee · 2024-09-19T12:14:04Z

badminton-api/src/main/java/org/badminton/api/common/error/ErrorCodeIfs.java

@@ -0,0 +1,10 @@
+package org.badminton.api.common.error;
+
+public interface ErrorCodeIfs {


Ifs 는 어떤 의미예요?

@LichKing-lee 에러 코드 인터페이스입니다!

아 인터페이스만드신건 이해했는데 네이밍에 Ifs 가 무슨의미인가해서요 interfaces 인가..??

앗 .. 강의를 참고하면서 작성한 코드라서 사실 잘 모르겠습니다.. ㅠㅠ 그렇지 않아도 이름이 와닿지 않아서 수정 요청이 있었는데 제가 아직 생각해내지를 못했습니다.. ㅎㅎ 😢

헉 어떤 강의길래..

@LichKing-lee 지금 에러 코드 인터페이스, exception 인터페이스가 모두 getter 메서드를 가져서 이를 구현한 클래스들이 멤버 변수를 가지도록 강제하고 있습니다.

에러 코드 인터페이스의 get 메서드 -> 에러 코드 구현 클래스들의 멤버 변수
getHttpStatusCode(); -> httpStatusCode
getErrorCode(); -> errorCode
getDescription(); -> description

이렇게 인터페이스에 getter 메서드를 두고 구현 클래스들이 멤버 변수를 가지도록 강제하는 것이 혹시 적절하지 않은 방법일까요 ?!

@soeunnPark 객체지향언어에서 인터페이스를 어떻게 생각하는지에 대해 생각해보면 되는데요.
인터페이스는 행위에 대한 스펙을 미리 정의해놓는거라 단순히 getter 들을 나열하는 인터페이스는 좋은 인터페이스는 아닙니다.
좋지 않다고해서 절대 쓰면 안돼 까지는 아니지만 좋은 인터페이스가 아니라는건 알고 계시면 좋을 것 같아서 말씀드려요.

LichKing-lee · 2024-09-19T12:20:01Z

badminton-api/src/main/java/org/badminton/api/exceptionhandler/BadmintonExceptionHandler.java

+
+@Slf4j
+@RestControllerAdvice
+@Order(value = Integer.MIN_VALUE)


Order 애노테이션 안달고, 그냥 이 핸들러 안에 Exception.class 에 대한 코드도 같이 넣어도 돼요.
그러면 예외가 발생했을때 BadmintonException 하위 타입은 BadmintonException 쪽으로 가고 아닌애들은 Exception 으로 가요.
(쓰고보니까 약간 자신없는데 혹시 안되면 말씀주세요.....)

@LichKing-lee 넵 이렇게 해보겠습니다 !!

@LichKing-lee 이렇게 했을 때 잘 동작합니다 !! 감사합니다 😊

@soeunnPark 오 부끄러울뻔했는데 다행 ㅎㅎㅎ

LichKing-lee · 2024-09-19T12:20:38Z

badminton-api/src/main/java/org/badminton/api/common/exception/BadmintonExceptionIfs.java

+
+import org.badminton.api.common.error.ErrorCodeIfs;
+
+public interface BadmintonExceptionIfs {


이 인터페이스는 왜 만든거에요?

@LichKing-lee Exception 클래스들이 늘어날 것에 대비해 만들었습니다. Exception 클래스는 ErrorCode 인터페이스 구현체와 에러에 대한 설명(description)을 가지도록 했습니다. . !

LichKing-lee · 2024-09-19T12:21:32Z

badminton-api/src/main/java/org/badminton/api/common/error/ErrorCode.java

+
+@AllArgsConstructor
+@Getter
+public enum ErrorCode implements ErrorCodeIfs {


ClubErrorCode 이랑 이 ErrorCode 의 차이는 뭐에요? 각 도메인별로 따로 ErrorCode 를 관리하기 위한건가?

@LichKing-lee 네 맞습니다. 에러 코드 인터페이스를 정의하고 도메인별로 만들려고 했습니다. 이를 하나로 합칠지 고민을 했는데 나눌지 말지 고민이 될 때 일단 나눠보자는 생각에 일단 유지했습니다. pr 본문에 궁금한 점을 남겨뒀는데 저 고민에서 시작된 분리인 것 같습니다 .. ! custom exception 을 추가하지 않고 도메인별로 발생할 수 있는 에러들을 에러 코드로 나눠 표현하려고 했습니다. 에러 코드는 한 곳에 모아놓는게 더 적절할까요?

아.. 본문에 있는거부터 답변을 좀 드려야겠네요 😂

팀원분들이랑 어느정도 컨센서스가 맞춰진건진 모르겠지만 지금 열려있는 강민님 PR 보면 ErrorCode 하나에 다 몰빵해서 모아놓고, 그걸 다시 각 도메인별 ErrorCode 로 분리되고 있잖아요
이런식으로 갈거면 나눌 필요가 없어요 도메인별로 ErrorCode 를 나누려는 소은님 의도를 지키려면 몰빵해서 모아놓는 ErrorCode 가 있으면 안되고, 모아놓는 ErrorCode 가 있을거면 도메인별로 나누는 의미가 사라집니다 ErrorCode 를 쓰실거면 둘중 하나를 결정하셔야돼요

@LichKing-lee 네!
처음에 나누려던 이유는 error code가 많아지면 나눠서 관리하는게 더 편할 것 같아서였습니다. 또 custom exception을 나누지 않고 에러 코드를 세분화하려 했습니다. 그런데 custom exception을 만들 예정이라 후자의 의미는 없어졌습니다.

에러코드를 다시 한곳에 모아놓게 된 이유는 Integer 타입의 에러 코드가 겹칠까봐였습니다. 또 에러 코드가 그렇게 많을까? 굳이 나눌 필요가 있나? 라는 생각도 있었던 것 같습니다. 그런데 에러 코드를 String 타입으로 변경하면 에러 코드가 겹치는 실수는 줄어들 것 같습니다.

혹시 멘토님이라면 어떤 선택을 하실지, 현업에서는 어떻게 하시는지 궁금합니다 .. !💭

@LichKing-lee 팀원들이랑 같이 코드 수정해서 pr 다시 올렸습니다 ㅎㅎ

@soeunnPark 에러코드를 만들어서 처리하는 방식의 경우 하나의 ErrorCode enum 에 모든 에러코드를 정의하는 방식을 많이 써요
그런데 그걸 분리하려고 하신건 보기 좋았어요 😄 저라면 에러코드를 각각 Exception 내에서 정의했을 것 같아요 이건 멘토링시간에 더 얘기나눠봐요

LichKing-lee · 2024-09-19T14:08:53Z

음.. 텍스트로 적으려다보니 어디서부터 얘기를 하면 좋을지 고민이 좀 되는데요.
일단 본문에 적어주신 것처럼 이전에는 각 상황별 예외클래스를 만들었는데 달라지는 부분이 메시지밖에 없어서 이걸 계속 만드는게 의문이 든건 충분히 공감을 할 수 있는 부분입니다.

본문에 예시로 적어주신 DuplicateEmailException 같은 경우 생성자 파라미터로 String email 을 받고 있는데요. 제가 알기로 이거 시간이 지나서 email 이 Email 이라는 타입으로 표현되고, 이에 따라 예외 클래스도 String 을 받던걸 Email 로 받도록 변경되었을거에요. 그쵸?

BadmintonException 하나만 만들어서 각 상황별로 ErrorCode 를 파라미터로 던지는것과 상황별 Exception 클래스를 만드는게 어떤 차이가 있냐면 크게 두가지 차이가 있는데요.

타입안정성
Email, PhoneNumber 와 같은 정보는 String 하나로 표현할 수 있습니다. 하지만 굳이 왜 별도의 타입을 선언하셨었나요? Email 이 사용되는 곳들마다 공통적으로 처리돼야하는 로직을 한곳으로 모아 응집도를 높이고, 타입을 이용해 표현력을 살리기 위해서 였습니다. 예외도 마찬가지입니다. 다만 지금 소은님은 예외 클래스의 타입으로 표현하기보다 내부 상태인 ErrorCode 를 이용해 예외를 분리했는데요. 이것도 한가지 방법이고, 실제로 이 방식도 많이 씁니다. 다만 이 방식은 아래 설명할 문제를 해결하기가 어렵습니다.
표현력

public class DuplicateEmailException extends CatchLineException {

    public DuplicateEmailException(String email) {
        super("이미 존재하는 이메일입니다. : " + email);
    }
}

소은님이 작성하신 이 클래스는 이미 존재하는 이메일입니다. 라는 메시지만 노출하는게 아니라 이미 존재하는 이메일이 어떤 이메일인지까지 포함하고 있습니다. 그리고 위 코드에선 String 으로 받지만 나중엔 Email 타입을 받게되죠? 이걸 ErrorCode 를 이용해서 어떻게 표현할 수 있을까요? 현재 PR 과 같이 ErrorCode 를 이용하면 동적인 파라미터를 넣기가 힘들어집니다. ErrorCode 를 정의하는 enum 자체가 정적 필드이기 때문이죠. 예외 클래스의 타입으로 예외를 나타내면 해당 타입은 각 상황에 맞는 맥락을 필드와 생성자로 나타낼 수 있습니다. email 과 연관된 예외 상황에선 Email 을 파라미터로 받고, 핸드폰 번호와 연관된 상황에선 PhoneNumber 를 받을 수 있죠. 그런데 ErrorCode 를 이용할땐 어떤 상황에나 똑같은 BadmintonException 을 사용하게되기 때문에 타입이 맥락을 갖기 어렵습니다. 그래서 전 예외 클래스의 타입을 이용해 맥락을 표현하는걸 권장하는 편입니다.

feat: error code interface 정의 및 exception, exception handler 추가

ebd3065

soeunnPark added docs📉 문서 수정 5PM🌝 오후 5시 전까지 리뷰가 필요합니다. labels Sep 19, 2024

soeunnPark requested review from km2535 and I-migi September 19, 2024 08:02

soeunnPark self-assigned this Sep 19, 2024

soeunnPark linked an issue Sep 19, 2024 that may be closed by this pull request

[docs] custom error code, exception configuration #31

Closed

4 tasks

km2535 approved these changes Sep 19, 2024

View reviewed changes

km2535 merged commit 164ec2f into develop Sep 19, 2024
1 check passed

soeunnPark deleted the 31-docs-custom-error-code-exception-configuration branch September 19, 2024 08:27

LichKing-lee reviewed Sep 19, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: error code interface 정의 및 exception, exception handler 추가 #32

feat: error code interface 정의 및 exception, exception handler 추가 #32

soeunnPark commented Sep 19, 2024 •

edited

Loading

km2535 left a comment

LichKing-lee Sep 19, 2024

soeunnPark Sep 19, 2024

LichKing-lee Sep 19, 2024

soeunnPark Sep 19, 2024 •

edited

Loading

LichKing-lee Sep 19, 2024

soeunnPark Sep 19, 2024

LichKing-lee Sep 19, 2024

soeunnPark Sep 19, 2024

LichKing-lee Sep 19, 2024

soeunnPark Sep 19, 2024 •

edited

Loading

LichKing-lee Sep 20, 2024

soeunnPark Sep 20, 2024

LichKing-lee Sep 20, 2024

LichKing-lee Sep 19, 2024

soeunnPark Sep 19, 2024

soeunnPark Sep 20, 2024

LichKing-lee Sep 20, 2024

LichKing-lee Sep 19, 2024

soeunnPark Sep 19, 2024

LichKing-lee Sep 19, 2024

soeunnPark Sep 19, 2024

LichKing-lee Sep 19, 2024

LichKing-lee Sep 20, 2024

soeunnPark Sep 20, 2024

soeunnPark Sep 20, 2024

LichKing-lee Sep 20, 2024

LichKing-lee commented Sep 19, 2024

		@@ -0,0 +1,10 @@
		package org.badminton.api.common.error;

		public interface ErrorCodeIfs {


		import org.badminton.api.common.error.ErrorCodeIfs;

		public interface BadmintonExceptionIfs {

feat: error code interface 정의 및 exception, exception handler 추가 #32

feat: error code interface 정의 및 exception, exception handler 추가 #32

Conversation

soeunnPark commented Sep 19, 2024 • edited Loading

PR에 대한 설명 🔍

변경된 사항 📝

궁금한 점 ❓

PR에서 중점적으로 확인되어야 하는 사항

km2535 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

soeunnPark Sep 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

soeunnPark Sep 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LichKing-lee commented Sep 19, 2024

soeunnPark commented Sep 19, 2024 •

edited

Loading

soeunnPark Sep 19, 2024 •

edited

Loading

soeunnPark Sep 19, 2024 •

edited

Loading