Skip to content

[#256][be][assistant] 피드백 리트라이/서킷브레이커 테스트 추가 및 오류 수정 외#257

Merged
HanaHww2 merged 3 commits intodevfrom
refactor/#256
Oct 27, 2025
Merged

[#256][be][assistant] 피드백 리트라이/서킷브레이커 테스트 추가 및 오류 수정 외#257
HanaHww2 merged 3 commits intodevfrom
refactor/#256

Conversation

@HanaHww2
Copy link
Copy Markdown
Contributor

@HanaHww2 HanaHww2 commented Oct 22, 2025

#️⃣연관된 이슈

변경 타입

  • 신규 기능 추가/수정
  • 버그 수정
  • 리팩토링
  • 설정
  • 비기능 (주석 등 기능에 영향을 주지 않음)

변경 내용

  • as-is

    • (변경 전 설명을 여기에 작성)
  • to-be(변경 후 설명을 여기에 작성)

    • 피드백 생성시 리트라이/서킷 브레이커 테스트 추가 및 오류 수정
    • 예외 클래스가 개별 팩토리 메소드 갖도록 개선

체크리스트

  • 코드가 제대로 동작하는지 확인했습니다.
  • 관련 테스트를 추가했습니다.
  • 문서(코드, 주석, README 등)를 업데이트했습니다.

코멘트

Summary by CodeRabbit

릴리스 노트

  • 버그 수정

    • 일일 피드백 사용량 초과 오류 처리 개선
    • AI 어시스턴트 호출 오류 처리 강화 (HTTP 4xx 오류 매핑)
    • 재시도 메커니즘 및 서킷 브레이커 동작 안정화
  • 새 기능

    • Retry-After 헤더 파싱 기능 추가
    • HTTP 상태 기반 오류 응답 지원 확대
    • 재시도 가능한 예외 처리 개선
  • 기타

    • 프롬프트 템플릿 구조 개선
    • 오류 처리 및 복원력 설정 최적화

- 피드백 생성시 리트라이/서킷 브레이커 테스트 추가 및 오류 수정
- 예외 클래스가 개별 팩토리 메소드 갖도록 개선
@HanaHww2 HanaHww2 self-assigned this Oct 22, 2025
@HanaHww2 HanaHww2 added ♻️ refactor 코드 리팩토링 🛠️ backend 백엔드 labels Oct 22, 2025
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Oct 22, 2025

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

개요

이 변경 사항은 예외 처리 인프라를 개선하고 신뢰성 메커니즘을 강화합니다. 주요 내용은 다음과 같습니다: 여러 예외 클래스에 정적 팩토리 메서드(from(), of()) 및 HttpStatus 기반 생성자를 추가하여 예외 인스턴스 생성을 표준화합니다. AssistantExceptionAssistantInfraException으로 대체하고 새로운 RateLimitException 추상 클래스 및 RetryAfterParser 유틸리티를 도입하여 재시도 정책을 초단위로 처리합니다. 회로 차단기 및 재시도 설정을 업데이트하고 에러 코드를 정규화하며(EXCEEDED_DAILY_USAGEEXCEEDED_DAILY_FEEDBACK_USAGE), 통합 테스트와 함께 강화된 오류 처리 경로를 검증합니다.

시퀀스 다이어그램

sequenceDiagram
    participant Client
    participant GeminiClient as GeminiAssistantClient
    participant ChatClient
    participant CircuitBreaker
    participant RetryRegistry
    participant ErrorHandler

    rect rgb(200, 220, 240)
    Note over GeminiClient,RetryRegistry: 성공 경로 (재시도 포함)
    Client->>GeminiClient: generateResponseFromPrompt()
    GeminiClient->>CircuitBreaker: 회로 상태 확인
    CircuitBreaker-->>GeminiClient: CLOSED (허용)
    
    rect rgb(255, 240, 200)
    Note over GeminiClient,ChatClient: 재시도 루프 (최대 3회)
    loop 시도 1, 2 (실패)
        GeminiClient->>ChatClient: prompt() 호출
        ChatClient-->>GeminiClient: 예외 발생
    end
    
    Note over GeminiClient,RetryRegistry: 시도 3 (성공)
    GeminiClient->>ChatClient: prompt() 호출
    ChatClient-->>GeminiClient: 응답 반환
    end
    
    GeminiClient->>ErrorHandler: 응답 변환
    GeminiClient-->>Client: 성공 결과
    end

    rect rgb(240, 200, 200)
    Note over GeminiClient,CircuitBreaker: 실패 경로 (회로 차단기 작동)
    Client->>GeminiClient: generateResponseFromPrompt()
    GeminiClient->>CircuitBreaker: 회로 상태 확인
    loop 최소 호출 수 도달 시까지
        GeminiClient->>ChatClient: prompt() 호출
        ChatClient-->>GeminiClient: 계속 실패
        CircuitBreaker->>CircuitBreaker: 실패 횟수 증가
    end
    
    Note over CircuitBreaker: 회로 OPEN (임계값 초과)
    Client->>GeminiClient: generateResponseFromPrompt()
    GeminiClient->>CircuitBreaker: 회로 상태 확인
    CircuitBreaker-->>GeminiClient: OPEN (차단)
    GeminiClient->>ErrorHandler: CallNotPermittedException 처리
    ErrorHandler->>ErrorHandler: AssistantInfraException<br/>(SERVICE_UNAVAILABLE) 생성
    GeminiClient-->>Client: 예외 발생 (재시도 불가)
    end
Loading
sequenceDiagram
    participant NotificationSender as WebClientFcmSender
    participant FCMServer as FCM Server
    participant Parser as RetryAfterParser
    participant Exception as NotificationEventFcmRetryableException

    rect rgb(200, 220, 240)
    Note over NotificationSender,Parser: 재시도 대기 시간 처리 (기존 vs 신규)
    
    NotificationSender->>FCMServer: 알림 전송 요청
    FCMServer-->>NotificationSender: 429 응답<br/>(Retry-After 헤더 포함)
    
    rect rgb(255, 240, 200)
    Note over NotificationSender,Parser: 신규 방식: RetryAfterParser 사용
    NotificationSender->>Parser: parseRetryAfterSeconds(headers)
    Parser->>Parser: Retry-After 헤더 파싱
    alt 숫자 형식 (초 단위)
        Parser-->>NotificationSender: retryAfterSeconds (long)
    else HTTP-Date 형식
        Parser->>Parser: UTC 시간으로 변환
        Parser->>Parser: 현재 시간과의 차이 계산
        Parser-->>NotificationSender: 남은 시간 (초)
    else 파싱 실패
        Parser-->>NotificationSender: -1
    end
    end
    
    NotificationSender->>Exception: of(HttpStatus, message,<br/>retryAfterSeconds)
    Exception-->>NotificationSender: RateLimitException 반환
    NotificationSender-->>NotificationSender: 재시도 스케줄링
    end
Loading

예상 코드 리뷰 난이도

🎯 4 (복잡함) | ⏱️ ~45분

근거:

  • 높은 이질성: 예외 클래스 (15개 이상) 전반에 걸친 반복적이지만 상세한 팩토리 메서드 추가
  • 복합 로직 변화: GeminiAssistantClient의 에러 처리 경로 재구성 (HTTP 4xx 매핑, 회로 차단기 처리, 새로운 예외 타입)
  • 새로운 추상화: RateLimitException 기본 클래스, RetryAfterParser 유틸리티, AssistantInfraException/AssistantInfraRetryableException 신규 클래스
  • 설정 변경: resilience4j 회로 차단기/재시도 설정의 상당한 구조 변화 (recordExceptions → ignoreExceptions, assistance → assistant)
  • 상호 의존성: 여러 파일에서 새로운 예외 클래스와 유틸리티를 참조하므로 각 변경 지점에서 일관성 검증 필요
  • 테스트 커버리지: GeminiAssistantClientTest 통합 테스트로 회로 차단기/재시도 동작 검증 필요
  • 파일 분산: 약 25개 파일에 걸친 변경으로 전체 맥락 파악 필요

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Out of Scope Changes Check ⚠️ Warning 대부분의 변경사항은 PR objectives와 관련이 있으나, back/src/main/resources/db/insert-template.sql의 변경은 피드백 생성 테스트, 리트라이/서킷브레이커 개선, 예외 클래스 리팩토링과 직접적인 관련성이 없습니다. 이 파일은 AI 어시스턴트 프롬프트 템플릿의 구조를 변경하는 것으로, 진술된 PR objectives에 포함되지 않았습니다. insert-template.sql의 변경사항을 이 PR에서 제거하거나, PR objectives에 프롬프트 템플릿 개선이 포함되었음을 명시하는 것이 필요합니다. 이 변경이 현재 PR의 리팩토링과 어떤 관계가 있는지 명확히 해주시기 바랍니다.
Docstring Coverage ⚠️ Warning Docstring coverage is 1.22% 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 PR 제목은 "피드백 리트라이/서킷브레이커 테스트 추가 및 오류 수정 외"로, 실제 변경사항 중 핵심인 GeminiAssistantClientTest 추가, GeminiAssistantClient의 에러 처리 개선, 예외 클래스 리팩토링 등을 반영하고 있습니다. 다만 "외"라는 표현이 모호하고 실제 변경 범위가 여러 도메인의 예외 클래스 수정, resilience 설정 변경 등으로 광범위하여 제목의 구체성이 다소 낮습니다.
Linked Issues Check ✅ Passed Issue #256의 요구사항은 "피드백 생성 테스트 추가 및 리팩토링"이며, PR objectives에서 구체적으로 정의한 "피드백 생성시 리트라이/서킷 브레이커 테스트 추가 및 오류 수정"과 "예외 클래스가 개별 팩토리 메소드 갖도록 개선"을 충족합니다. GeminiAssistantClientTest 추가로 테스트 요구사항이 충족되었고, 다양한 예외 클래스에 static factory 메소드가 추가되어 리팩토링 목표도 달성되었습니다.

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.

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: 15

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
back/src/main/java/com/rouby/notification/notificationEvent/infrastructure/messaging/fcm/WebClientFcmSender.java (2)

27-27: 구문 오류: 세미콜론 중복

private String projectId;; → 세미콜론 1개여야 합니다. 컴파일 불가합니다.

-  private String projectId;;
+  private String projectId;

48-57: HttpStatus 캐스팅으로 인한 ClassCastException 위험

resp.statusCode()는 HttpStatusCode이며, 미인식 상태코드 수신 시 (HttpStatus) 강제 캐스팅에서 ClassCastException 발생 가능합니다. HttpStatus.resolve()로 안전하게 변환하고, null 반환 시 기본값으로 처리하세요.

+                HttpStatus status = HttpStatus.resolve(resp.statusCode().value());
+                if (status == null) {
+                  status = HttpStatus.INTERNAL_SERVER_ERROR;
+                }
                 if (resp.statusCode().is4xxClientError() && resp.statusCode().value() != 429) {
                   return new NotificationEventFcmException(
-                      (HttpStatus) resp.statusCode(),
+                      status,
                       "FCM WebClient error: " + resp.statusCode() + " body=" + body);
                 }
-
                 return new NotificationEventFcmRetryableException(
-                    (HttpStatus) resp.statusCode(),
+                    status,
                     "FCM WebClient error: " + resp.statusCode() + " body=" + body,
                     RetryAfterParser.parseRetryAfterSeconds(resp.headers().asHttpHeaders()));

추가로, RetryAfterParser가 파싱 실패 시 -1을 반환하는데, 이 값이 재시도 지연으로 직접 전달됩니다. 기본 대기값 설정 또는 null 체크를 통한 방어 코드를 고려하세요(예: 최소 5초 이상).

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c0aab7b and 4adf2c1.

📒 Files selected for processing (26)
  • back/src/main/java/com/rouby/assistant/feedback/application/exception/FeedbackErrorCode.java (1 hunks)
  • back/src/main/java/com/rouby/assistant/feedback/application/exception/FeedbackException.java (1 hunks)
  • back/src/main/java/com/rouby/assistant/feedback/application/service/FeedbackWriteService.java (1 hunks)
  • back/src/main/java/com/rouby/assistant/prompt/application/exception/PromptException.java (1 hunks)
  • back/src/main/java/com/rouby/assistant/prompt/infrastructure/ai/GeminiAssistantClient.java (3 hunks)
  • back/src/main/java/com/rouby/assistant/prompt/infrastructure/exception/AssistantErrorCode.java (1 hunks)
  • back/src/main/java/com/rouby/assistant/prompt/infrastructure/exception/AssistantException.java (0 hunks)
  • back/src/main/java/com/rouby/assistant/prompt/infrastructure/exception/AssistantInfraException.java (1 hunks)
  • back/src/main/java/com/rouby/assistant/prompt/infrastructure/exception/AssistantInfraRetryableException.java (1 hunks)
  • back/src/main/java/com/rouby/batch/notification/DefaultDispatcher.java (1 hunks)
  • back/src/main/java/com/rouby/common/exception/RateLimitException.java (1 hunks)
  • back/src/main/java/com/rouby/common/utils/RetryAfterParser.java (1 hunks)
  • back/src/main/java/com/rouby/notification/email/application/exception/EmailException.java (1 hunks)
  • back/src/main/java/com/rouby/notification/notificationEvent/application/exception/NotificationEventException.java (1 hunks)
  • back/src/main/java/com/rouby/notification/notificationEvent/infrastructure/exception/NotificationEventFcmRetryableException.java (1 hunks)
  • back/src/main/java/com/rouby/notification/notificationEvent/infrastructure/exception/NotificationEventInfraException.java (1 hunks)
  • back/src/main/java/com/rouby/notification/notificationEvent/infrastructure/messaging/fcm/WebClientFcmSender.java (2 hunks)
  • back/src/main/java/com/rouby/notification/notificationtemplate/application/exception/NotificationTemplateException.java (1 hunks)
  • back/src/main/java/com/rouby/routine/daily_task/application/exception/DailyTaskException.java (1 hunks)
  • back/src/main/java/com/rouby/routine/routine_task/application/exception/RoutineTaskException.java (1 hunks)
  • back/src/main/java/com/rouby/schedule/application/exception/ScheduleException.java (2 hunks)
  • back/src/main/java/com/rouby/user/user/application/exception/UserException.java (1 hunks)
  • back/src/main/resources/db/insert-template.sql (1 hunks)
  • back/src/main/resources/properties/resilience.yml (3 hunks)
  • back/src/test/java/com/rouby/assistant/prompt/infrastructure/ai/GeminiAssistantClientTest.java (1 hunks)
  • back/src/test/java/com/rouby/common/support/IntegrationTestSupport.java (2 hunks)
💤 Files with no reviewable changes (1)
  • back/src/main/java/com/rouby/assistant/prompt/infrastructure/exception/AssistantException.java
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-07-12T23:01:37.791Z
Learnt from: hyezuu
PR: HI-dle/rouby#138
File: back/src/test/java/com/rouby/user/presentation/UserControllerTest.java:54-103
Timestamp: 2025-07-12T23:01:37.791Z
Learning: UserException inherits from a parent CustomException class in the Rouby project, making the from() method available through inheritance even though the of() method was removed from UserException itself.

Applied to files:

  • back/src/main/java/com/rouby/user/user/application/exception/UserException.java
📚 Learning: 2025-07-12T17:07:18.895Z
Learnt from: hyezuu
PR: HI-dle/rouby#123
File: back/src/main/java/com/rouby/user/application/UserService.java:30-31
Timestamp: 2025-07-12T17:07:18.895Z
Learning: hyezuu prefers using custom domain-specific exceptions like UserException instead of generic IllegalArgumentException for user-related validation errors in the Rouby project. This follows clean architecture principles for better error categorization and handling.

Applied to files:

  • back/src/main/java/com/rouby/user/user/application/exception/UserException.java
📚 Learning: 2025-10-21T08:43:41.986Z
Learnt from: HanaHww2
PR: HI-dle/rouby#255
File: front/src/features/assistant/constants.js:44-46
Timestamp: 2025-10-21T08:43:41.986Z
Learning: In the rouby project (HI-dle/rouby), the server and frontend use the error code key `EXCEEDED_DAILY_FEEDBACK_USAGE` for daily feedback usage limit exceeded errors in the assistant feedback feature.

Applied to files:

  • back/src/main/java/com/rouby/assistant/feedback/application/exception/FeedbackErrorCode.java
  • back/src/main/java/com/rouby/assistant/feedback/application/service/FeedbackWriteService.java
🧬 Code graph analysis (11)
back/src/main/java/com/rouby/assistant/prompt/infrastructure/exception/AssistantInfraRetryableException.java (1)
back/src/main/java/com/rouby/common/exception/RateLimitException.java (1)
  • Getter (6-16)
back/src/main/java/com/rouby/assistant/prompt/infrastructure/ai/GeminiAssistantClient.java (2)
back/src/main/java/com/rouby/assistant/prompt/infrastructure/exception/AssistantInfraException.java (1)
  • AssistantInfraException (7-32)
back/src/main/java/com/rouby/assistant/prompt/infrastructure/ai/ResponseConverterManager.java (1)
  • Component (10-46)
back/src/test/java/com/rouby/assistant/prompt/infrastructure/ai/GeminiAssistantClientTest.java (1)
back/src/main/java/com/rouby/assistant/prompt/infrastructure/exception/AssistantInfraException.java (1)
  • AssistantInfraException (7-32)
back/src/main/java/com/rouby/user/user/application/exception/UserException.java (1)
front/src/features/auth/password.js (1)
  • message (11-12)
back/src/main/java/com/rouby/routine/routine_task/application/exception/RoutineTaskException.java (1)
front/src/features/auth/password.js (1)
  • message (11-12)
back/src/main/java/com/rouby/common/exception/RateLimitException.java (1)
back/src/main/java/com/rouby/assistant/prompt/infrastructure/exception/AssistantInfraRetryableException.java (1)
  • Getter (7-17)
back/src/main/java/com/rouby/assistant/prompt/application/exception/PromptException.java (1)
front/src/features/auth/password.js (1)
  • message (11-12)
back/src/main/java/com/rouby/notification/email/application/exception/EmailException.java (1)
front/src/features/auth/password.js (1)
  • message (11-12)
back/src/main/java/com/rouby/assistant/feedback/application/service/FeedbackWriteService.java (1)
back/src/main/java/com/rouby/assistant/feedback/application/exception/FeedbackException.java (1)
  • FeedbackException (7-32)
back/src/main/java/com/rouby/common/utils/RetryAfterParser.java (2)
back/src/main/java/com/rouby/batch/notification/DefaultDispatcher.java (1)
  • Slf4j (34-269)
back/src/main/java/com/rouby/notification/notificationEvent/infrastructure/messaging/fcm/WebClientFcmSender.java (1)
  • Slf4j (21-69)
back/src/main/java/com/rouby/notification/notificationEvent/application/exception/NotificationEventException.java (1)
front/src/features/auth/password.js (1)
  • message (11-12)
🔇 Additional comments (25)
back/src/test/java/com/rouby/common/support/IntegrationTestSupport.java (2)

40-41: 캡슐화 개선 확인됨

DatabaseCleanUpprivate으로 변경하고 필드 순서를 재배치한 것은 좋은 개선입니다. 이 필드는 tearDown() 메서드 내부에서만 사용되므로 외부 노출이 필요하지 않습니다.


6-6: CircuitBreakerRegistry 추가가 테스트에서 활용 중임을 확인

GeminiAssistantClientTest에서 cbRegistry.circuitBreaker("assistant")로 실제 사용되고 있습니다. 이 import과 필드 추가는 테스트 클래스에서 서킷 브레이커를 제어하고 검증하기 위해 필수적인 변경사항입니다.

back/src/main/java/com/rouby/notification/notificationtemplate/application/exception/NotificationTemplateException.java (4)

5-5: LGTM!

새로운 생성자와 팩토리 메서드에 필요한 import가 올바르게 추가되었습니다.


21-31: LGTM! 팩토리 메서드 패턴이 올바르게 구현되었습니다.

팩토리 메서드가 잘 구현되었습니다:

  • from(ErrorCode): ErrorCode로부터 예외 생성 (의미론적으로 변환을 나타냄)
  • of(HttpStatus, ...): HttpStatus와 메시지로 예외 생성 (의미론적으로 직접 생성을 나타냄)

메서드 네이밍이 명확하고 일관성 있으며, private 생성자와의 조합이 적절합니다. PR 목표에서 언급한 "예외 클래스가 개별 팩토리 메소드를 갖도록 개선"이 잘 반영되었습니다.


9-11: 생성자 접근 제한 변경 검증 완료

전체 codebase 검색 결과, NotificationTemplateException의 생성자를 직접 호출하는 외부 코드가 없음이 확인되었습니다. 모든 인스턴스 생성이 from(), of() 팩토리 메서드를 통해 이루어지고 있으므로, 생성자를 private으로 제한하는 변경사항은 기존 코드에 영향을 주지 않습니다. 팩토리 메서드 패턴이 올바르게 적용되었습니다.


13-19: 부모 클래스 생성자 시그니처 검증 완료

CustomException에서 두 생성자가 모두 정확히 일치합니다:

  • protected CustomException(HttpStatus httpStatus, String message) (라인 28-31)
  • protected CustomException(HttpStatus httpStatus, String code, String message) (라인 33-37)

NotificationTemplateException의 private 생성자들이 올바르게 부모 클래스의 해당 생성자에 위임하고 있으며, 구현이 정확합니다.

back/src/main/resources/db/insert-template.sql (1)

103-118: 프롬프트 섹션 재구성의 일관성 검증 필요

"결과 응답 포맷" 섹션이 "응답 요소 지침" 앞으로 이동되었습니다. 이는 프롬프트 구조를 변경하는 것으로, 다음 사항들을 확인하면 좋겠습니다:

  1. 이전 중복 블록 제거 확인: AI 요약에서 이전에 중복된 "결과 응답 포맷" 블록이 있었으나 제거되었다고 명시되어 있습니다. 실제로 제거되었는지 확인이 필요합니다.
  2. 섹션 순서 변경의 영향: 기존 피드백 생성 로직이 특정 섹션 순서를 기대하지는 않는지 확인이 필요합니다.
  3. 테스트 커버리지: PR 목표에서 "피드백 생성시 리트라이/서킷 브레이커 테스트 추가"를 언급했으나, 이 템플릿 변경사항이 테스트로 커버되는지 확인이 필요합니다.
back/src/main/java/com/rouby/routine/daily_task/application/exception/DailyTaskException.java (4)

5-5: LGTM!

새로운 생성자에서 HttpStatus를 사용하기 위해 필요한 import입니다.


21-23: DailyTaskException.from() 팩토리 메소드가 코드베이스에서 적절히 사용되고 있습니다.

검증 결과, 새로운 팩토리 메소드는 다음 위치에서 실제로 사용 중입니다:

  • DailyTaskWriteService.java (라인 33, 40): 예외 생성 시 사용
  • DailyTaskControllerTest.java (라인 169, 202): 테스트에서 예외 모킹

사용 패턴이 일관되고 적절합니다. 코드 변경사항에 문제가 없습니다.


25-31: Based on my verification, the search results confirm that the new DailyTaskException.of(HttpStatus, ...) factory methods are not currently being used anywhere in the codebase.

The original review comment correctly identified the need to verify their usage. Here's what I found:

  • Zero usages of DailyTaskException.of(HttpStatus, ...) across the entire codebase
  • Similar factory pattern is used in the assistant module for AssistantInfraException, but not for DailyTaskException
  • The .of() naming is a recognized Java convention for static factory methods, popularized by Java's standard library

The factory methods follow best practices and are a legitimate API design choice. However, their current non-usage suggests either:

  1. These are proactive additions for future retry/circuit breaker logic that hasn't been implemented yet
  2. The implementation is incomplete


팩토리 메소드의 실제 사용처를 확인하세요.

코드 검증 결과, DailyTaskException.of(HttpStatus, ...) 팩토리 메소드는 현재 코드베이스 어디에서도 사용되지 않고 있습니다.

  • of() 네이밍 컨벤션은 Java 표준 라이브러리에서 널리 사용되는 정적 팩토리 메소드 관례입니다
  • 같은 코드베이스에서 AssistantInfraException은 이 패턴을 사용하고 있지만, DailyTaskException은 그렇지 않음
  • 현재 코드 어디에서도 이 팩토리 메소드를 호출하지 않음

이 메소드들이 향후 리트라이/서킷 브레이커 기능용으로 계획된 API인지, 아니면 미완성 구현인지 확인하고 명시해 주세요. 필요시 주석으로 의도를 문서화하는 것을 권장합니다.


13-19: 부모 클래스 호환성이 확인되었습니다. 코드 변경 승인합니다.

CustomException 클래스에서 필요한 모든 생성자 시그니처가 확인되었습니다:

  • protected CustomException(HttpStatus httpStatus, String message)
  • protected CustomException(HttpStatus httpStatus, String code, String message)

DailyTaskException의 private 생성자들이 올바르게 부모 클래스의 protected 생성자에 위임되고 있으며, 팩토리 메소드 패턴이 일관되게 적용되어 있습니다.

back/src/main/java/com/rouby/batch/notification/DefaultDispatcher.java (1)

204-206: 재시도 지연 시간 계산 로직이 올바르게 수정되었습니다.

retryAfter() (밀리초 단위 Long)에서 getRetryAfterSeconds() (초 단위 long)로 변경하고, Line 206에서 1000L을 곱하여 밀리초로 올바르게 변환하고 있습니다.

back/src/main/java/com/rouby/assistant/feedback/application/exception/FeedbackErrorCode.java (1)

15-15: 에러 코드명이 더 명확하게 개선되었습니다.

EXCEEDED_DAILY_USAGE에서 EXCEEDED_DAILY_FEEDBACK_USAGE로 변경하여 피드백 기능에 특화된 의미를 명확히 전달합니다. 학습된 정보에 따르면 프론트엔드에서도 이미 EXCEEDED_DAILY_FEEDBACK_USAGE를 사용하고 있어 일관성이 확보됩니다.

Based on learnings

back/src/main/java/com/rouby/assistant/feedback/application/service/FeedbackWriteService.java (1)

26-26: 리네이밍된 에러 코드가 올바르게 적용되었습니다.

FeedbackErrorCode.EXCEEDED_DAILY_FEEDBACK_USAGE를 사용하여 에러 코드 리네이밍이 일관되게 반영되었습니다.

back/src/main/java/com/rouby/user/user/application/exception/UserException.java (1)

5-31: 예외 생성을 위한 정적 팩토리 메서드 패턴이 잘 적용되었습니다.

  • from(ErrorCode): ErrorCode 기반 예외 생성
  • of(HttpStatus, String): HTTP 상태와 메시지로 예외 생성
  • of(HttpStatus, String, String): HTTP 상태, 코드, 메시지로 예외 생성

정적 팩토리 메서드를 통해 예외 생성 의도가 명확해지고, 프로젝트 전반의 예외 클래스들과 일관된 API를 제공합니다.

back/src/main/java/com/rouby/schedule/application/exception/ScheduleException.java (1)

5-39: 예외 생성 API가 일관되게 확장되었습니다.

기존 of(ErrorCode, String) 팩토리 메서드를 유지하면서, HTTP 상태 기반 팩토리 메서드들(from(ErrorCode), of(HttpStatus, String), of(HttpStatus, String, String))을 추가하여 다양한 예외 생성 시나리오를 지원합니다.

back/src/main/resources/properties/resilience.yml (2)

29-48: 서킷 브레이커 설정이 명확하게 개선되었습니다.

assistance에서 assistant로 네이밍을 변경하여 일관성을 확보하고, ignoreExceptionsAssistantInfraException을 추가하여 인프라 예외는 서킷 브레이커 실패로 카운트되지 않도록 올바르게 설정되었습니다.


79-94: 테스트 프로파일 설정이 적절합니다.

테스트 환경에 맞게 더 작은 sliding window size(10), 짧은 대기 시간(200ms, 10ms)을 설정하여 빠른 테스트 실행을 지원합니다.

back/src/main/java/com/rouby/assistant/feedback/application/exception/FeedbackException.java (1)

5-31: 정적 팩토리 메서드 패턴이 일관되게 적용되었습니다.

다른 예외 클래스들과 동일한 패턴으로 from()of() 팩토리 메서드를 추가하여, 프로젝트 전반의 예외 처리 API가 통일되었습니다.

back/src/main/java/com/rouby/assistant/prompt/application/exception/PromptException.java (1)

5-31: 예외 클래스 리팩토링이 완료되었습니다.

정적 팩토리 메서드를 추가하여 프로젝트의 모든 주요 예외 클래스들이 일관된 생성 패턴을 갖게 되었습니다.

back/src/main/java/com/rouby/assistant/prompt/infrastructure/exception/AssistantErrorCode.java (1)

13-17: 에러 코드 추가 LGTM

ASSISTANT_INVALID_REQUEST 추가로 4xx 매핑이 명확해졌습니다. 문서/에러 응답 스펙에 코드(ASSISTANT_INVALID_REQUEST)와 메시지 반영 부탁드립니다.

back/src/main/java/com/rouby/notification/notificationEvent/application/exception/NotificationEventException.java (2)

21-31: ---

검증 완료: 직접 생성 호출이 정적 팩토리 메서드 내부로 제한됨

수행된 검색에서 new NotificationEventException(...) 직접 호출은 오직 3곳만 발견되었으며, 모두 정적 팩토리 메서드(from, of, of) 내부입니다. 외부에서의 직접 생성 호출은 존재하지 않으므로 컴파일 에러 없이 팩토리 메서드 일원화가 완전히 적용되었습니다.


13-19: 기반 클래스 생성자 시그니처 호환성 확인 완료 - 문제 없음

CustomException의 전체 생성자를 확인한 결과, NotificationEventException에서 호출하는 두 super() 시그니처 모두 존재합니다:

  • super(status, message) → CustomException의 (HttpStatus httpStatus, String message) 생성자와 매핑됨
  • super(httpStatus, code, message) → CustomException의 (HttpStatus httpStatus, String code, String message) 생성자와 매핑됨

컴파일 실패 걱정은 없습니다. 다만 첫 번째 생성자에서 파라미터 명을 statushttpStatus로 통일하면 가독성이 향상될 것입니다.

back/src/main/java/com/rouby/routine/routine_task/application/exception/RoutineTaskException.java (2)

21-31: 정적 팩토리 메서드 패턴 적용 확인됨

검색 결과 new RoutineTaskException(...) 호출이 세 개의 정적 팩토리 메서드 내부(from(), of())에만 존재하며, 외부 코드에서의 직접 생성 호출은 없습니다. 정적 팩토리 메서드 패턴이 올바르게 적용되었습니다.


13-19: 생성자 시그니처 호환성 확인 완료 - 이슈 없음

검증 결과, RoutineTaskException의 super() 호출이 CustomException의 보호된(protected) 생성자와 완벽하게 일치합니다:

  • super(HttpStatus httpStatus, String message) ✓ CustomException 라인 27-31에 존재
  • super(HttpStatus httpStatus, String code, String message) ✓ CustomException 라인 33-37에 존재

파라미터 명도 양쪽 모두 httpStatus로 일관되게 사용 중입니다. 따라서 호환성 문제나 명명 불일치는 없습니다.

Comment on lines +25 to +31
public static AssistantInfraException of(HttpStatus httpStatus, String message) {
return new AssistantInfraException(httpStatus, message);
}

public static AssistantInfraException of(HttpStatus httpStatus, String code, String message) {
return new AssistantInfraException(httpStatus, code, message);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

HttpStatusCode 오버로드 추가 제안 (확장코드 안전)

다른 계층에서 HttpStatusCode를 직접 전달할 수 있습니다. 다음 오버로드를 추가하면 매핑 안전성이 좋아집니다.

   public static AssistantInfraException of(HttpStatus httpStatus, String code, String message) {
     return new AssistantInfraException(httpStatus, code, message);
   }
+
+  public static AssistantInfraException of(org.springframework.http.HttpStatusCode status, String message) {
+    var resolved = org.springframework.http.HttpStatus.resolve(status.value());
+    return new AssistantInfraException(resolved != null ? resolved : org.springframework.http.HttpStatus.INTERNAL_SERVER_ERROR, message);
+  }
📝 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
public static AssistantInfraException of(HttpStatus httpStatus, String message) {
return new AssistantInfraException(httpStatus, message);
}
public static AssistantInfraException of(HttpStatus httpStatus, String code, String message) {
return new AssistantInfraException(httpStatus, code, message);
}
public static AssistantInfraException of(HttpStatus httpStatus, String message) {
return new AssistantInfraException(httpStatus, message);
}
public static AssistantInfraException of(HttpStatus httpStatus, String code, String message) {
return new AssistantInfraException(httpStatus, code, message);
}
public static AssistantInfraException of(org.springframework.http.HttpStatusCode status, String message) {
var resolved = org.springframework.http.HttpStatus.resolve(status.value());
return new AssistantInfraException(resolved != null ? resolved : org.springframework.http.HttpStatus.INTERNAL_SERVER_ERROR, message);
}
🤖 Prompt for AI Agents
In
back/src/main/java/com/rouby/assistant/prompt/infrastructure/exception/AssistantInfraException.java
around lines 25-31, add overloaded static factory methods that accept Spring's
HttpStatusCode (to improve mapping safety across layers) and delegate to the
existing constructors by resolving the HttpStatusCode to an HttpStatus (use
HttpStatus.resolve(httpStatusCode.value()) and fall back to a sensible default
like HttpStatus.INTERNAL_SERVER_ERROR if null); provide both overloads that take
(HttpStatusCode httpStatusCode, String message) and (HttpStatusCode
httpStatusCode, String code, String message) so callers can pass HttpStatusCode
directly.

Comment on lines +25 to +31
public static NotificationEventInfraException of(HttpStatus httpStatus, String message) {
return new NotificationEventInfraException(httpStatus, message);
}

public static NotificationEventInfraException of(HttpStatus httpStatus, String code, String message) {
return new NotificationEventInfraException(httpStatus, code, message);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

HttpStatusCode 오버로드 추가로 상태코드 매핑 강화

확장 상태코드 대비 안전한 오버로드를 권장합니다.

   public static NotificationEventInfraException of(HttpStatus httpStatus, String code, String message) {
     return new NotificationEventInfraException(httpStatus, code, message);
   }
+
+  public static NotificationEventInfraException of(org.springframework.http.HttpStatusCode status, String message) {
+    var resolved = org.springframework.http.HttpStatus.resolve(status.value());
+    return new NotificationEventInfraException(resolved != null ? resolved : org.springframework.http.HttpStatus.INTERNAL_SERVER_ERROR, message);
+  }
📝 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
public static NotificationEventInfraException of(HttpStatus httpStatus, String message) {
return new NotificationEventInfraException(httpStatus, message);
}
public static NotificationEventInfraException of(HttpStatus httpStatus, String code, String message) {
return new NotificationEventInfraException(httpStatus, code, message);
}
public static NotificationEventInfraException of(HttpStatus httpStatus, String message) {
return new NotificationEventInfraException(httpStatus, message);
}
public static NotificationEventInfraException of(HttpStatus httpStatus, String code, String message) {
return new NotificationEventInfraException(httpStatus, code, message);
}
public static NotificationEventInfraException of(org.springframework.http.HttpStatusCode status, String message) {
var resolved = org.springframework.http.HttpStatus.resolve(status.value());
return new NotificationEventInfraException(resolved != null ? resolved : org.springframework.http.HttpStatus.INTERNAL_SERVER_ERROR, message);
}
🤖 Prompt for AI Agents
In
back/src/main/java/com/rouby/notification/notificationEvent/infrastructure/exception/NotificationEventInfraException.java
around lines 25 to 31, add safe overloads that accept an int statusCode (and an
int+code variant) to handle extended/non-standard status codes: implement public
static factory methods that call HttpStatus.resolve(statusCode) and if resolve
returns null fall back to a safe default (e.g.,
HttpStatus.INTERNAL_SERVER_ERROR) before delegating to the existing
constructors, ensuring both the message-only and code+message variants are
covered.

Comment thread back/src/main/resources/db/insert-template.sql
Comment on lines +112 to +116
.thenThrow(AssistantInfraException.class)
.thenThrow(new RuntimeException("always"));
verify(chatClient, times(1)).prompt(); // when 절에서 한 번 호출

// 무시되는 예외
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 | 🔴 Critical

thenThrow(Class) 사용 시 인스턴스 생성 실패 가능 — 팩토리로 교체

AssistantInfraException에는 무인자 public 생성자가 없어 Mockito가 예외 인스턴스를 만들지 못할 수 있습니다. 팩토리 결과를 넘기세요. 또한 stubbing 직후의 verify는 상호작용 카운트를 오염시킬 수 있어 제거 권장입니다.

-        .thenThrow(AssistantInfraException.class)
-        .thenThrow(new RuntimeException("always"));
-    verify(chatClient, times(1)).prompt(); // when 절에서 한 번 호출
+        .thenThrow(AssistantInfraException.of(HttpStatus.BAD_REQUEST, "bad"))
+        .thenThrow(new RuntimeException("always"));
+    // stubbing 자체 호출에 의존한 verify 제거
📝 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
.thenThrow(AssistantInfraException.class)
.thenThrow(new RuntimeException("always"));
verify(chatClient, times(1)).prompt(); // when 절에서 한 번 호출
// 무시되는 예외
.thenThrow(AssistantInfraException.of(HttpStatus.BAD_REQUEST, "bad"))
.thenThrow(new RuntimeException("always"));
// stubbing 자체 호출에 의존한 verify 제거
// 무시되는 예외
🤖 Prompt for AI Agents
In
back/src/test/java/com/rouby/assistant/prompt/infrastructure/ai/GeminiAssistantClientTest.java
around lines 112 to 116, the test uses thenThrow(AssistantInfraException.class)
which can fail because Mockito may try to instantiate the exception via a no-arg
constructor; replace it with stubbing that supplies an actual exception instance
(e.g., thenThrow(new AssistantInfraException(...)) or use a supplier/answer that
throws a constructed instance) and remove the immediate verify(chatClient,
times(1)).prompt() call after stubbing to avoid contaminating interaction
counts.

Copy link
Copy Markdown
Member

@letsgilit letsgilit left a comment

Choose a reason for hiding this comment

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

고생 많으셨습니다!

@HanaHww2 HanaHww2 merged commit b3c48b4 into dev Oct 27, 2025
1 check passed
@HanaHww2 HanaHww2 deleted the refactor/#256 branch October 27, 2025 06:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🛠️ backend 백엔드 ♻️ refactor 코드 리팩토링

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[assistant] 피드백 생성 테스트 추가 및 리팩토링

2 participants