Skip to content

[Feat] 어플리케이션 단위 500 에러 디스코드로 알림 전송기능 개발#320

Merged
hd0rable merged 23 commits into
developfrom
feat/#319-discord-error
Oct 5, 2025
Merged

[Feat] 어플리케이션 단위 500 에러 디스코드로 알림 전송기능 개발#320
hd0rable merged 23 commits into
developfrom
feat/#319-discord-error

Conversation

@hd0rable
Copy link
Copy Markdown
Member

@hd0rable hd0rable commented Oct 2, 2025

#️⃣ 연관된 이슈

closes #319

📝 작업 내용

  • 서버에서 500에러 발생시 디스코드 웹훅을 이용해 알림을 전송하도록했습니다.
  • 기존에 500에러가 핸들러에서 분산되어있어서 500에러 핸들러를 하나로 합쳤습니다.
  • 500 에러를 위한 InternalServerException를 추가했습니다.
  • 네이버,알라딘 api 이용중에 외부 api에대한 예외처리를 ExternalApiException로 통합했습니다.

📸 스크린샷

💬 리뷰 요구사항

리뷰어가 특별히 봐주었으면 하는 부분이 있다면 작성해주세요

📌 PR 진행 시 이러한 점들을 참고해 주세요

* P1 : 꼭 반영해 주세요 (Request Changes) - 이슈가 발생하거나 취약점이 발견되는 케이스 등
* P2 : 반영을 적극적으로 고려해 주시면 좋을 것 같아요 (Comment)
* P3 : 이런 방법도 있을 것 같아요~ 등의 사소한 의견입니다 (Chore)

Summary by CodeRabbit

  • New Features

    • 모든 성공/에러 응답에 requestId 포함 및 X-Request-ID 수집(미제공 시 생성)
    • 요청 단위 MDC 기록(요청ID·사용자ID) 및 인증 성공 시 사용자ID 기록
    • 서버 오류 발생 시 디스코드 알림 전송 기능 추가(동기 전송)
    • 콘솔/레벨별 롤링 파일 로그 구성 추가
  • Bug Fixes

    • 서버 예외 처리 경로 통합 및 예외 타입 정비로 오류 분류·보고 일관성 개선
    • Firebase 전송/배치 실패 처리 및 관련 에러 코드 개선
  • Chores

    • Spring WebFlux 및 Logstash Logback Encoder 의존성 추가

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Oct 2, 2025

Walkthrough

서버 전역 예외 유형 재분류와 MDC 기반 요청 추적을 도입하고, Discord 웹훅 전송을 위한 DiscordClient와 전역 예외 핸들러 통합을 추가했다. 로깅 설정(logback)과 Spring WebFlux 의존성도 추가되었다.

Changes

Cohort / File(s) Summary
Build & Logging config
build.gradle, src/main/resources/logback-spring.xml
spring-boot-starter-webflux, net.logstash.logback:logstash-logback-encoder:7.4 의존성 추가 및 콘솔/롤링 파일 기반 logback 설정 추가.
Discord 알림 & 전역 예외 핸들러
src/main/java/.../common/discord/DiscordClient.java, src/main/java/.../common/exception/handler/GlobalExceptionHandler.java
DiscordClient 컴포넌트 추가. GlobalExceptionHandler를 통합해 Runtime/IllegalState/Firebase/InternalServer/ExternalApi 예외를 단일 핸들러로 처리하고 MDC(requestId/userId)를 포함해 Discord로 오류 전송 시도 로직 추가.
MDC 요청 추적 도입
src/main/java/.../common/logging/LoggingConstant.java, src/main/java/.../common/logging/MdcLoggingFilter.java, src/main/java/.../common/dto/BaseResponse.java, src/main/java/.../common/dto/ErrorResponse.java, src/main/java/.../common/security/filter/JwtAuthenticationFilter.java, src/main/java/.../config/SecurityConfig.java
REQUEST_ID/USER_ID 상수 추가, MdcLoggingFilter로 X-Request-ID 주입 및 MDC 관리, BaseResponse/ErrorResponse에 requestId 포함, JwtAuthenticationFilter가 userId를 MDC에 기록하고 SecurityConfig에 필터 등록.
예외 체계 정비 — 공통/신규 예외
src/main/java/.../common/exception/InternalServerException.java, src/main/java/.../common/exception/FirebaseException.java, src/main/java/.../common/exception/code/ErrorCode.java
InternalServerException 클래스 추가. FirebaseException이 BusinessException 계층에서 RuntimeException 기반으로 변경되어 errorCode 필드/생성자 재구성. ErrorCode enum에 상수 추가/수정/삭제.
예외 치환 — 외부 API 어댑터
src/main/java/.../book/adapter/out/api/aladin/AladinApiUtil.java, src/main/java/.../book/adapter/out/api/naver/NaverApiUtil.java
Aladin/Naver API 유틸에서 기존 BusinessException/InvalidStateException 등을 ExternalApiException 또는 InternalServerException으로 대체(로직은 동일, 예외 타입 변경).
예외 치환 — Redis/퍼시스턴스/도메인
src/main/java/.../book/adapter/out/persistence/BookRedisAdapter.java, src/main/java/.../user/adapter/out/persistence/UserTokenBlacklistRedisAdapter.java, src/main/java/.../common/aop/StatusFilterAspect.java, src/main/java/.../notification/domain/value/NotificationRedirectSpecConverter.java, src/main/java/.../user/domain/User.java
JSON 처리 실패·상태 검증 실패 등에서 던지는 예외를 InternalServerException으로 통일(일부 경로에서 ExternalApiException→InternalServerException 전환).
보안/OAuth 경로의 예외 치환
src/main/java/.../common/security/oauth2/CustomSuccessHandler.java, src/main/java/.../common/security/oauth2/tokenstorage/RedisLoginTokenStorage.java
설정 누락·JSON 처리 오류 시 AuthException 대신 InternalServerException을 던지도록 변경.
파이어베이스 전송 경로 정비
src/main/java/.../message/adapter/out/firebase/FirebaseAdapter.java
FCM 전송 실패/리스트 크기 불일치 처리에서 FirebaseException을 명시적 errorCode 기반으로 던지도록 정리(원인 래핑 제거 및 고정된 ErrorCode 사용).

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Client as 클라이언트
  participant MdcF as MdcLoggingFilter
  participant Security as SecurityChain
  participant JwtF as JwtAuthenticationFilter
  participant Controller as Controller
  participant GErr as GlobalExceptionHandler
  participant DClient as DiscordClient
  participant Discord as Discord Webhook

  Client->>MdcF: 요청 (X-Request-ID 가능)
  MdcF->>MdcF: requestId 결정 및 MDC.put(REQUEST_ID)
  MdcF->>Security: 전달
  Security->>JwtF: Jwt 필터 실행
  JwtF->>JwtF: validateToken() → 성공 시 MDC.put(USER_ID)
  JwtF->>Controller: 요청 전달
  alt 정상 흐름
    Controller-->>Client: BaseResponse/ErrorResponse (requestId 포함)
  else 5xx 예외 발생
    Controller->>GErr: 예외 전파
    GErr->>GErr: 클래스/스택 요약, MDC에서 requestId/userId 추출
    GErr->>DClient: sendErrorMessage(msg, stack, requestId, userId)
    DClient->>Discord: POST webhook (embed)
    Discord-->>DClient: 응답
    GErr-->>Client: 5xx ErrorResponse(requestId 포함)
  end
  MdcF->>MdcF: MDC.clear()
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • seongjunnoh

Poem

토끼가 깡충, 로그를 물고 와 🐇
request_id 달고 길을 지나
에러가 울면 웹훅에 속삭여
디스코드에 별빛 한번 번쩍 ✨
서버는 정리, 토끼는 당근 한 입 🥕

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Out of Scope Changes Check ⚠️ Warning 이 PR에는 Discord 알림 기능과 직접 관련이 낮은 BaseResponse 및 ErrorResponse의 요청 ID 추가, logback 설정 파일 도입, 다양한 외부 API 예외 처리 통합 및 공통 로깅 필터·SecurityConfig 변경 등 광범위한 리팩터링이 포함되어 있어 핵심 기능 구현과 분리된 과도한 변경이 확인됩니다. Discord 알림 기능 구현과 직접 무관한 변경사항은 별도의 PR로 분리하여 리뷰 범위를 좁히고 기능별로 검토할 수 있도록 권장합니다.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed 제목이 500 에러 발생 시 디스코드 알림 전송 기능 개발이라는 핵심 변경사항을 명확하고 간결하게 요약하고 있어 PR의 주요 내용을 바로 파악할 수 있습니다.
Linked Issues Check ✅ Passed PR은 DiscordClient 생성과 GlobalExceptionHandler 통합을 통해 애플리케이션 단위 5xx 에러 발생 시 디스코드 웹훅으로 알림을 전송하도록 구현하고 InternalServerException 클래스 추가 및 분산된 500 에러 핸들러 통합을 수행하여 #319 이슈의 주요 요구사항을 충족합니다.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/#319-discord-error

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Oct 2, 2025

Test Results

481 tests   481 ✅  43s ⏱️
143 suites    0 💤
143 files      0 ❌

Results for commit 9cab189.

♻️ This comment has been updated with latest results.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 8

🧹 Nitpick comments (2)
src/main/java/konkuk/thip/common/Discord/DiscordClient.java (2)

62-62: 요청 스레드 블로킹 우려

block() 호출은 요청 처리 스레드를 차단하여 처리량을 저하시킬 수 있습니다. 에러 알림 전송은 비동기로 처리하는 것이 바람직합니다.

다음 diff를 적용하여 비동기 처리로 변경하세요:

         webClient.post()
                 .uri(webhookUrl)
                 .contentType(MediaType.APPLICATION_JSON)
                 .bodyValue(payload)
                 .retrieve()
                 .bodyToMono(Void.class)
-                .block();
+                .subscribe(
+                        result -> {},
+                        error -> log.error("[Discord] 알림 전송 실패", error)
+                );

추가로, 클래스 상단에 @Slf4j 어노테이션을 추가하세요:

+import lombok.extern.slf4j.Slf4j;
+
+@Slf4j
 @Component
 public class DiscordClient {

26-26: WebClient 재사용 권장

매 호출마다 WebClient 인스턴스를 생성하는 것은 비효율적입니다. 필드로 선언하여 재사용하는 것이 좋습니다.

다음 diff를 적용하세요:

 @Component
 public class DiscordClient {
 
     @Value("${discord.env}")
     private String env;
 
     @Value("${discord.webhook-url}")
     private String webhookUrl;
+
+    private final WebClient webClient = WebClient.create();
 
     public void sendErrorMessage(String message, String stackTrace, String requestId, String userId) {
         if("test".equals(env)) return;
 
-        WebClient webClient = WebClient.create();
-
         Map<String, Object> embedData = new HashMap<>();
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between af7709e and 47f2874.

📒 Files selected for processing (17)
  • build.gradle (1 hunks)
  • src/main/java/konkuk/thip/book/adapter/out/api/aladin/AladinApiUtil.java (2 hunks)
  • src/main/java/konkuk/thip/book/adapter/out/api/naver/NaverApiUtil.java (5 hunks)
  • src/main/java/konkuk/thip/book/adapter/out/persistence/BookRedisAdapter.java (3 hunks)
  • src/main/java/konkuk/thip/common/Discord/DiscordClient.java (1 hunks)
  • src/main/java/konkuk/thip/common/aop/StatusFilterAspect.java (2 hunks)
  • src/main/java/konkuk/thip/common/exception/FirebaseException.java (1 hunks)
  • src/main/java/konkuk/thip/common/exception/InternalServerException.java (1 hunks)
  • src/main/java/konkuk/thip/common/exception/code/ErrorCode.java (2 hunks)
  • src/main/java/konkuk/thip/common/exception/handler/GlobalExceptionHandler.java (3 hunks)
  • src/main/java/konkuk/thip/common/security/oauth2/CustomSuccessHandler.java (2 hunks)
  • src/main/java/konkuk/thip/common/security/oauth2/tokenstorage/RedisLoginTokenStorage.java (2 hunks)
  • src/main/java/konkuk/thip/config/AwsS3ImageUrlInitializer.java (2 hunks)
  • src/main/java/konkuk/thip/message/adapter/out/firebase/FirebaseAdapter.java (3 hunks)
  • src/main/java/konkuk/thip/notification/domain/value/NotificationRedirectSpecConverter.java (3 hunks)
  • src/main/java/konkuk/thip/user/adapter/out/persistence/UserTokenBlacklistRedisAdapter.java (2 hunks)
  • src/main/java/konkuk/thip/user/domain/User.java (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (9)
src/main/java/konkuk/thip/book/adapter/out/api/naver/NaverApiUtil.java (1)
src/main/java/konkuk/thip/common/exception/InternalServerException.java (1)
  • InternalServerException (5-17)
src/main/java/konkuk/thip/book/adapter/out/persistence/BookRedisAdapter.java (1)
src/main/java/konkuk/thip/common/exception/InternalServerException.java (1)
  • InternalServerException (5-17)
src/main/java/konkuk/thip/common/security/oauth2/CustomSuccessHandler.java (1)
src/main/java/konkuk/thip/common/exception/InternalServerException.java (1)
  • InternalServerException (5-17)
src/main/java/konkuk/thip/common/security/oauth2/tokenstorage/RedisLoginTokenStorage.java (1)
src/main/java/konkuk/thip/common/exception/InternalServerException.java (1)
  • InternalServerException (5-17)
src/main/java/konkuk/thip/config/AwsS3ImageUrlInitializer.java (1)
src/main/java/konkuk/thip/common/exception/InternalServerException.java (1)
  • InternalServerException (5-17)
src/main/java/konkuk/thip/common/aop/StatusFilterAspect.java (1)
src/main/java/konkuk/thip/common/exception/InternalServerException.java (1)
  • InternalServerException (5-17)
src/main/java/konkuk/thip/user/adapter/out/persistence/UserTokenBlacklistRedisAdapter.java (1)
src/main/java/konkuk/thip/common/exception/InternalServerException.java (1)
  • InternalServerException (5-17)
src/main/java/konkuk/thip/user/domain/User.java (1)
src/main/java/konkuk/thip/common/exception/InternalServerException.java (1)
  • InternalServerException (5-17)
src/main/java/konkuk/thip/notification/domain/value/NotificationRedirectSpecConverter.java (1)
src/main/java/konkuk/thip/common/exception/InternalServerException.java (1)
  • InternalServerException (5-17)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build
🔇 Additional comments (14)
src/main/java/konkuk/thip/user/domain/User.java (1)

84-84: 변경 승인: 예외 타입이 서버 에러 의미에 더 적합합니다.

oauth2Id가 null인 상태는 비즈니스 로직 위반이 아닌 서버 내부 상태 오류이므로, InternalServerException으로 변경한 것이 적절합니다. 이는 PR의 500 에러 통합 방침과도 일치합니다.

build.gradle (1)

96-97: 변경 승인: Discord 웹훅 호출을 위한 필수 의존성입니다.

spring-boot-starter-webfluxDiscordClient에서 사용하는 WebClient를 제공합니다. 비동기 HTTP 호출에 적합하며, Spring Boot BOM에서 버전을 관리하므로 별도 버전 명시 없이 추가하는 것이 올바릅니다.

src/main/java/konkuk/thip/common/security/oauth2/tokenstorage/RedisLoginTokenStorage.java (1)

42-42: 변경 승인: JSON 역직렬화 실패는 서버 내부 오류입니다.

Redis에서 로그인 토큰을 읽을 때 JSON 역직렬화가 실패하는 것은 인증 실패(AuthException)가 아닌 서버 내부 데이터 손상 또는 설정 오류를 의미하므로, InternalServerException으로 변경한 것이 적절합니다.

src/main/java/konkuk/thip/common/aop/StatusFilterAspect.java (1)

33-33: 변경 승인: 트랜잭션 부재는 서버 설정 오류입니다.

@Transactional 컨텍스트가 없는 상태에서 Hibernate 세션을 사용하려는 것은 개발자 실수 또는 프레임워크 설정 오류이므로, InternalServerException으로 변경한 것이 적절합니다. 이는 비즈니스 예외가 아닌 기술 스택 수준의 문제입니다.

src/main/java/konkuk/thip/book/adapter/out/persistence/BookRedisAdapter.java (1)

83-83: 변경 승인: JSON 직렬화/역직렬화 오류는 서버 내부 문제입니다.

Redis에서 도서 순위 정보를 읽고 쓸 때 발생하는 JSON 처리 오류는 서버 내부 데이터 손상 또는 설정 문제를 의미하므로, InternalServerException으로 처리하는 것이 적절합니다. 두 경로 모두 일관되게 변경되었습니다.

Also applies to: 109-109

src/main/java/konkuk/thip/notification/domain/value/NotificationRedirectSpecConverter.java (2)

25-25: 변경 승인: JSON 직렬화/역직렬화 오류는 서버 내부 문제입니다.

JPA AttributeConverter에서 발생하는 JSON 처리 오류는 데이터베이스 데이터 손상 또는 스키마 불일치를 의미하므로, InternalServerException으로 처리하는 것이 적절합니다.

Also applies to: 35-35


31-31: 변경 승인: null/blank 체크로 불필요한 역직렬화를 방지합니다.

데이터베이스 값이 null이거나 공백일 때 NotificationRedirectSpec.none()을 반환하는 것은 방어적 프로그래밍 관점에서 올바른 접근입니다. 불필요한 예외 발생을 방지합니다.

src/main/java/konkuk/thip/user/adapter/out/persistence/UserTokenBlacklistRedisAdapter.java (1)

55-55: 변경 승인: JSON 직렬화 오류는 내부 처리 오류입니다.

토큰 블랙리스트 정보를 JSON으로 직렬화할 때 발생하는 오류는 외부 API 오류(ExternalApiException)가 아닌 서버 내부 처리 오류이므로, InternalServerException으로 변경한 것이 의미적으로 정확합니다.

src/main/java/konkuk/thip/common/exception/code/ErrorCode.java (1)

227-227: 변경 승인: FCM 전송 시 배열 크기 불일치는 개발자 오류입니다.

FCM_TOKEN_DEVICE_ARRAY_MISMATCH ErrorCode 추가는 적절합니다. 메시지, FCM 토큰, 디바이스 ID 리스트의 크기가 다른 것은 서버 로직 오류이므로 INTERNAL_SERVER_ERROR(500)가 맞습니다.

src/main/java/konkuk/thip/config/AwsS3ImageUrlInitializer.java (1)

4-4: LGTM!

설정 누락 시 InternalServerException을 사용하는 것이 적절합니다. 서버 초기화 시점의 설정 오류는 내부 서버 문제로 분류하는 것이 타당합니다.

Also applies to: 23-23

src/main/java/konkuk/thip/book/adapter/out/api/aladin/AladinApiUtil.java (1)

5-5: LGTM!

알라딘 API 관련 예외를 ExternalApiException으로 변경한 것이 적절합니다. 외부 API 호출 실패와 응답 파싱 오류를 명확히 구분할 수 있습니다.

Also applies to: 57-57, 64-64

src/main/java/konkuk/thip/message/adapter/out/firebase/FirebaseAdapter.java (1)

14-15: LGTM!

ErrorCode 기반으로 FirebaseException을 통일하여 예외 처리의 일관성이 향상되었습니다. 디스코드 알림 전송 시 명확한 에러 코드를 전달할 수 있습니다.

Also applies to: 35-35, 42-42, 68-68, 72-72

src/main/java/konkuk/thip/book/adapter/out/api/naver/NaverApiUtil.java (1)

3-4: LGTM!

네이버 API 관련 예외를 내부 오류(InternalServerException)와 외부 API 오류(ExternalApiException)로 명확히 구분한 것이 적절합니다. 인코딩/URL 생성 오류는 내부 문제, API 요청/응답 오류는 외부 문제로 분류하는 것이 논리적입니다.

Also applies to: 67-67, 88-88, 100-102, 120-120

src/main/java/konkuk/thip/common/security/oauth2/CustomSuccessHandler.java (1)

6-6: LGTM!

허용 웹 도메인 설정 누락 시 InternalServerException을 사용하는 것이 적절합니다. 서버 설정 오류를 내부 서버 예외로 분류하는 것이 타당합니다.

Also applies to: 54-54

Comment thread src/main/java/konkuk/thip/common/Discord/DiscordClient.java Outdated
Comment thread src/main/java/konkuk/thip/common/Discord/DiscordClient.java Outdated
Comment thread src/main/java/konkuk/thip/common/discord/DiscordClient.java
Comment thread src/main/java/konkuk/thip/common/exception/code/ErrorCode.java
Comment thread src/main/java/konkuk/thip/common/exception/FirebaseException.java
Comment thread src/main/java/konkuk/thip/common/exception/handler/GlobalExceptionHandler.java Outdated
Comment thread src/main/java/konkuk/thip/common/exception/handler/GlobalExceptionHandler.java Outdated
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

♻️ Duplicate comments (1)
src/main/java/konkuk/thip/common/exception/handler/GlobalExceptionHandler.java (1)

131-132: 이전 리뷰에서 지적된 하드코딩된 더미 값이 여전히 남아있습니다.

requestIduserId를 "dummyRequestId", "dummyUserId"로 하드코딩하면 실제 장애 발생 시 요청 추적이 불가능합니다. 이전 리뷰 코멘트를 참고하여 MDC에서 실제 값을 가져오도록 수정해주세요.

🧹 Nitpick comments (2)
src/main/java/konkuk/thip/common/exception/handler/GlobalExceptionHandler.java (2)

7-7: 와일드카드 import 대신 명시적 import 사용을 권장합니다.

import konkuk.thip.common.exception.*은 코드의 명확성을 떨어뜨리고, 어떤 예외 클래스들이 실제로 사용되는지 파악하기 어렵게 만듭니다. 사용 중인 예외 클래스들(InternalServerException, ExternalApiException, FirebaseException, AuthException, BusinessException 등)을 명시적으로 import하는 것을 권장합니다.

-import konkuk.thip.common.exception.*;
+import konkuk.thip.common.exception.AuthException;
+import konkuk.thip.common.exception.BusinessException;
+import konkuk.thip.common.exception.ExternalApiException;
+import konkuk.thip.common.exception.FirebaseException;
+import konkuk.thip.common.exception.InternalServerException;

135-139: Discord 전송 실패에 대한 방어 로직이 추가되었습니다.

이전 리뷰에서 지적된 무한 재귀 문제를 try-catch로 해결했습니다. Discord 전송 실패 시 로그만 남기고 원본 에러 응답은 정상적으로 반환되도록 구현되어 있습니다.

단, Exception을 catch하면 InterruptedException도 포함되므로, 필요시 더 구체적인 예외 타입(RuntimeException.class 또는 ExternalApiException.class)을 명시하는 것을 고려해보세요.

-        } catch (Exception sendException) {
+        } catch (RuntimeException sendException) {
             log.error("[ServerErrorHandler] -> [Discord] 전송 실패", sendException);
         }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 47f2874 and b89820d.

📒 Files selected for processing (1)
  • src/main/java/konkuk/thip/common/exception/handler/GlobalExceptionHandler.java (3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build
🔇 Additional comments (3)
src/main/java/konkuk/thip/common/exception/handler/GlobalExceptionHandler.java (3)

28-28: LGTM!

DiscordClient 의존성 주입이 올바르게 구현되었습니다.


117-124: 스택 트레이스 요약 로직이 적절합니다.

스택 트레이스를 2번째, 3번째, 마지막 줄로 요약하는 로직이 안전하게 구현되었습니다. 경계 검사도 올바르게 수행되고 있습니다.


108-110: RuntimeException을 전역 핸들러에 포함해도 괜찮습니다.
프로덕션 코드에서 RuntimeException을 명시적으로 throw하는 부분은 테스트용 TestController에만 존재하며, unchecked 예외를 500으로 응답하는 fallback 처리로 적절합니다.

Likely an incorrect or invalid review comment.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

♻️ Duplicate comments (2)
src/main/java/konkuk/thip/common/discord/DiscordClient.java (1)

41-41: Discord 필드 길이 제한 초과 가능

Discord embed field의 value는 최대 1024자로 제한됩니다. 긴 스택 트레이스는 전송 실패를 유발할 수 있습니다.

다음 diff를 적용하여 스택 트레이스를 제한하세요:

+    private static final int MAX_FIELD_LENGTH = 1000;
+
     public void sendErrorMessage(String message, String stackTrace, String requestId, String userId) {
         if("test".equals(env)) return;
 
         WebClient webClient = WebClient.create();
 
         Map<String, Object> embedData = new HashMap<>();
         embedData.put("title", "THIP 서버 500 에러 발생");
 
         Map<String, String> field1 = new HashMap<>();
         field1.put("name", "발생시각");
         field1.put("value", LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss")));
 
         Map<String, String> field2 = new HashMap<>();
         field2.put("name", "에러 명");
-        field2.put("value", message);
+        field2.put("value", truncateField(message));
 
         Map<String, String> field3 = new HashMap<>();
         field3.put("name", "스택 트레이스");
-        field3.put("value", stackTrace);
+        field3.put("value", truncateField(stackTrace));
+
+    private String truncateField(String value) {
+        if (value == null) return "N/A";
+        if (value.length() <= MAX_FIELD_LENGTH) return value;
+        return value.substring(0, MAX_FIELD_LENGTH) + "...";
+    }
src/main/java/konkuk/thip/common/exception/handler/GlobalExceptionHandler.java (1)

130-131: Null 안전 기본값 적용 검토 (Lines 130-131)
MDC.get()가 null을 반환할 수 있어, @Async/@scheduled 등 비동기 환경에서 requestId/userId가 누락됩니다. 아래와 같이 명시적 기본값을 적용해주세요:

-        String requestId = MDC.get(REQUEST_ID.getValue());
-        String userId    = MDC.get(USER_ID.getValue());
+        String requestId = Optional.ofNullable(MDC.get(REQUEST_ID.getValue()))
+                .orElse("unknown-request");
+        String userId    = Optional.ofNullable(MDC.get(USER_ID.getValue()))
+                .orElse("unknown-user");
🧹 Nitpick comments (9)
build.gradle (2)

96-97: WebFlux와 Servlet 스택 혼용 확인 필요

spring-boot-starter-web (Servlet 스택)과 spring-boot-starter-webflux (Reactive 스택)를 동시에 사용하고 있습니다. WebClient만 필요한 경우 spring-boot-starter-webflux 전체 대신 spring-webfluxreactor-netty 의존성만 추가하는 것이 더 가볍습니다.

현재 구성에서는 Servlet 컨테이너가 우선되지만, 불필요한 Reactive 컴포넌트들이 클래스패스에 포함되어 애플리케이션 크기와 시작 시간에 영향을 줄 수 있습니다.

다음 diff를 적용하여 필요한 의존성만 추가하세요:

-    // Webflux
-    implementation 'org.springframework.boot:spring-boot-starter-webflux'
+    // WebClient for Discord webhook (without full WebFlux stack)
+    implementation 'org.springframework:spring-webflux'
+    implementation 'io.projectreactor.netty:reactor-netty-http'

99-100: Logstash-logback-encoder 버전 8.1로 업데이트 권장
보안 패치 및 최신 기능 확보를 위해 build.gradle의 의존성을 7.4에서 8.1로 업그레이드하세요.

src/main/java/konkuk/thip/common/dto/ErrorResponse.java (1)

21-21: requestId null 처리 고려

MDC.get(REQUEST_ID.getValue())가 null을 반환할 수 있습니다. MdcLoggingFilter가 실행되지 않는 비동기 스레드나 예외 상황에서 requestId가 null일 수 있습니다.

일관성을 위해 DiscordClient에서 사용하는 것처럼 requestId != null ? requestId : "N/A" 패턴을 적용하는 것을 고려하세요.

다음 diff를 적용하여 null 안전성을 개선하세요:

 private ErrorResponse(ResponseCode response) {
-    this(response.isSuccess(), response.getCode(), response.getMessage(), MDC.get(REQUEST_ID.getValue()));
+    this(response.isSuccess(), response.getCode(), response.getMessage(), 
+         MDC.get(REQUEST_ID.getValue()) != null ? MDC.get(REQUEST_ID.getValue()) : "N/A");
 }

 public static ErrorResponse of(ResponseCode response, String message) {
     StringBuilder sb =  new StringBuilder();
     sb.append(response.getMessage()).append(" ").append(message);
-    return new ErrorResponse(response.isSuccess(), response.getCode(), sb.toString(), MDC.get(REQUEST_ID.getValue()));
+    return new ErrorResponse(response.isSuccess(), response.getCode(), sb.toString(),
+         MDC.get(REQUEST_ID.getValue()) != null ? MDC.get(REQUEST_ID.getValue()) : "N/A");
 }

Also applies to: 31-31, 41-41

src/main/java/konkuk/thip/common/discord/DiscordClient.java (2)

56-62: 동기 블로킹 호출로 인한 성능 저하 우려

block() 호출은 요청 스레드를 차단합니다. Discord webhook이 느리거나 타임아웃되면 응답 시간에 영향을 줄 수 있습니다.

비동기 실행(@Async 또는 subscribe())으로 전환하여 요청 스레드를 차단하지 않는 것을 권장합니다.

다음과 같이 비동기로 변경하세요:

+@Async
 public void sendErrorMessage(String message, String stackTrace, String requestId, String userId) {
     if("test".equals(env)) return;
 
     WebClient webClient = WebClient.create();
     
     // ... embed 구성 코드 ...
     
     webClient.post()
             .uri(webhookUrl)
             .contentType(MediaType.APPLICATION_JSON)
             .bodyValue(payload)
             .retrieve()
             .bodyToMono(Void.class)
-            .block();
+            .subscribe(
+                result -> log.info("Discord 알림 전송 성공"),
+                error -> log.error("Discord 알림 전송 실패: {}", error.getMessage())
+            );
 }

그리고 Configuration 클래스에 @EnableAsync를 추가하세요.


26-26: WebClient 재사용 권장

매 호출마다 WebClient를 새로 생성하는 것은 비효율적입니다. WebClient를 빈으로 생성하여 재사용하는 것이 좋습니다.

다음 diff를 적용하여 WebClient를 필드로 선언하세요:

 @Component
 public class DiscordClient {
 
     @Value("${discord.env}")
     private String env;
 
     @Value("${discord.webhook-url}")
     private String webhookUrl;
+
+    private final WebClient webClient;
+
+    public DiscordClient() {
+        this.webClient = WebClient.builder()
+            .defaultHeader("Content-Type", "application/json")
+            .build();
+    }
 
     public void sendErrorMessage(String message, String stackTrace, String requestId, String userId) {
         if("test".equals(env)) return;
 
-        WebClient webClient = WebClient.create();
-
         // ... 나머지 코드 ...
     }
 }
src/main/java/konkuk/thip/common/dto/BaseResponse.java (2)

11-11: 사용하지 않는 @slf4j 어노테이션 제거

@Slf4j 어노테이션이 추가되었지만 이 클래스에서 log를 사용하지 않습니다.

다음 diff를 적용하여 불필요한 어노테이션을 제거하세요:

-@Slf4j
 @Getter
 @JsonPropertyOrder({"success", "code", "message", "requestId", "data"})
 public class BaseResponse<T> {

36-36: requestId null 처리 일관성

ErrorResponse와 동일하게 MDC.get(REQUEST_ID.getValue())가 null을 반환할 수 있습니다. 일관된 null 처리를 고려하세요.

ErrorResponse와 동일한 패턴 적용:

 private BaseResponse(ResponseCode response, T data) {
-    this(response.isSuccess(), response.getCode(), response.getMessage(), MDC.get(REQUEST_ID.getValue()), data);
+    this(response.isSuccess(), response.getCode(), response.getMessage(), 
+         MDC.get(REQUEST_ID.getValue()) != null ? MDC.get(REQUEST_ID.getValue()) : "N/A", data);
 }
src/main/java/konkuk/thip/common/exception/handler/GlobalExceptionHandler.java (2)

7-7: 와일드카드 import를 명시적 import로 교체하는 것을 권장합니다.

import konkuk.thip.common.exception.*;는 실제로 사용되는 예외 타입들을 숨기고, 잠재적인 네이밍 충돌 가능성을 높입니다. 핸들러에서 사용하는 구체적인 예외 클래스들(FirebaseException, InternalServerException, ExternalApiException 등)을 명시적으로 import하면 코드 가독성과 유지보수성이 향상됩니다.

-import konkuk.thip.common.exception.*;
+import konkuk.thip.common.exception.AuthException;
+import konkuk.thip.common.exception.BusinessException;
+import konkuk.thip.common.exception.ExternalApiException;
+import konkuk.thip.common.exception.FirebaseException;
+import konkuk.thip.common.exception.InternalServerException;

121-127: 스택트레이스 요약 로직의 경계 케이스를 명확히 해주세요.

lines.length == 3일 때 lines[1] + lines[2] + lines[lines.length-1]lines[2]를 중복 포함합니다. 의도한 동작이라면 주석으로 명시하고, 아니라면 조건을 lines.length <= 2로 수정하는 것을 권장합니다.

옵션 1: 경계 조건 조정

-        if (lines.length <= 3) {
+        if (lines.length <= 2) {
             stackSummary = stackTrace; // 짧으면 다 보여줌
         } else {
             stackSummary = lines[1] + "\n" + lines[2] + "\n" + lines[lines.length - 1];
         }

옵션 2: 의도 명시 (현재 로직 유지)

         // 스택트레이스 요약: 두,세번째 줄 + 마지막줄
         String[] lines = stackTrace.split("\n");
         String stackSummary;
-        if (lines.length <= 3) {
+        if (lines.length <= 3) {  // 3줄 이하는 전체 표시 (중복 포함)
             stackSummary = stackTrace;
         } else {
             stackSummary = lines[1] + "\n" + lines[2] + "\n" + lines[lines.length - 1];
         }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 648f8db and 3ee0084.

📒 Files selected for processing (10)
  • build.gradle (1 hunks)
  • src/main/java/konkuk/thip/common/discord/DiscordClient.java (1 hunks)
  • src/main/java/konkuk/thip/common/dto/BaseResponse.java (2 hunks)
  • src/main/java/konkuk/thip/common/dto/ErrorResponse.java (3 hunks)
  • src/main/java/konkuk/thip/common/exception/handler/GlobalExceptionHandler.java (3 hunks)
  • src/main/java/konkuk/thip/common/logging/LoggingConstant.java (1 hunks)
  • src/main/java/konkuk/thip/common/logging/MdcLoggingFilter.java (1 hunks)
  • src/main/java/konkuk/thip/common/security/filter/JwtAuthenticationFilter.java (4 hunks)
  • src/main/java/konkuk/thip/config/SecurityConfig.java (3 hunks)
  • src/main/resources/logback-spring.xml (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/konkuk/thip/common/dto/BaseResponse.java (1)
src/main/java/konkuk/thip/common/dto/ErrorResponse.java (1)
  • Getter (10-43)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build
🔇 Additional comments (8)
src/main/java/konkuk/thip/common/logging/LoggingConstant.java (1)

1-16: LGTM!

MDC 키를 중앙 집중화한 깔끔한 설계입니다. 문자열 중복을 방지하고 타입 안정성을 제공합니다.

src/main/java/konkuk/thip/common/security/filter/JwtAuthenticationFilter.java (3)

50-50: LGTM!

MdcLoggingFilter를 JWT 필터 전에 배치하여 요청 ID가 인증 처리 전에 설정되도록 했습니다. 필터 순서가 올바릅니다.

Also applies to: 74-74


98-114: LGTM!

토큰 검증 로직을 별도 메서드로 추출하여 가독성과 유지보수성이 향상되었습니다. 검증 단계가 명확하게 분리되어 있습니다.


77-77: MDC USER_ID 정리 로직 검증 완료
MdcLoggingFilter의 finally 블록에서 MDC.clear()가 호출됨을 확인했습니다. 현재 필터 순서(MdcLoggingFilter → JwtAuthenticationFilter)에서 USER_ID가 인증 후 설정되고 요청 종료 시 자동으로 정리됩니다.

src/main/java/konkuk/thip/common/logging/MdcLoggingFilter.java (1)

1-34: LGTM!

MDC 기반 요청 추적을 올바르게 구현했습니다. 주요 장점:

  • 클라이언트 제공 X-Request-ID 헤더 지원
  • UUID 자동 생성 폴백
  • finally 블록에서 MDC 정리하여 스레드 풀 오염 방지
src/main/java/konkuk/thip/config/SecurityConfig.java (3)

3-3: LGTM!

MdcLoggingFilter 임포트가 올바르게 추가되었습니다.


50-50: LGTM!

필드 선언이 기존 패턴과 일관되게 작성되었으며, 생성자 주입을 통해 올바르게 초기화됩니다.


74-74: 필터 순서와 MDC 정리가 모두 올바르게 구성되었습니다.

MdcLoggingFilter가 JwtAuthenticationFilter 이전에 실행되며, finally 블록에서 MDC.clear()를 호출해 요청 완료 후 MDC를 확실히 정리하고 있습니다.

Comment on lines +56 to +62
webClient.post()
.uri(webhookUrl)
.contentType(MediaType.APPLICATION_JSON)
.bodyValue(payload)
.retrieve()
.bodyToMono(Void.class)
.block();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

웹훅 전송 실패 처리 추가 필요

Discord 웹훅 호출 실패 시 에러 처리가 없습니다. 네트워크 문제나 Discord 서비스 장애 시 로그를 남기는 것이 좋습니다.

에러 핸들링을 추가하세요:

 webClient.post()
         .uri(webhookUrl)
         .contentType(MediaType.APPLICATION_JSON)
         .bodyValue(payload)
         .retrieve()
+        .onStatus(HttpStatusCode::isError, response -> 
+            response.bodyToMono(String.class)
+                .map(body -> new RuntimeException("Discord webhook failed: " + body))
+        )
         .bodyToMono(Void.class)
-        .block();
+        .block();
+        
+// 또는 subscribe 사용 시:
+        .subscribe(
+            result -> log.debug("Discord notification sent"),
+            error -> log.error("Failed to send Discord notification", error)
+        );

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/common/discord/DiscordClient.java around lines 56
to 62, the WebClient.post() call that sends the Discord webhook has no error
handling; wrap the call to detect HTTP errors and network exceptions, log a
descriptive error with the status code and response body (for non-2xx responses)
and catch runtime exceptions for network/timeouts, e.g., use WebClient's
exchangeToMono/onStatus to map non-successful responses to a Mono error and add
a try/catch (or subscribe with onError) that logs the full error and webhook
payload; ensure the method does not swallow failures silently and include enough
context (webhookUrl, payload summary, exception message) in the logs.

Comment on lines +36 to +40
<rollingPolicy class="ch.qos.logback.core.rolling.SizeAndTimeBasedRollingPolicy">
<fileNamePattern>${LOG_PATH}/info/info.%d{yyyy-MM-dd}.%i.log</fileNamePattern>
<maxFileSize>10MB</maxFileSize>
<maxHistory>10</maxHistory>
</rollingPolicy>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

디스크 용량 보호를 위해 totalSizeCap을 설정해주세요.

SizeAndTimeBasedRollingPolicytotalSizeCap이 빠져 있어, 하루에 생성되는 로그 파일 수가 무한히 늘 경우 10일 보관 기간 내에서도 디스크가 가득 차 서비스 장애로 이어질 수 있습니다. 전체 용량 상한을 지정해 로그가 일정 크기를 넘지 않도록 방지해주세요.

         <rollingPolicy class="ch.qos.logback.core.rolling.SizeAndTimeBasedRollingPolicy">
             <fileNamePattern>${LOG_PATH}/info/info.%d{yyyy-MM-dd}.%i.log</fileNamePattern>
             <maxFileSize>10MB</maxFileSize>
             <maxHistory>10</maxHistory>
+            <totalSizeCap>1GB</totalSizeCap>
         </rollingPolicy>

WARN_FILEERROR_FILE에도 동일하게 추가해주시길 권장드립니다.

Also applies to: 54-58, 72-76

🤖 Prompt for AI Agents
In src/main/resources/logback-spring.xml around lines 36-40 (and apply same
change to 54-58 and 72-76), the SizeAndTimeBasedRollingPolicy is missing a
totalSizeCap which can allow daily logs to consume all disk space; add a
totalSizeCap element (preferably referencing a configurable property such as
${LOG_TOTAL_SIZE_CAP} or a sensible default like 1GB) inside each
SizeAndTimeBasedRollingPolicy block so the total size of rolled files is capped
and prevents disk exhaustion.

Copy link
Copy Markdown
Collaborator

@seongjunnoh seongjunnoh left a comment

Choose a reason for hiding this comment

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

고생하셨습니다! 궁금즘 몇개 리뷰달아보았는데 확인부탁드립니다!

추가적으로 제가 아직 이해를 제대로 못한거 같아서 여쭤보는데,
지난 회의때 언급하신 filter 에서 catch 하지못한 jwt signature exception 의 핸들링 부분은 현재 pr 코드에서 해결이 된걸까요??

현재 jwt filter 부분에서는 단지 private 메서드로 jwt validate 관련 코드를 분리한 것 말고는 변경된 부분이 없는거 같아서 여쭤봅니다! @buzz0331

// 보상 시나리오 : 유저에게 "page 정보를 찾을 수 없는 책입니다. 직접 page 정보를 입력하세요" 라고 안내
// 일단 지금은 exception throw 만 진행
throw new BusinessException(BOOK_ALADIN_API_ISBN_NOT_FOUND);
throw new ExternalApiException(BOOK_ALADIN_API_ISBN_NOT_FOUND);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

Comment on lines -66 to +67
throw new BusinessException(BOOK_KEYWORD_ENCODING_FAILED);
throw new InternalServerException(BOOK_KEYWORD_ENCODING_FAILED);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

private Session currentTxSession() {
if (!TransactionSynchronizationManager.isActualTransactionActive()) {
throw new InvalidStateException(PERSISTENCE_TRANSACTION_REQUIRED);
throw new InternalServerException(PERSISTENCE_TRANSACTION_REQUIRED);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

Comment on lines +110 to +113
// 서버 내부 오류 예외 (500) 처리
@ExceptionHandler({RuntimeException.class, IllegalStateException.class,
FirebaseException.class, InternalServerException.class, ExternalApiException.class})
public ResponseEntity<ErrorResponse> handleServerErrors(Exception e) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM


String exceptionClassName = e.getClass().getSimpleName(); // 예외 클래스명
String combinedMessage = "[" + exceptionClassName + "] " + e.getMessage(); // 메시지에 예외 클래스명 포함
String stackTrace = org.apache.commons.lang3.exception.ExceptionUtils.getStackTrace(e);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

p2 : static import 문으로 수정 부탁드립니다!!


@Override
protected void doFilterInternal(HttpServletRequest request,
jakarta.servlet.http.HttpServletResponse response,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

p2 : static import 가 좋을 것 같습니다!

jakarta.servlet.http.HttpServletResponse response,
FilterChain filterChain) throws ServletException, IOException {
try {
String rawRequestId = request.getHeader("X-Request-ID");
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

p3 : 이게 nginx 에서 설정한 requestId 를 의미하는 건가요??

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nginx에서 넣어주는 헤더 정보입니다~

Comment on lines +107 to +109
if (!jwtUtil.validateToken(token)) {
throw new AuthException(AUTH_INVALID_TOKEN);
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

p3 : 현재 JwtUtil.validateToken() 메서드를 확인해보니, 내부에서 ExpiredJwtException 을 catch 해서 false 를 반환하고 있습니다!

이러면 JwtAuthenticationFilter.validateToken() 메서드에서 if(jwtUtil.isExpired(token)) 의 분기처리까지 도달하는 일이 없지 않나요??

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

엇 그러네요 현재 코드흐름에서는 만료된 토큰일경우 예외처리도 유효하지 않은 토큰입니다.로 처리되고있네요

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

엇 저는 이번 pr에서 저거 메서드 추출만 했던건데 기존 로직에서 문제가 있었나 보네요 메서드 순서만 바꾸면 될 것 같습니다


request.setAttribute(JWT_TOKEN_ATTRIBUTE.getValue(), token);
LoginUser loginUser = jwtUtil.getLoginUser(token);
MDC.put(USER_ID.getValue(), String.valueOf(loginUser.userId()));
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

확인했습니다

.formLogin(AbstractHttpConfigurer::disable)
.httpBasic(AbstractHttpConfigurer::disable)
.addFilterBefore(jwtAuthenticationFilter, UsernamePasswordAuthenticationFilter.class)
.addFilterBefore(mdcLoggingFilter, JwtAuthenticationFilter.class)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

오호 jwtFilter 에서 mdc를 활용하는 로직이 있어서 mdcFilter 를 jwtFilter 보다 앞쪽에 배치시킨 건가요??

Copy link
Copy Markdown
Contributor

@buzz0331 buzz0331 Oct 4, 2025

Choose a reason for hiding this comment

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

넵 그것도 맞고 둘다 같은 필터를 상속하고 있는 구조이기 때문에 두 필터의 우선순위를 정해주어야 해서 순서를 정해준 것입니다!

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (2)
src/main/java/konkuk/thip/common/exception/handler/GlobalExceptionHandler.java (2)

7-7: 와일드카드 import 대신 명시적 import 권장

현재 konkuk.thip.common.exception.*로 모든 예외를 import하고 있습니다. 명시적으로 사용되는 클래스만 import하면 코드 가독성이 향상되고 네이밍 충돌 가능성이 줄어듭니다.

예시:

-import konkuk.thip.common.exception.*;
+import konkuk.thip.common.exception.AuthException;
+import konkuk.thip.common.exception.BusinessException;
+import konkuk.thip.common.exception.FirebaseException;
+import konkuk.thip.common.exception.InternalServerException;
+import konkuk.thip.common.exception.ExternalApiException;

131-132: MDC 값이 없을 때 fallback 처리 추가 권장

MDC.get()은 값이 설정되지 않은 경우 null을 반환합니다. Discord 알림에서 null 대신 명확한 대체값(예: "unknown")을 사용하면 추적이 더 용이합니다.

다음과 같이 수정하시면 됩니다:

-        String requestId = MDC.get(REQUEST_ID.getValue());
-        String userId = MDC.get(USER_ID.getValue());
+        String requestId = Optional.ofNullable(MDC.get(REQUEST_ID.getValue()))
+                .orElse("unknown-requestId");
+        String userId = Optional.ofNullable(MDC.get(USER_ID.getValue()))
+                .orElse("unknown-userId");
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 3ee0084 and 573d268.

📒 Files selected for processing (1)
  • src/main/java/konkuk/thip/common/exception/handler/GlobalExceptionHandler.java (3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build
🔇 Additional comments (4)
src/main/java/konkuk/thip/common/exception/handler/GlobalExceptionHandler.java (4)

23-25: Static import 적용 완료

LoggingConstant의 상수들과 ExceptionUtils의 유틸리티 메서드를 static import로 깔끔하게 처리하셨습니다. 코드 가독성이 향상되었습니다.


121-128: 스택 트레이스 요약 로직 적절함

스택 트레이스가 길 경우 핵심 정보만 추출하는 로직이 잘 구현되었습니다. Discord 메시지 길이 제한을 고려한 실용적인 접근입니다.


135-139: Discord 전송 실패 방어 로직 우수함

Discord 웹훅 전송을 try-catch로 감싸서 전송 실패 시에도 원본 에러 응답이 정상적으로 반환되도록 구현하셨습니다. 과거 리뷰에서 지적된 무한 루프 이슈가 완전히 해결되었습니다.


112-113: RuntimeException 처리 대상 범위 확인 필요

RuntimeException은 거의 모든 unchecked 예외의 상위 타입(NullPointerException, IllegalArgumentException 등)을 포함하므로, 의도치 않은 예외까지 여기서 잡힐 수 있습니다. 특정 예외만 처리할 의도라면 더 구체적인 예외 타입을 명시하세요. 모든 예상치 못한 서버 에러를 포괄 처리하려는 의도라면 현재 구현을 유지하십시오.

@buzz0331
Copy link
Copy Markdown
Contributor

buzz0331 commented Oct 4, 2025

고생하셨습니다! 궁금즘 몇개 리뷰달아보았는데 확인부탁드립니다!

추가적으로 제가 아직 이해를 제대로 못한거 같아서 여쭤보는데, 지난 회의때 언급하신 filter 에서 catch 하지못한 jwt signature exception 의 핸들링 부분은 현재 pr 코드에서 해결이 된걸까요??

현재 jwt filter 부분에서는 단지 private 메서드로 jwt validate 관련 코드를 분리한 것 말고는 변경된 부분이 없는거 같아서 여쭤봅니다! @buzz0331

카톡에서 말씀드렸다시피 현재 저희 코드상에는 이슈가 없는 것 같긴합니다. 다만, 저희는 jwtauthenticationentrypoint가 자동으로 핸들링해주는 authenticationexception을 사용하고 있지 않으니 정석적으로는 jwtauthenticationentrypoint의 commence를 수동으로 호출해주는게 맞는 것 같은데 현재 이슈는 없으니 이 부분은 추후에 예상치 못한 이슈가 생기면 그때 고려해봐야 할 것 같습니다.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 573d268 and 9cab189.

📒 Files selected for processing (2)
  • src/main/java/konkuk/thip/common/logging/MdcLoggingFilter.java (1 hunks)
  • src/main/java/konkuk/thip/common/security/filter/JwtAuthenticationFilter.java (4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/main/java/konkuk/thip/common/logging/MdcLoggingFilter.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build
🔇 Additional comments (1)
src/main/java/konkuk/thip/common/security/filter/JwtAuthenticationFilter.java (1)

16-16: Import 추가 확인 완료

MDC 로깅을 위한 import가 적절하게 추가되었습니다.

Also applies to: 30-30


request.setAttribute(JWT_TOKEN_ATTRIBUTE.getValue(), token);
LoginUser loginUser = jwtUtil.getLoginUser(token);
MDC.put(USER_ID.getValue(), String.valueOf(loginUser.userId()));
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

MDC 정리 및 null 처리 확인 필요

MDC에 userId를 추가하는 것은 좋은 개선이지만 다음 사항들을 확인해주세요:

  1. MDC 정리: 현재 코드에서 MDC.remove() 또는 MDC.clear() 호출이 보이지 않습니다. AI 요약에 따르면 MdcLoggingFilter가 MDC 생명주기를 관리한다고 하는데, 이 필터와의 협력이 올바르게 구성되어 있는지 확인이 필요합니다. MDC가 정리되지 않으면 다른 요청에 컨텍스트가 누수될 수 있습니다.

  2. null 값 처리: Line 79-84를 보면 loginUser.userId()가 null일 수 있습니다(회원가입 토큰의 경우). 현재 코드는 String.valueOf(null)을 호출하여 문자열 "null"을 MDC에 저장하게 되는데, 이는 의도한 동작인지 확인이 필요합니다.

다음 스크립트로 MdcLoggingFilter의 MDC 정리 로직을 확인해주세요:

userId가 null일 때의 처리 개선 제안:

         request.setAttribute(JWT_TOKEN_ATTRIBUTE.getValue(), token);
         LoginUser loginUser = jwtUtil.getLoginUser(token);
-        MDC.put(USER_ID.getValue(), String.valueOf(loginUser.userId()));
+        if (loginUser.userId() != null) {
+            MDC.put(USER_ID.getValue(), String.valueOf(loginUser.userId()));
+        }

🏁 Script executed:

#!/bin/bash
# MdcLoggingFilter에서 MDC 정리가 구현되어 있는지 확인
ast-grep --pattern $'class MdcLoggingFilter {
  $$$
}'

Length of output: 0


🏁 Script executed:

#!/bin/bash
# MdcLoggingFilter 클래스 정의 위치 확인
rg -n "class MdcLoggingFilter" -g "*.java" -C3

# 해당 파일에서 MDC.remove 또는 MDC.clear 호출 여부 확인
rg -n "MDC\\.(remove|clear)" -g "*MdcLoggingFilter.java"

Length of output: 773


🏁 Script executed:

#!/bin/bash
# LoginUser.userId() 메서드 반환 타입 및 null 가능성 확인
rg -n "class LoginUser" -g "*.java" -C3
rg -n "userId()" -g "*.java" -C3

Length of output: 264644


🏁 Script executed:

#!/bin/bash
# getLoginUser 메서드 시그니처 확인
rg -n "getLoginUser" -g "*.java" -C3

# LoginUser 타입 정의(클래스 또는 레코드) 검색
rg -n "record LoginUser" -g "*.java"
rg -n "class LoginUser" -g "*.java"

Length of output: 3427


아래 스크립트를 실행해 LoginUser 레코드 정의(userId 필드 타입 및 null 허용 여부)와 MdcLoggingFilter 의 필터 순서·clear 호출 위치를 확인해주세요.


🏁 Script executed:

#!/bin/bash
sed -n '1,100p' src/main/java/konkuk/thip/common/security/oauth2/LoginUser.java
sed -n '1,50p' src/main/java/konkuk/thip/common/logging/MdcLoggingFilter.java

Length of output: 1606


null 체크 후 MDC에 userId 설정
MdcLoggingFilter가 finally 블록에서 MDC.clear()를 호출하므로 MDC 정리는 정상입니다. 신규 유저인 경우 userId가 null이어서 "null"이 저장되니, 아래처럼 null 체크 후에만 MDC.put을 호출하세요.

-        MDC.put(USER_ID.getValue(), String.valueOf(loginUser.userId()));
+        if (loginUser.userId() != null) {
+            MDC.put(USER_ID.getValue(), String.valueOf(loginUser.userId()));
+        }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
MDC.put(USER_ID.getValue(), String.valueOf(loginUser.userId()));
if (loginUser.userId() != null) {
MDC.put(USER_ID.getValue(), String.valueOf(loginUser.userId()));
}
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/common/security/filter/JwtAuthenticationFilter.java
around line 77, avoid writing the string "null" into MDC by checking
loginUser.userId() for null before calling MDC.put; only call
MDC.put(USER_ID.getValue(), String.valueOf(loginUser.userId())) when
loginUser.userId() != null (convert to string only after the null check).

Comment on lines +98 to +114
private void validateToken(String token) {
if (token == null) {
throw new AuthException(AUTH_TOKEN_NOT_FOUND);
}

if (userTokenBlacklistQueryPort.isTokenBlacklisted(token)) {
throw new AuthException(AUTH_BLACKLIST_TOKEN);
}

if (jwtUtil.isExpired(token)) {
throw new AuthException(AUTH_EXPIRED_TOKEN);
}

if (!jwtUtil.validateToken(token)) {
throw new AuthException(AUTH_INVALID_TOKEN);
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

유효성 검사 로직 추출 승인 및 중복 검증 확인 필요

유효성 검사 로직을 별도 메서드로 추출한 것은 가독성과 유지보수성 측면에서 좋은 개선입니다.

다만, 과거 리뷰 코멘트에서 지적된 내용을 확인해주세요: JwtUtil.validateToken() 메서드 내부에서 이미 ExpiredJwtException을 catch하여 false를 반환하고 있다면, Line 107-109의 만료 체크는 도달하지 않고 대신 Line 111-113에서 AUTH_INVALID_TOKEN 예외가 발생할 수 있습니다. 이 경우 만료된 토큰에 대해 더 구체적인 에러 메시지(AUTH_EXPIRED_TOKEN)를 제공하지 못할 수 있습니다.

다음 스크립트로 JwtUtil.validateToken()의 구현을 확인하여 ExpiredJwtException 처리 로직을 검증해주세요:


🏁 Script executed:

#!/bin/bash
# JwtUtil.validateToken() 메서드의 ExpiredJwtException 처리 확인
ast-grep --pattern $'class JwtUtil {
  $$$
  validateToken($$$) {
    $$$
  }
  $$$
}'

Length of output: 0


아래 스크립트로 JwtUtil.java 파일 내 validateToken 구현부를 검색하여 ExpiredJwtException 처리 여부를 확인해주세요.


🏁 Script executed:

#!/bin/bash
# JwtUtil.validateToken() 메서드 구현부 및 예외 처리 로직 확인
rg -nP --type java "validateToken\s*\(" -C5 --glob "*JwtUtil.java"

Length of output: 1108


아래 스크립트로 JwtUtil.validateToken() 내에서 ExpiredJwtException을 캐치하는지 확인해주세요.


🏁 Script executed:

rg -nP --type java --glob "src/main/java/konkuk/thip/common/security/util/JwtUtil.java" -C3 "ExpiredJwtException"

Length of output: 2036


아래 스크립트로 JwtUtil.javaisExpired 메서드 구현을 확인해주세요.


🏁 Script executed:

rg -nP --type java --glob "src/main/java/konkuk/thip/common/security/util/JwtUtil.java" -C5 "isExpired\s*\("

Length of output: 1168


ExpiredJwtException 처리 강화: isExpired()에서 예외 캐치 후 true 반환

  • JwtUtil.isExpired()ExpiredJwtException을 캐치하지 않아 필터에 라이브러리 예외가 전파됩니다.
  • 반면 validateToken()에서는 이미 ExpiredJwtException을 catch하여 false를 반환하므로, 만료된 토큰이 AUTH_EXPIRED_TOKEN 대신 잘못 처리될 수 있습니다.
  • isExpired() 내부에서 ExpiredJwtException을 캐치해 true를 반환하도록 수정해주세요.
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/common/security/filter/JwtAuthenticationFilter.java
around lines 98 to 114: the filter calls jwtUtil.isExpired(token) but JwtUtil
currently lets ExpiredJwtException propagate, causing library exceptions to
escape the filter and leading to inconsistent handling compared to
validateToken(); update JwtUtil.isExpired(token) to catch ExpiredJwtException
and return true when caught (keeping existing behavior for other exceptions), so
the filter receives a simple boolean and expired tokens are consistently
translated to AUTH_EXPIRED_TOKEN.

Copy link
Copy Markdown
Collaborator

@seongjunnoh seongjunnoh left a comment

Choose a reason for hiding this comment

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

확인했습니다!

@hd0rable hd0rable merged commit fc8ff1a into develop Oct 5, 2025
4 checks passed
@hd0rable hd0rable deleted the feat/#319-discord-error branch October 5, 2025 07:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[THIP2025-363] [feat] 어플리케이션 단위 에러 디스코드로 알림 전송기능 개발

3 participants