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

[#34] ExceptionHandler 추가하고 Custom Exception으로 변환 #35

Merged
merged 2 commits into from
Sep 4, 2024

Conversation

youngreal
Copy link
Member

What is this PR? 🔍

기존에 RuntimeException으로 던져줬던 모든 예외를 Custom한 예외로 변환해 예외별로 커스텀 핸들링이 가능하게끔 변환

Changes 📝

  • 예외핸들러가 점점 커질것은데 개선여지가 있어보입니다
  • 클라이언트에게 구체적인 예외 메시지를 바디에 전달하지않고 문서로 구체적인 예외를 전달해주고있습니다. 장점도 있겠지만 이게 더 편리한지는 의문이라 조금더 고민해봐야합니다

@youngreal youngreal requested a review from min-0 September 4, 2024 07:16
@youngreal youngreal self-assigned this Sep 4, 2024
@youngreal youngreal linked an issue Sep 4, 2024 that may be closed by this pull request
Comment on lines +55 to +74
@ExceptionHandler(AppleTokenDecodingException.class)
public ResponseEntity<String> runtimeException(AppleTokenDecodingException e) {
return ResponseEntity
.status(HttpStatus.UNAUTHORIZED)
.body("토큰 인증에 실패했습니다");
}

@ExceptionHandler(JwtTokenExpiredException.class)
public ResponseEntity<String> runtimeException(JwtTokenExpiredException e) {
return ResponseEntity
.status(HttpStatus.UNAUTHORIZED)
.body("토큰 인증에 실패했습니다");
}

@ExceptionHandler(JwtTokenDecodingException.class)
public ResponseEntity<String> runtimeException(JwtTokenDecodingException e) {
return ResponseEntity
.status(HttpStatus.UNAUTHORIZED)
.body("토큰 인증에 실패했습니다");
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

음 .. 이 부분은 중복이 크게 띄니까 handleTokenExceptions 같은 이름으로 합치는 것이 어떨까요 ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드로 어떻게 합칠수있나요?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ExceptionHandler(handleTokenExceptions.class)
	public ResponseEntity<String> runtimeException(handleTokenExceptions e) {
		return ResponseEntity
			.status(HttpStatus.UNAUTHORIZED)
			.body("토큰 인증에 실패했습니다");
	}

이런 느낌으로용 !

Copy link
Member Author

@youngreal youngreal Sep 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

handleTokenExceptions 추상클래스 를 만들어서 위 3개의 예외를 상속하는 방식 말씀하시는건가요?? 위 3개의 예외의 응답이 완전 똑같다는 확신이 생기면 어떻게든 줄일수 있을것같은데 클라랑 조금더 얘기해보고 변경해봐도 괜찮을것 같아요

import com.dnd.dndtravel.map.exception.PhotoInvalidException;
import com.dnd.dndtravel.map.exception.PhotoUploadFailException;
import com.dnd.dndtravel.map.exception.RegionNotFoundException;
//todo 예외클래스가 많아지면 해당클래스가 길어질것으로 예상, 개선필요해보이고 보안 때문에 상태코드별로 애매하게 동일한 메시지를 전달해주고, 스웨거 문서로 상세 오류를 전달해주는데 이 구조가 적절한건지 고민해봐야한다.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

특정 예외 상황에 대한 상세한 오류 메시지는 로그로 남기고, 사용자나 스웨거에는 일반적인 메시지를 전달하는 건 어떨까용 ?
개발자는 상세하게 로그를 봐야 문제 발생 시 빠른 대처가 가능할 것 같다는 생각이 듭니다!!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

음 .. 클라이언트가 구체적인 예외를 알아야 하는 경우가 예를 들면 어떤 것들이 있을 수 있을까여 ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

사용자의 방문기록을 저장하는 API에서는 클라이언트 입장에서 상태코드 400을 받는 경우의수가 엄청 다양해요.
(방문기록 자체의 검증실패, 유저나 지역이 존재하지 않는경우, 방문기록자체가 없는경우, 요청 헤더에 토큰 형식이 잘못된 경우 등..)

HTTP 바디에는 일반적인 메시지가 전달되고 이는 스웨거에서도 그렇게 표기됩니다. 주석으로 적어놨던 스웨거 문서로 상세오류를 전달한다는건 description 같은 부가설명을 적는부분이에요 스웨거 한번보시면 이해하실듯 해요

@youngreal youngreal merged commit 192b22b into main Sep 4, 2024
1 check passed
@youngreal youngreal deleted the feat/#34 branch September 10, 2024 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

예외처리 고도화
2 participants