[BE] fix: 학생 인증 코드가 유효하지 않을 시 500이 아닌 400 에러를 반환 (#447) #622
Conversation
- 사용자 입력에 영향을 받는 생성자 이므로 400 예외를 던지기 위함
| throw new UnexpectedException("VerificationCode의 길이는 %d 이어야 합니다.".formatted(LENGTH)); | ||
| throw new ValidException("VerificationCode의 길이는 %d 이어야 합니다.".formatted(LENGTH)); | ||
| } | ||
| } | ||
|
|
||
| private void validatePositive(String value) { | ||
| if (!POSITIVE_REGEX.matcher(value).matches()) { | ||
| throw new UnexpectedException("VerificationCode는 0~9의 양수 형식이어야 합니다."); | ||
| throw new ValidException("VerificationCode는 0~9의 양수 형식이어야 합니다."); |
There was a problem hiding this comment.
해당 예외는 ValidException으로 할 지, BadRequest로 할 지 헷갈리네요.
아마 값에 대한 기본적인 예외가 아닌, 도메인 요구 사항에 대한 예외니까 BadRequest가 더 맞아 보이긴 하는데..
의견이 필요합니다.
There was a problem hiding this comment.
저는 BadRequest가 맞다고 느낍니다.
이용자의 입력값에 의한 예외니까 닉네임 4자리 미만인데 3자리 입력 이런 느낌?
There was a problem hiding this comment.
이 부분에서 고민인게 저희가 가진 VerificationCode가 우리가 생성하는데 검증(잘못되면 5xx), 이용자가 입력한 값일때도 검증(현재 5xx이지만 4xx를 던져야겠죠?)을 같은 방식으로 하다보니 생기는 문제 같아요.
처음에 500으로 한 것도 우리가 code 잘못 만들어서 생기는 문제 때문에 500으로 했던 걸로 기억하고요.
그래서 이를 이분화해서 validate 메소드를 boolean을 반환해주는 public method로 활용하고 (혹은 validator class에서) 결과에 따라 적절한 예외를 service layer에서 던져주는 것도 고민해봤는데, 에러 메시지 처리도 복잡해지고, 코드가 과하게 복잡하게 된다고 여겨져요.
그렇다보니 전 issue 및 PR 코드처럼 그냥 우리가 만든 code가 만들어지는 방식은 테스트코드를 믿고, 4xx를 던져주는게 좋다고 여겨지네요.
| throw new UnexpectedException("VerificationCode의 길이는 %d 이어야 합니다.".formatted(LENGTH)); | ||
| throw new ValidException("VerificationCode의 길이는 %d 이어야 합니다.".formatted(LENGTH)); | ||
| } | ||
| } | ||
|
|
||
| private void validatePositive(String value) { | ||
| if (!POSITIVE_REGEX.matcher(value).matches()) { | ||
| throw new UnexpectedException("VerificationCode는 0~9의 양수 형식이어야 합니다."); | ||
| throw new ValidException("VerificationCode는 0~9의 양수 형식이어야 합니다."); |
There was a problem hiding this comment.
저는 BadRequest가 맞다고 느낍니다.
이용자의 입력값에 의한 예외니까 닉네임 4자리 미만인데 3자리 입력 이런 느낌?
BadRequestException으로 처리하려니, 인자를 ErrorCode Enum으로 관리하기 때문에, 동적인 메시지를 적는 것이 불가능하네요. 😂
따라서 BadRequestException의 생성자를 추가하는게 아니라면 처리가 힘들어 보입니다. |
|
추가로 #625 이슈 처리하면서 생각해보니, |
|
제가 확인 제대로 못한게 있었네요 !! 글렌 의견 듣고보니 ValidException이 적절하다 느껴집니다. |
|
불필요 브랜치를 삭제하다가 삭제해버렸네요. 죄송합니다! |
📌 관련 이슈
✨ PR 세부 내용
#482 에서 처리해야 했을 기능입니다.