Conversation
- BaseErrorCode 인터페이스: status, code, message를 포함한 표준 에러 형식 정의 - SentryException 인터페이스: Sentry 연동을 위한 예외 포맷 정의
- Sentry 연동을 위한 SentryUncheckedException 생성 - InvalidSlackPayloadException 정의 및 BaseErrorCode 기반 예외 생성 메서드 추가
- 메시지 생성 실패: SlackMessageBuildException - 전송 실패: SlackSendException - Sentry 연동을 위한 SentryCheckedException 기반 설계
- 서비스 유형별 환경 변수 prefix 매핑 - Slack, Discord 지원 - 웹훅 URL 누락 및 미지원 서비스 유형에 대한 커스텀 예외 처리 추가
- 서비스 유형별 환경 변수 prefix 매핑 - Slack, Discord 지원 - 웹훅 URL 누락 및 미지원 서비스 유형에 대한 커스텀 예외 처리 추가
- Java 11 HttpClient 기반 POST 요청 전송 유틸리티 추가 - 기본 timeout 10초 설정 - Content-Type 지정 가능
- SlackConstant 클래스 생성: Slack 메시지 포맷에 필요한 상수 정의 (날짜 포맷, 블록/텍스트/요소 타입, JSON 키 등) - Color enum 클래스 추가: 로그 레벨(fatal, error, info 등)에 따라 색상 코드를 매핑하는 getColorByLevel 메서드 구현
- NotificationService 인터페이스 정의: 알림 전송 기능을 위한 메서드 정의 - SlackNotificationService 클래스 구현: Slack 알림 메시지를 생성하고 전송하는 로직 구현 - NotificationServiceFactory 클래스 구현: 서비스 유형에 맞는 알림 서비스 구현체를 반환하는 팩토리 메서드 추가 - 지원하지 않는 서비스 유형에 대한 예외 처리: 서비스 유형이 지원되지 않으면 예외를 발생시키도록 처리
- ObjectMapper 인스턴스를 싱글턴 방식으로 생성하여 재사용할 수 있도록 설정 - Jackson 설정: - NULL 값을 포함하지 않도록 설정 - 알 수 없는 프로퍼티를 무시하도록 설정 - 날짜를 타임스탬프 대신 ISO 8601 형식으로 출력하도록 설정 - ObjectMapper 인스턴스를 안전하게 반환하는 메서드 제공
- Sentry 이벤트 정보를 담기 위한 DTO 클래스 추가 - `issueId`, `webUrl`, `message`, `datetime`, `level` 필드 포함 - `JsonNode`를 이용해 Sentry 이벤트 데이터를 DTO로 변환하는 `from` 메서드 제공
|
Summary by CodeRabbit
Summary by CodeRabbit
Summary by CodeRabbit
Walkthrough이번 변경에서는 Sentry Webhook을 수신하고, Slack 등으로 알림을 전송하는 전체 플로우가 구현되었습니다. AWS Lambda 환경에서 동작하는 핸들러가 추가되었으며, Sentry 이벤트 파싱, 환경 변수 기반 Webhook URL 조회, 서비스 타입별 NotificationService 팩토리 및 Slack 메시지 생성/전송 로직이 포함되었습니다. 또한, 예외 처리 체계와 Slack 메시지 데이터 구조, 색상 및 상수 관리, 공통 유틸리티 등이 새롭게 도입되었습니다. 기존의 Main 진입점은 삭제되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant API Gateway
participant Lambda(SentryWebhookHandler)
participant NotificationServiceFactory
participant SlackNotificationService
participant EnvUtil
participant HttpClientUtil
API Gateway->>Lambda(SentryWebhookHandler): Webhook 요청 전달
Lambda(SentryWebhookHandler)->>Lambda(SentryWebhookHandler): WebhookRequest/이벤트 파싱
Lambda(SentryWebhookHandler)->>EnvUtil: Webhook URL 조회
Lambda(SentryWebhookHandler)->>NotificationServiceFactory: 서비스 타입별 NotificationService 생성
Lambda(SentryWebhookHandler)->>SlackNotificationService: sendNotification 호출
SlackNotificationService->>HttpClientUtil: Slack Webhook POST 요청
HttpClientUtil-->>SlackNotificationService: 응답 반환
SlackNotificationService-->>Lambda(SentryWebhookHandler): 처리 결과 반환
Lambda(SentryWebhookHandler)-->>API Gateway: HTTP 응답 반환
Assessment against linked issues
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (2)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (6)
src/main/java/org/sopt/makers/global/constant/Color.java (1)
22-33: getColorByLevel 메서드 구현이 견고합니다.메서드가 null 체크와 입력 정규화를 적절히 처리하고 있어 안정적입니다. 또한 기본값으로 GRAY를 사용하는 것은 좋은 접근법입니다. 다만, 성능 최적화를 위해 다음과 같은 개선을 고려해 보세요:
현재 구현은 매번 모든 색상을 스트림으로 처리하고 있습니다. 정적 Map을 사용하면 성능을 향상시킬 수 있습니다:
@Getter @RequiredArgsConstructor public enum Color { RED("#FF0000", Set.of("fatal", "critical")), ORANGE("#E36209", Set.of("error")), YELLOW("#FFCC00", Set.of("warning")), GREEN("#36A64F", Set.of("info")), BLUE("#87CEFA", Set.of("debug")), GRAY("#AAAAAA", Set.of()); private final String value; private final Set<String> levels; + private static final Map<String, String> LEVEL_TO_COLOR_MAP = new HashMap<>(); + + static { + for (Color color : Color.values()) { + for (String level : color.getLevels()) { + LEVEL_TO_COLOR_MAP.put(level, color.getValue()); + } + } + } public static String getColorByLevel(String level) { if (level == null || level.trim().isEmpty()) { return GRAY.value; } String normalized = level.trim().toLowerCase(); - return Arrays.stream(Color.values()) - .filter(color -> color.levels.contains(normalized)) - .map(Color::getValue) - .findFirst() - .orElse(GRAY.value); + return LEVEL_TO_COLOR_MAP.getOrDefault(normalized, GRAY.value); } }src/main/java/org/sopt/makers/dto/SentryEventDetail.java (1)
16-24: JSON 변환 시 null/empty 처리를 고려해보세요.path() 메소드를 사용하여 안전하게 JSON 속성에 접근하고 있지만, 속성이 없거나 null인 경우에 대한 추가 처리가 없습니다. 이 경우 문자열 "null"이 반환될 수 있습니다.
다음과 같이 개선할 수 있습니다:
public static SentryEventDetail from(JsonNode eventNode) { return SentryEventDetail.builder() - .issueId(eventNode.path("issue_id").asText()) - .webUrl(eventNode.path("web_url").asText()) - .message(eventNode.path("message").asText()) - .datetime(eventNode.path("datetime").asText()) - .level(eventNode.path("level").asText()) + .issueId(eventNode.path("issue_id").asText("")) + .webUrl(eventNode.path("web_url").asText("")) + .message(eventNode.path("message").asText("")) + .datetime(eventNode.path("datetime").asText("")) + .level(eventNode.path("level").asText("")) .build(); }또는
null체크를 추가하는 방식도 고려할 수 있습니다:public static SentryEventDetail from(JsonNode eventNode) { + String issueId = eventNode.path("issue_id").isNull() ? "" : eventNode.path("issue_id").asText(); + String webUrl = eventNode.path("web_url").isNull() ? "" : eventNode.path("web_url").asText(); + String message = eventNode.path("message").isNull() ? "" : eventNode.path("message").asText(); + String datetime = eventNode.path("datetime").isNull() ? "" : eventNode.path("datetime").asText(); + String level = eventNode.path("level").isNull() ? "" : eventNode.path("level").asText(); return SentryEventDetail.builder() - .issueId(eventNode.path("issue_id").asText()) - .webUrl(eventNode.path("web_url").asText()) - .message(eventNode.path("message").asText()) - .datetime(eventNode.path("datetime").asText()) - .level(eventNode.path("level").asText()) + .issueId(issueId) + .webUrl(webUrl) + .message(message) + .datetime(datetime) + .level(level) .build(); }src/main/java/org/sopt/makers/global/exception/unchecked/SentryUncheckedException.java (1)
8-16: 예외 계층 구조가 잘 설계되었습니다.SentryException 인터페이스를 구현하고 BaseErrorCode를 포함하는 방식으로 예외 클래스가 잘 설계되었습니다. 다만, RuntimeException은 Serializable을 구현하므로 serialVersionUID를 추가하는 것을 고려해 보세요.
@Getter public class SentryUncheckedException extends RuntimeException implements SentryException { + private static final long serialVersionUID = 1L; private final BaseErrorCode baseErrorCode; protected SentryUncheckedException(BaseErrorCode baseErrorCode) { super(baseErrorCode.getMessage()); this.baseErrorCode = baseErrorCode; } }src/main/java/org/sopt/makers/global/util/EnvUtil.java (1)
36-44: 환경 변수 값 캐싱 고려System.getenv() 호출은 비용이 크지 않지만, 자주 호출되는 경우 환경 변수 값을 캐싱하는 것을 고려해 볼 수 있습니다. 특히 애플리케이션 실행 중에 환경 변수가 변경되지 않는다면 더욱 효율적일 수 있습니다.
src/main/java/org/sopt/makers/global/constant/SlackConstant.java (1)
14-17: 사용되지 않는 상수 제거 고려
HEADER_CONTENT_TYPE는 현재 코드에서 참조되지 않습니다.
불필요한 상수는 제거하여 유지보수 오버헤드를 줄여 주세요.src/main/java/org/sopt/makers/global/exception/message/ErrorMessage.java (1)
11-17: HTTP 상태 코드 재검토 권장
INVALID_SLACK_PAYLOAD(잘못된 요청)는 400,SLACK_NETWORK_ERROR(외부 서비스 연결 실패)는 504(게이트웨이 타임아웃) 등이 REST 관례에 더 부합합니다.
상태 코드를 조정해 API 일관성을 높여 보세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
src/main/java/org/sopt/makers/Main.java(0 hunks)src/main/java/org/sopt/makers/dto/SentryEventDetail.java(1 hunks)src/main/java/org/sopt/makers/global/config/ObjectMapperConfig.java(1 hunks)src/main/java/org/sopt/makers/global/constant/Color.java(1 hunks)src/main/java/org/sopt/makers/global/constant/SlackConstant.java(1 hunks)src/main/java/org/sopt/makers/global/exception/base/BaseErrorCode.java(1 hunks)src/main/java/org/sopt/makers/global/exception/base/SentryException.java(1 hunks)src/main/java/org/sopt/makers/global/exception/checked/SentryCheckedException.java(1 hunks)src/main/java/org/sopt/makers/global/exception/checked/SlackMessageBuildException.java(1 hunks)src/main/java/org/sopt/makers/global/exception/checked/SlackSendException.java(1 hunks)src/main/java/org/sopt/makers/global/exception/message/ErrorMessage.java(1 hunks)src/main/java/org/sopt/makers/global/exception/unchecked/InvalidSlackPayloadException.java(1 hunks)src/main/java/org/sopt/makers/global/exception/unchecked/SentryUncheckedException.java(1 hunks)src/main/java/org/sopt/makers/global/exception/unchecked/UnsupportedServiceTypeException.java(1 hunks)src/main/java/org/sopt/makers/global/exception/unchecked/WebhookUrlNotFoundException.java(1 hunks)src/main/java/org/sopt/makers/global/util/EnvUtil.java(1 hunks)src/main/java/org/sopt/makers/global/util/HttpClientUtil.java(1 hunks)src/main/java/org/sopt/makers/service/NotificationService.java(1 hunks)src/main/java/org/sopt/makers/service/NotificationServiceFactory.java(1 hunks)src/main/java/org/sopt/makers/service/SlackNotificationService.java(1 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/org/sopt/makers/Main.java
🧰 Additional context used
🧬 Code Graph Analysis (4)
src/main/java/org/sopt/makers/service/NotificationService.java (2)
src/main/java/org/sopt/makers/global/exception/checked/SlackMessageBuildException.java (1)
SlackMessageBuildException(5-13)src/main/java/org/sopt/makers/global/exception/checked/SlackSendException.java (1)
SlackSendException(5-13)
src/main/java/org/sopt/makers/global/util/EnvUtil.java (2)
src/main/java/org/sopt/makers/global/exception/unchecked/UnsupportedServiceTypeException.java (1)
UnsupportedServiceTypeException(5-13)src/main/java/org/sopt/makers/global/exception/unchecked/WebhookUrlNotFoundException.java (1)
WebhookUrlNotFoundException(5-13)
src/main/java/org/sopt/makers/service/SlackNotificationService.java (3)
src/main/java/org/sopt/makers/global/exception/checked/SlackMessageBuildException.java (1)
SlackMessageBuildException(5-13)src/main/java/org/sopt/makers/global/exception/checked/SlackSendException.java (1)
SlackSendException(5-13)src/main/java/org/sopt/makers/service/NotificationServiceFactory.java (1)
Slf4j(13-38)
src/main/java/org/sopt/makers/service/NotificationServiceFactory.java (2)
src/main/java/org/sopt/makers/global/exception/unchecked/UnsupportedServiceTypeException.java (1)
UnsupportedServiceTypeException(5-13)src/main/java/org/sopt/makers/service/SlackNotificationService.java (1)
Slf4j(29-193)
🔇 Additional comments (18)
src/main/java/org/sopt/makers/global/exception/base/BaseErrorCode.java (1)
3-7: 인터페이스 설계가 명확하고 간결합니다.오류 코드를 위한 인터페이스 설계가 잘 되어 있습니다. HTTP 상태 코드, 오류 코드, 오류 메시지를 일관되게 제공하는 메서드를 명확하게 정의했습니다. 이 인터페이스는 예외 처리 시스템에 표준화된 오류 정보를 제공하기 위한 좋은 기반이 될 것입니다.
src/main/java/org/sopt/makers/global/exception/base/SentryException.java (1)
3-5: Sentry 통합을 위한 인터페이스 설계가 잘 되어 있습니다.Sentry 관련 예외를 식별하고 적절한 오류 코드를 제공할 수 있도록 인터페이스가 잘 설계되어 있습니다. 이는 BaseErrorCode와 잘 통합되어 일관된 예외 처리 시스템을 구축하는 데 도움이 될 것입니다.
src/main/java/org/sopt/makers/global/util/HttpClientUtil.java (1)
13-19: 유틸리티 클래스 구조가 적절합니다.유틸리티 클래스를 final로 선언하고 private 생성자를 사용한 설계가 좋습니다. 또한 HTTP 클라이언트를 싱글톤으로 관리하고 타임아웃을 상수로 정의한 점도 좋은 접근 방식입니다.
src/main/java/org/sopt/makers/global/constant/Color.java (1)
11-21: 컬러 열거형 설계가 체계적입니다.로그 레벨에 따른 색상 매핑이 잘 정의되어 있습니다. 색상 값과 관련 로그 레벨을 함께 관리하는 접근 방식이 좋습니다. Lombok을 사용하여 getter와 생성자를 자동화한 것도 효율적입니다.
src/main/java/org/sopt/makers/global/config/ObjectMapperConfig.java (1)
1-29: 적절한 싱글톤 패턴과 ObjectMapper 설정 구현ObjectMapper 설정을 위한 싱글톤 패턴이 적절하게 구현되었습니다. 정적 홀더 패턴을 사용하여 스레드 안전한 지연 초기화를 제공하고 있으며, ObjectMapper의 설정이 효율적으로 구성되어 있습니다.
다음 설정들이 잘 적용되어 있습니다:
JsonInclude.Include.NON_NULL: null 값 제외 직렬화DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES: 알 수 없는 속성 무시SerializationFeature.WRITE_DATES_AS_TIMESTAMPS: 날짜를 ISO-8601 형식으로 직렬화// 불변성을 보장하기 위한 선택적 개선 사항(필수는 아님) private static ObjectMapper createObjectMapper() { ObjectMapper objectMapper = new ObjectMapper(); objectMapper.setSerializationInclusion(JsonInclude.Include.NON_NULL); objectMapper.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false); objectMapper.configure(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS, false); + // 설정 완료 후 불변 상태로 만들기 + // objectMapper.findAndRegisterModules(); return objectMapper; }src/main/java/org/sopt/makers/global/exception/unchecked/InvalidSlackPayloadException.java (1)
1-13: 예외 계층 구조가 잘 설계되었습니다Slack 페이로드 관련 예외 처리가 명확하게 구현되어 있습니다. 정적 팩토리 메서드를 통해 가독성 있는 예외 생성 방식을 제공하고 있습니다.
다만, 원인 예외를 연결할 수 있는 생성자가 있으면 더 완벽할 것 같습니다:
public class InvalidSlackPayloadException extends SentryUncheckedException { public InvalidSlackPayloadException(BaseErrorCode errorCode) { super(errorCode); } + public InvalidSlackPayloadException(BaseErrorCode errorCode, Throwable cause) { + super(errorCode, cause); + } public static InvalidSlackPayloadException from(BaseErrorCode errorCode) { return new InvalidSlackPayloadException(errorCode); } + public static InvalidSlackPayloadException from(BaseErrorCode errorCode, Throwable cause) { + return new InvalidSlackPayloadException(errorCode, cause); + } }src/main/java/org/sopt/makers/global/exception/checked/SlackSendException.java (1)
1-13: 명확한 예외 처리 구현Slack 메시지 전송 오류에 대한 예외 클래스가 체계적으로 구현되어 있습니다. 체크드 예외를 통해 호출자가 명시적으로 예외를 처리하도록 강제하는 것이 적절합니다.
원인이 되는 예외를 전달할 수 있는 생성자를 추가하면 더 유용할 것 같습니다:
public class SlackSendException extends SentryCheckedException { public SlackSendException(BaseErrorCode errorCode) { super(errorCode); } + public SlackSendException(BaseErrorCode errorCode, Throwable cause) { + super(errorCode, cause); + } public static SlackSendException from(BaseErrorCode errorCode) { return new SlackSendException(errorCode); } + public static SlackSendException from(BaseErrorCode errorCode, Throwable cause) { + return new SlackSendException(errorCode, cause); + } }src/main/java/org/sopt/makers/global/exception/checked/SlackMessageBuildException.java (1)
1-13: 일관된 예외 처리 패턴 적용Slack 메시지 구성 오류에 대한 예외 클래스가 일관된 패턴으로 구현되어 있습니다. 다른 예외 클래스들과 동일한 패턴을 따르고 있어 코드 일관성이 좋습니다.
다른 예외 클래스들과 마찬가지로 원인 예외를 처리할 수 있는 생성자를 추가하면 더 완성도 높은 구현이 될 것 같습니다:
public class SlackMessageBuildException extends SentryCheckedException { public SlackMessageBuildException(BaseErrorCode errorCode) { super(errorCode); } + public SlackMessageBuildException(BaseErrorCode errorCode, Throwable cause) { + super(errorCode, cause); + } public static SlackMessageBuildException from(BaseErrorCode errorCode) { return new SlackMessageBuildException(errorCode); } + public static SlackMessageBuildException from(BaseErrorCode errorCode, Throwable cause) { + return new SlackMessageBuildException(errorCode, cause); + } }src/main/java/org/sopt/makers/global/exception/unchecked/WebhookUrlNotFoundException.java (1)
5-13: 예외 처리 계층 구조가 잘 설계되어 있습니다.SentryUncheckedException을 상속받아 웹훅 URL을 찾을 수 없을 때 발생하는 예외를 명확하게 정의했습니다. 정적 팩토리 메소드
from을 제공하여 가독성이 좋고 코드 재사용성이 높습니다.src/main/java/org/sopt/makers/global/exception/checked/SentryCheckedException.java (1)
8-15: 체크 예외 처리 구조가 체계적으로 구현되었습니다.SentryException 인터페이스를 구현하고 BaseErrorCode를 활용하여 일관된 에러 코드 관리 방식을 구축했습니다. protected 생성자를 통해 상속 구조를 명확히 하고, Lombok의 @Getter를 사용하여 불필요한 보일러플레이트 코드를 줄였습니다.
src/main/java/org/sopt/makers/dto/SentryEventDetail.java (1)
8-15: Java의 record 기능과 빌더 패턴을 적절히 활용했습니다.DTO로 record를 사용한 것은 좋은 선택입니다. AccessLevel.PRIVATE 설정으로 빌더 접근 제어를 통해 생성 방식을 제한한 것도 좋습니다.
src/main/java/org/sopt/makers/global/exception/unchecked/UnsupportedServiceTypeException.java (1)
5-13: 서비스 타입 예외 처리 구현이 일관되게 작성되었습니다.WebhookUrlNotFoundException과 유사한 패턴으로 구현되어 일관성 있는 코드 스타일을 유지하고 있습니다. 정적 팩토리 메소드를 사용하여 인스턴스 생성을 추상화한 것도 좋은 방법입니다.
src/main/java/org/sopt/makers/service/NotificationService.java (1)
8-21: 인터페이스 설계가 깔끔합니다.알림 서비스를 위한 인터페이스가 잘 설계되었습니다. 메서드 시그니처와 예외 처리가 명확하게 정의되어 있고, Javadoc 주석도 적절히 작성되어 있습니다.
src/main/java/org/sopt/makers/service/NotificationServiceFactory.java (2)
13-37: 팩토리 패턴이 잘 구현되었습니다.정적 맵을 사용한 서비스 타입별 구현체 관리가 잘 되어 있습니다. 지원하지 않는 서비스 타입에 대한 예외 처리도 적절합니다.
21-21: 디스코드 서비스 구현 계획이 있나요?주석 처리된 DiscordNotificationService 항목이 있는데, 향후 구현 계획이 있는지 확인해 보세요. 만약 구현 계획이 있다면 TODO 주석으로 명시해두는 것이 좋을 것 같습니다.
src/main/java/org/sopt/makers/global/util/EnvUtil.java (2)
11-44: 환경 변수 처리 로직이 깔끔합니다.서비스 타입에 따른 웹훅 URL을 환경 변수에서 가져오는 로직이 잘 구현되어 있습니다. 예외 처리와 로깅도 적절합니다.
46-52: switch 식 사용이 적절합니다.Java의 새로운 switch 식을 사용하여 코드가 간결하고 가독성이 좋습니다. 향후 지원할 서비스가 추가될 때 이 부분만 확장하면 됩니다.
src/main/java/org/sopt/makers/service/SlackNotificationService.java (1)
32-36:ObjectMapper싱글톤 사용 👍DI 없이도
ObjectMapperConfig싱글톤을 주입해 thread‑safe 하게 재사용하도록 한 점이 좋습니다.
src/main/java/org/sopt/makers/service/SlackNotificationService.java
Outdated
Show resolved
Hide resolved
- Slack API 요청 구조를 표현하는 Value Object 추가 - Block, Element, Text 인터페이스 및 구현체 추가 - 모든 VO는 불변 객체로 record 타입으로 구현 - 각 클래스에 newInstance 패턴의 정적 팩토리 메서드 추가 - 타입 안전성과 유지보수성 향상을 위한 클래스 구조 설계
- Map<String, Object> 데이터 구조를 타입 안전한 VO로 교체 - 메서드 반환 타입을 구체적인 VO 타입으로 변경 - JSON 키를 직접 다루는 대신 VO 객체의 정적 팩토리 메서드 활용 - 예외 처리 로직은 기존 방식 유지
- APIGatewayProxyRequestEvent 기반 Sentry 웹훅 수신 처리 - 팀, 타입, 서비스 정보 추출 및 알림 전송 처리 - 이벤트 유효성 검사 및 SentryEventDetail 추출 로직 추가 - 성공/오류 응답 포맷 JSON 형태로 구성 - checked/unchecked/custom 예외에 따른 응답 처리 분기
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/main/java/org/sopt/makers/vo/slack/block/SectionBlock.java (3)
8-12: 레코드의 필드에 대한 Javadoc 주석 추가 권장SectionBlock 레코드의 필드에 대한 설명이 없어 다른 개발자들이 각 필드의 목적과 사용법을 이해하기 어려울 수 있습니다. 각 필드(type, fields, text)의 역할에 대한 Javadoc 주석을 추가하는 것이 좋습니다.
1-20: 클래스 수준의 Javadoc 문서화 필요SectionBlock 클래스의 목적과 Slack API와의 관계에 대한 설명이 없습니다. 클래스 수준의 Javadoc을 추가하여 이 클래스가 Slack의 Section Block을 표현하는 것임을 명시하고, 관련 Slack API 문서 링크를 제공하는 것이 좋습니다:
/** * Slack의 Section Block을 표현하는 레코드입니다. * Section Block은 텍스트 필드나 단일 텍스트를 포함할 수 있습니다. * * @see <a href="https://api.slack.com/reference/block-kit/blocks#section">Slack API: Section Block</a> */ public record SectionBlock(...)
1-20: 메서드 명명 규칙 통일 검토static factory 메서드의 명명 규칙이
newInstanceWithXXX형태인데, 일반적으로 Java에서는of,from,create등의 접두사를 많이 사용합니다. 프로젝트 내에서 일관된 메서드 명명 규칙을 사용하는지 확인하고, 필요하다면 아래와 같이 변경을 고려해보세요:- public static SectionBlock newInstanceWithFields(List<Text> fields) + public static SectionBlock withFields(List<Text> fields) - public static SectionBlock newInstanceWithText(Text text) + public static SectionBlock withText(Text text)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/main/java/org/sopt/makers/global/constant/SlackConstant.java(1 hunks)src/main/java/org/sopt/makers/service/SlackNotificationService.java(1 hunks)src/main/java/org/sopt/makers/vo/slack/SlackMessage.java(1 hunks)src/main/java/org/sopt/makers/vo/slack/block/ActionsBlock.java(1 hunks)src/main/java/org/sopt/makers/vo/slack/block/Block.java(1 hunks)src/main/java/org/sopt/makers/vo/slack/block/HeaderBlock.java(1 hunks)src/main/java/org/sopt/makers/vo/slack/block/SectionBlock.java(1 hunks)src/main/java/org/sopt/makers/vo/slack/element/Button.java(1 hunks)src/main/java/org/sopt/makers/vo/slack/element/Element.java(1 hunks)src/main/java/org/sopt/makers/vo/slack/text/Text.java(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- src/main/java/org/sopt/makers/vo/slack/element/Element.java
- src/main/java/org/sopt/makers/vo/slack/SlackMessage.java
- src/main/java/org/sopt/makers/vo/slack/text/Text.java
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/org/sopt/makers/global/constant/SlackConstant.java
- src/main/java/org/sopt/makers/service/SlackNotificationService.java
🔇 Additional comments (7)
src/main/java/org/sopt/makers/vo/slack/block/Block.java (1)
3-5: 잘 정의된 sealed 인터페이스입니다.sealed 인터페이스를 사용하여 구현 클래스를 HeaderBlock, SectionBlock, ActionsBlock으로 제한한 점이 좋습니다. 이를 통해 타입 안전성을 보장하고 Slack 메시지 블록의 종류를 명확하게 제한할 수 있습니다.
src/main/java/org/sopt/makers/vo/slack/block/ActionsBlock.java (2)
8-11: 잘 설계된 record 클래스입니다.Block 인터페이스를 구현하는 ActionsBlock record 클래스가 적절하게 설계되었습니다. Slack API 형식에 맞게 type과 elements 필드를 포함하고 있습니다.
12-14: 팩토리 메서드 패턴 적용이 좋습니다.newInstance 메서드를 통해 ActionsBlock 객체 생성을 캡슐화한 점이 좋습니다. 상수를 활용하여 코드 일관성을 유지하고 있습니다.
src/main/java/org/sopt/makers/vo/slack/block/HeaderBlock.java (2)
7-10: 잘 설계된 HeaderBlock record 클래스입니다.Block 인터페이스를 구현하는 HeaderBlock record 클래스가 적절하게 설계되었습니다. Slack API 형식에 맞게 type과 text 필드를 포함하고 있습니다.
11-13: 팩토리 메서드 패턴 적용이 좋습니다.newInstance 메서드를 통해 HeaderBlock 객체 생성을 캡슐화하고, Text 객체 생성까지 처리하여 사용자 편의성을 높인 점이 좋습니다. 상수를 활용하여 코드 일관성을 유지하고 있습니다.
src/main/java/org/sopt/makers/vo/slack/element/Button.java (2)
6-11: 잘 설계된 Button record 클래스입니다.Element 인터페이스를 구현하는 Button record 클래스가 적절하게 설계되었습니다. Slack API 형식에 맞게 필요한 필드들을 모두 포함하고 있습니다.
12-14: 팩토리 메서드 패턴 적용이 좋습니다.newInstance 메서드를 통해 Button 객체 생성을 캡슐화하고, Text 객체 생성까지 처리하여 사용자 편의성을 높인 점이 좋습니다. 상수를 활용하여 코드 일관성을 유지하고 있습니다.
…리팩토링 - 경로 및 쿼리 파라미터 파싱 책임을 WebhookRequest로 분리하여 관심사 분리
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
src/main/java/org/sopt/makers/dto/WebhookRequest.java (1)
10-16: Builder 사용이 과도할 수 있습니다
레코드(immutability) 특성상new WebhookRequest(stage, team, type, serviceType)로도 충분히 간결합니다. Builder를private로 열어두고 내부에서만 사용하는 현재 형태는 가독성 대비 이점이 크지 않습니다. 불필요한 Lombok 의존을 줄이고 싶다면 단순 생성자 호출로 전환을 검토해 주세요.src/main/java/org/sopt/makers/global/constant/SlackConstant.java (1)
6-7: 유틸리티 클래스용 Lombok 애너테이션 활용 제안
@NoArgsConstructor(access = AccessLevel.PRIVATE)+final class패턴 대신 Lombok 의@UtilityClass를 사용하면
- 자동으로
private생성자와static필드를 생성- 클래스명을 복수형(
SlackConstants) 으로 변경하는 권장 스타일 반영
할 수 있습니다.-@NoArgsConstructor(access = AccessLevel.PRIVATE) -public final class SlackConstant { +@UtilityClass +public class SlackConstants {src/main/java/org/sopt/makers/handler/SentryWebhookHandler.java (2)
90-97: JSON 파싱 예외 메시지 구체화 및null입력 대비
readTree실패 시 동일한INVALID_SLACK_PAYLOAD로 매핑하는 것은 좋으나,null과 JSON 구문 오류를 로그 구분 없이 같은 예외로 처리하고 있습니다. 로그에서 원인을 쉽게 파악하도록 메시지를 세분화하거나 Validation 단계에서 먼저null여부를 확인해 주세요.
179-183: 불변 헤더 캐싱으로 객체 생성 비용 절감 가능
매 요청마다HashMap을 새로 생성할 필요 없이, 불변Map을static final로 선언해 재사용하면 GC 부하를 줄일 수 있습니다.-private Map<String, String> createJsonContentTypeHeaders() { - Map<String, String> headers = new HashMap<>(); - headers.put(HEADER_CONTENT_TYPE, CONTENT_TYPE_JSON); - return headers; -} +private static final Map<String, String> JSON_HEADERS = + Map.of(HEADER_CONTENT_TYPE, CONTENT_TYPE_JSON); + +private Map<String, String> createJsonContentTypeHeaders() { + return JSON_HEADERS; +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/org/sopt/makers/dto/WebhookRequest.java(1 hunks)src/main/java/org/sopt/makers/global/constant/SlackConstant.java(1 hunks)src/main/java/org/sopt/makers/global/exception/message/ErrorMessage.java(1 hunks)src/main/java/org/sopt/makers/handler/SentryWebhookHandler.java(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/main/java/org/sopt/makers/handler/SentryWebhookHandler.java (1)
src/main/java/org/sopt/makers/global/exception/unchecked/InvalidSlackPayloadException.java (1)
InvalidSlackPayloadException(5-13)
🔇 Additional comments (1)
src/main/java/org/sopt/makers/global/exception/message/ErrorMessage.java (1)
12-24: 에러 코드 정의 일관성 확인 완료
HTTP 상태, 메시지, 코드가 명확히 구분되어 있으며 향후 확장성을 고려한 설계로 보입니다. 별다른 문제 없습니다.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/main/java/org/sopt/makers/service/SlackNotificationService.java (2)
82-93: Slack API 응답 검증이 개선되었습니다.이전 리뷰에서 지적된 사항이 잘 반영되어 HTTP 상태 코드와 응답 본문을 모두 확인하고 있습니다. 성공 시 적절한 로그를 남기는 것도 좋습니다.
151-156: 날짜 포맷팅 이슈가 해결되었습니다.이전 리뷰에서 지적된 ISO 날짜의 Z 오프셋 처리 문제가 OffsetDateTime을 사용하여 적절히 해결되었습니다. UTC에서 한국 시간으로의 변환도 정확하게 구현되어 있습니다.
🧹 Nitpick comments (1)
src/main/java/org/sopt/makers/service/SlackNotificationService.java (1)
51-60: 예외 처리가 잘 구현되었습니다.DateTimeException 발생 시 적절한 로깅과 함께 구체적인 예외 변환이 이루어지고 있습니다. 다만, 입력 파라미터에 대한 null 검증이 없어 NullPointerException이 발생할 가능성이 있습니다.
파라미터 유효성 검증을 추가하는 것이 좋겠습니다:
private SlackMessage createSlackMessage(String team, String type, String stage, SentryEventDetail sentryEventDetail) throws SentryCheckedException { + if (sentryEventDetail == null) { + log.error("Slack 메시지 생성 실패: sentryEventDetail이 null입니다."); + throw SlackMessageBuildException.from(ErrorMessage.SLACK_MESSAGE_BUILD_FAILED); + } try { return buildSlackMessage(team, type, stage, sentryEventDetail); } catch (DateTimeException e) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/org/sopt/makers/global/constant/SlackConstant.java(1 hunks)src/main/java/org/sopt/makers/global/util/HttpClientUtil.java(1 hunks)src/main/java/org/sopt/makers/service/SlackNotificationService.java(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/main/java/org/sopt/makers/global/constant/SlackConstant.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/org/sopt/makers/global/util/HttpClientUtil.java
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/main/java/org/sopt/makers/service/SlackNotificationService.java (3)
src/main/java/org/sopt/makers/global/exception/checked/SlackMessageBuildException.java (1)
SlackMessageBuildException(5-13)src/main/java/org/sopt/makers/global/exception/checked/SlackSendException.java (1)
SlackSendException(5-13)src/main/java/org/sopt/makers/service/NotificationServiceFactory.java (1)
Slf4j(13-38)
🔇 Additional comments (3)
src/main/java/org/sopt/makers/service/SlackNotificationService.java (3)
35-46: 잘 구조화된 클래스 설계입니다.NotificationService 인터페이스 구현과 ObjectMapper 의존성 주입 방식이 깔끔합니다. 싱글톤 패턴을 활용해 ObjectMapper 인스턴스를 효율적으로 관리하고 있습니다.
62-80: 예외 처리가 포괄적으로 구현되었습니다.다양한 종류의 예외를 적절히 처리하고 있으며, 특히 InterruptedException 처리 시 스레드 인터럽트 상태를 복원하는 부분이 좋습니다. 예외 발생 시 로그에 충분한 컨텍스트 정보를 포함하고 있습니다.
115-146: 블록 빌더 메서드가 잘 구성되어 있습니다.각 블록 타입별로 별도의 빌더 메서드를 통해 코드 가독성이 향상되었습니다. 상수 사용과 포맷팅 방식이 일관성 있게 적용되어 있습니다.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/main/java/org/sopt/makers/vo/slack/SlackMessage.java (3)
11-13: 팩토리 메서드 구현이 깔끔합니다.정적 팩토리 메서드를 사용하여 객체 생성을 캡슐화한 것은 좋은 패턴입니다. 하지만 파라미터에 대한 null 체크가 없어 NullPointerException이 발생할 수 있습니다.
public static SlackMessage newInstance(List<Block> blocks, String color) { + if (blocks == null) { + blocks = List.of(); + } + if (color == null) { + color = "#000000"; // 기본 색상 값 설정 + } return new SlackMessage(List.of(Attachment.of(color, blocks))); }
15-19: 중첩 레코드 클래스가 잘 구현되어 있습니다.
Attachment레코드와 팩토리 메서드가 잘 구현되어 있습니다. 여기서도 마찬가지로 null 체크를 추가하는 것이 안전할 것입니다.public static Attachment of(String color, List<Block> blocks) { + if (blocks == null) { + blocks = List.of(); + } + if (color == null) { + color = "#000000"; // 기본 색상 값 설정 + } return new Attachment(color, blocks); }
1-20: Javadoc 문서화를 추가하는 것이 좋겠습니다.코드의 가독성과 유지보수성을 위해 클래스와 메서드에 Javadoc 문서화를 추가하는 것을 권장합니다. 특히
color파라미터가 어떤 형식이어야 하는지(예: hex 코드) 명확히 하면 도움이 될 것입니다./** + * Slack API에 전송할 메시지 형식을 정의하는 레코드 클래스입니다. + * Slack Block Kit 형식을 따릅니다. + */ public record SlackMessage( List<Attachment> attachments ) { /** * 주어진 Block 목록과 색상으로 새로운 SlackMessage 인스턴스를 생성합니다. * * @param blocks 메시지에 포함될 Block 객체 목록 * @param color 메시지 왼쪽에 표시될 색상 (hex 코드 형식, 예: "#FF0000") * @return 새로운 SlackMessage 인스턴스 */ public static SlackMessage newInstance(List<Block> blocks, String color) { return new SlackMessage(List.of(Attachment.of(color, blocks))); } /** * 색상과 Block 목록을 포함하는 Attachment 레코드 클래스입니다. */ public record Attachment(String color, List<Block> blocks) { /** * 주어진 색상과 Block 목록으로 새로운 Attachment 인스턴스를 생성합니다. * * @param color 메시지 왼쪽에 표시될 색상 (hex 코드 형식) * @param blocks 첨부 파일에 포함될 Block 객체 목록 * @return 새로운 Attachment 인스턴스 */ public static Attachment of(String color, List<Block> blocks) { return new Attachment(color, blocks); } } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/org/sopt/makers/vo/slack/SlackMessage.java(1 hunks)
🔇 Additional comments (2)
src/main/java/org/sopt/makers/vo/slack/SlackMessage.java (2)
1-6: 패키지 구조와 임포트가 잘 정의되어 있습니다.패키지 이름과 필요한 클래스들이 잘 임포트되어 있습니다. 코드가 깔끔하게 정리되어 있습니다.
7-9:Details
❓ Verification inconclusive
Record 클래스의 사용이 적절합니다.
Java 16에서 도입된 Record 기능을 사용하여 불변 데이터 객체를 생성한 것은 좋은 접근 방식입니다. 다만, 프로젝트가 Java 16 이상으로 설정되어 있는지 확인하세요.
🏁 Script executed:
#!/bin/bash # Java 버전 확인하기 cat pom.xml | grep -A 3 "<java.version>"Length of output: 82
Record 클래스 사용 검토 및 Java 버전 확인 요청
레코드 사용은 적절하지만, Java 16 이상이 필요하므로 빌드 설정이 올바른지 직접 확인해 주세요.• Gradle을 사용하는 경우
–build.gradle또는build.gradle.kts파일 내의sourceCompatibility/targetCompatibility설정 확인
• Maven을 사용하는 경우
– 최상위 또는 모듈별pom.xml파일 내<java.version>또는<maven.compiler.source>/<maven.compiler.target>설정 확인
|
Tip For best results, initiate chat on the files or code changes. @hoonyworld PR 구현 내용 잘 봤습니다! Sentry 웹훅을 받아서 Slack으로 알림을 전송하는 기능을 잘 구현하셨네요. 특히 NotificationServiceFactory를 통한 서비스 타입 분리와 환경 변수 기반 URL 조회, 그리고 예외 처리 체계가 잘 설계되어 있는 것 같습니다. 코드 구조가 확장성을 고려해서 잘 설계되어 있어서, 나중에 Discord 같은 다른 서비스로도 쉽게 확장할 수 있을 것 같습니다. 시퀀스 다이어그램으로 흐름도 명확하게 표현해주셔서 이해하기 좋네요! 혹시 이 PR에 대해 추가적인 피드백이나 질문이 필요하신 부분이 있으신가요? |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/main/java/org/sopt/makers/global/util/HttpClientUtil.java (2)
39-47: 성공·실패 로깅 메시지 통일 및 응답 본문 포함 권장에러 로그에만
response.body()를 담고 있어 정상/비정상 상황에서 정보가 비대칭적입니다. 정상 응답도debug레벨로 기록하고, 오류 응답에서는 본문을 함께 남겨야 추후 Slack API 문서 대조가 용이합니다.
54-56: 불필요한Exceptioncatch는 제거마지막
catch (Exception e)블록은 앞선IOException,InterruptedException을 제외한 모든 런타임 예외를 삼켜 버립니다. 예상치 못한 버그를 조기에 드러내기 위해선 제거하고 상위로 전파하는 편이 좋습니다.src/main/java/org/sopt/makers/global/exception/message/ErrorMessage.java (1)
11-24: 에러 코드 관리 전략 명확화 필요
SLACK_SEND_FAILED(502)와SLACK_NETWORK_ERROR(503)처럼 HTTP 상태 코드를 직접 표기하고 있지만, 내부 로직에서 실제로 어떤 상황에 매핑되는지 주석이나 Javadoc이 없습니다. 팀 차원에서 “에러 코드 ↔ 실제 발생 조건” 표를 유지하지 않으면 중복·오용 위험이 큽니다. 간단한 설명이라도 각 enum 상수 위에 달아 두는 것을 권장합니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/org/sopt/makers/global/exception/message/ErrorMessage.java(1 hunks)src/main/java/org/sopt/makers/global/exception/unchecked/HttpRequestException.java(1 hunks)src/main/java/org/sopt/makers/global/util/HttpClientUtil.java(1 hunks)src/main/java/org/sopt/makers/service/SlackNotificationService.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/org/sopt/makers/service/SlackNotificationService.java
🧰 Additional context used
🧠 Learnings (1)
src/main/java/org/sopt/makers/global/util/HttpClientUtil.java (1)
Learnt from: hoonyworld
PR: sopt-makers/sentry-notifier#2
File: src/main/java/org/sopt/makers/global/util/HttpClientUtil.java:0-0
Timestamp: 2025-04-19T08:50:12.876Z
Learning: The HttpClientUtil class should throw a specific exception (HttpRequestException) that extends SentryUncheckedException rather than a generic Exception. It should also handle HTTP status codes (4xx, 5xx) appropriately, and include comprehensive logging for both success and failure cases.
🔇 Additional comments (1)
src/main/java/org/sopt/makers/global/util/HttpClientUtil.java (1)
41-44: HTTP 상태 코드별 세분화된 예외 코드 매핑 필요현재 구현은 4xx·5xx를 모두
SLACK_NETWORK_ERROR로 매핑하고 있습니다.
- 4xx(클라이언트 오류) →
SLACK_SEND_FAILED- 5xx(서버 오류) →
SLACK_NETWORK_ERROR또는 별도 코드처럼 구분하면 장애 원인 파악과 모니터링이 훨씬 수월해집니다.
- if (response.statusCode() >= 400) { - log.error("[HTTP 오류 응답] URL: {}, Status: {}", url, response.statusCode()); - throw HttpRequestException.from(ErrorMessage.SLACK_NETWORK_ERROR); - } + int status = response.statusCode(); + if (status >= 400 && status < 500) { + log.error("[HTTP 클라이언트 오류] URL: {}, Status: {}, Body: {}", url, status, response.body()); + throw HttpRequestException.from(ErrorMessage.SLACK_SEND_FAILED); + } else if (status >= 500) { + log.error("[HTTP 서버 오류] URL: {}, Status: {}, Body: {}", url, status, response.body()); + throw HttpRequestException.from(ErrorMessage.SLACK_NETWORK_ERROR); + }[ suggest_essential_refactor ]
src/main/java/org/sopt/makers/global/exception/unchecked/HttpRequestException.java
Show resolved
Hide resolved
1 participant
Related issue 🛠
Work Description ✏️
패키지 구조
주요 내용
구성 요소
1. Webhook 핸들러 (SentryWebhookHandler):
2. 알림 서비스 계층:
3. Slack 메시지 VO 객체:
4. 예외 처리 체계:
5. 유틸리티 클래스:
기술적 결정 사항
Trouble Shooting ⚽️