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

feat: exception handling #34

Merged
merged 16 commits into from
Jan 31, 2024
Merged

feat: exception handling #34

merged 16 commits into from
Jan 31, 2024

Conversation

Sangwook02
Copy link
Member

🌱 관련 이슈

📌 작업 내용 및 특이사항

  • GlobalExceptionHandler로 CustomException, RuntimeException 처리
  • ErrorResponse, ErrorCode 추가
  • JwtFilter에서 발생하는 Exception을 처리할 JwtExceptionFilter 추가

@Sangwook02 Sangwook02 added the ✨ feature 새로운 기능 추가 및 수정 label Jan 30, 2024
@Sangwook02 Sangwook02 self-assigned this Jan 30, 2024
@Sangwook02 Sangwook02 requested a review from a team as a code owner January 30, 2024 07:43
@@ -83,7 +85,7 @@ public AccessTokenDto parseAccessToken(String accessTokenValue) throws ExpiredJw
} catch (ExpiredJwtException e) {
throw e;
} catch (Exception e) {
return null;
Copy link
Member

Choose a reason for hiding this comment

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

null로 리턴하는거 의도한 부분인데 수정한 이유가 있나요?

Copy link
Member Author

Choose a reason for hiding this comment

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

만료 이외의 예외는 null을 반환해서 재발급 과정을 거치기보다 예외를 던지는 것이 적절한 것 같아 수정했습니다!

Copy link
Member

Choose a reason for hiding this comment

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

만료 이외의 예외가 발생해서 JwtFilter 의 line 52 로직을 타더라도, reissueAccessTokenIfExpired 에서 ExpiredJwtException 에 잡히지 않아 null로 반환되기 때문에 isPresent 로직을 타지 않아서 재발급이 이루어지지 않습니다.

로직 다시 한번 체크해보실래요?

Copy link
Member Author

Choose a reason for hiding this comment

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

잡히지 않는 이유를 확인해본 결과,
reissueAccessTokenIfExpired에 AT가 아닌 RT를 넘겨서 ExpiredJwtException이 아닌 SignatureException이 발생하는 것이 원인이었습니다.

이외에도 JwtService.getRefreshTokenFromRedis의 내부를 조금 수정했으니 같이 확인 부탁드립니다!

@@ -42,11 +42,7 @@ private void saveRefreshTokenToRedis(RefreshTokenDto refreshTokenDto) {
}

public AccessTokenDto retrieveAccessToken(String accessTokenValue) {
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

@Sangwook02 Sangwook02 Jan 30, 2024

Choose a reason for hiding this comment

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

이 부분은 제가 착각했습니다.
아래와 같이 수정하는 게 좋겠네요

try {
      return jwtUtil.parseAccessToken(accessTokenValue);
} catch (ExpiredJwtException e) {
      return null;
}

기존 메서드는 데이터의 유무와 무관하게 항상 비어있는 Optional을 반환함.

사용하지 않는 Repository의 메서드는 제거함.
accessToken을 받아야 하는 메서드인데 refreshToken을 받고 있었음.
Copy link
Member

@uwoobeat uwoobeat left a comment

Choose a reason for hiding this comment

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

어푸루부를드릴게요

@Sangwook02 Sangwook02 merged commit 8dcf2e1 into develop Jan 31, 2024
1 check passed
@Sangwook02 Sangwook02 deleted the feature/16-exception-handling branch January 31, 2024 05:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ feature 새로운 기능 추가 및 수정
Projects
None yet
Development

Successfully merging this pull request may close these issues.

✨ 예외 클래스 및 에러코드 세팅
2 participants