refactor: IllegalArgumentException을 CustomSlackException으로 변경 #749
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
동일한 이모지로 여러 사용자를 호출할 수 있도록 기능을 확장했습니다. 주요 변경사항: - MakersUserSlackRepository: 사용자별 이모지 조회/삭제 메서드 추가 - MessageContext: 단일 사용자에서 다중 사용자 리스트로 변경 - SlackMessageService: 다중 사용자 멘션 처리 로직 구현 - SlackUtils: 다중 사용자 멘션 포매팅 유틸리티 메서드 추가 - DTO: userSlackId 파라미터 추가로 사용자별 관리 지원 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Slack 패키지 내 예외 처리를 통일하기 위해 CustomSlackException 도입 - IllegalArgumentException을 CustomSlackException으로 전면 교체 - 로그 레벨을 error에서 warn으로 변경하여 적절한 심각도 반영 - 예외 처리 일관성 개선으로 유지보수성 향상 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
|
Summary by CodeRabbit
WalkthroughSlack 관련 오류 처리 일관화를 위해 CustomSlackException을 신설하고 SlackMessageService 전반에서 IllegalArgumentException을 대체했습니다. Slack 이벤트 리스너 및 소켓 모드 스타터의 예외 로그 레벨을 error에서 warn으로 조정했습니다. sendMention 경로의 예외 캐치 범위를 확장하고 실패 시 CustomSlackException을 던지도록 변경했습니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller as Service/Controller
participant SMS as SlackMessageService
participant Slack as Slack API
Caller->>SMS: sendMention(request)
rect rgba(200,230,255,0.3)
note right of SMS: 메시지 빌드 및 유저/템플릿 해석
SMS->>SMS: resolveSlackUser()/extractTemplateCd()
SMS->>Slack: chat.postMessage(payload)
alt Post 성공
Slack-->>SMS: ok
SMS-->>Caller: success
else Post 실패
Slack-->>SMS: error
SMS->>SMS: throw CustomSlackException
SMS-->>Caller: CustomSlackException 전파
end
end
rect rgba(255,240,200,0.3)
note over SMS: 예외 처리 (경고 로그)
Caller->>Caller: 필요시 상위에서 처리
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
main/src/main/java/org/sopt/makers/crew/main/slack/SlackMessageService.java (1)
63-86: 예외 처리 로직의 심각한 문제: 검증 예외가 숨겨집니다.현재 구조에서는
resolveSlackUser와messageBuild에서 발생하는 CustomSlackException(검증 오류)을 catch 블록에서 잡아서 경고 로그만 남기고 삼켜버립니다. 이는 다음과 같은 문제를 야기합니다:
- 검증 오류가 숨겨짐: 잘못된 이모지나 템플릿 누락 같은 설정 오류가 조용히 실패
- 불일치: 다른 메서드들(insertEvent, updateEvent 등)은 CustomSlackException을 던지는데, 이 메서드만 catch해서 삼킴
- 디버깅 어려움: warn 레벨 로그로는 근본 원인 파악이 어려움
다음과 같이 수정하여 검증 예외는 전파하고 Slack API 예외만 처리하세요:
public void sendMention(MethodsClient client, ReactionAddedEvent event) { String channel = event.getItem().getChannel(); String user = event.getUser(); - try { - List<MakersUserSlack> slackUser = resolveSlackUser(event); + List<MakersUserSlack> slackUser = resolveSlackUser(event); + String sendMessage = messageBuild(slackUser, user); - String sendMessage = messageBuild(slackUser, user); + try { ChatPostMessageResponse responses = client.chatPostMessage(r -> r .channel(channel) .text(sendMessage) .threadTs(event.getItem().getTs())); if (responses.isOk()) log.info("message send success"); else { - throw new CustomSlackException("ERROR OCCURED"); + log.error("Failed to send Slack message: {}", responses.getError()); + throw new CustomSlackException("Failed to send Slack message: " + responses.getError()); } - } catch (CustomSlackException | SlackApiException | IOException e) { - log.warn("now error : {}", e); - log.warn("message send failed"); + } catch (SlackApiException | IOException e) { + log.error("Slack API error occurred", e); + throw new CustomSlackException("Failed to send Slack message", e); } }이렇게 하면:
- 검증 오류는 호출자에게 전파되어 적절히 처리됨
- Slack API 관련 예외만 catch하여 CustomSlackException으로 래핑
- 원인 예외(cause)를 보존하여 디버깅 가능
- error 레벨 로깅으로 중요한 문제 감지 가능
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
main/src/main/java/org/sopt/makers/crew/main/global/exception/CustomSlackException.java(1 hunks)main/src/main/java/org/sopt/makers/crew/main/slack/SlackEventListener.java(1 hunks)main/src/main/java/org/sopt/makers/crew/main/slack/SlackMessageService.java(9 hunks)main/src/main/java/org/sopt/makers/crew/main/slack/SlackSocketModeStarter.java(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
main/src/main/java/org/sopt/makers/crew/main/slack/SlackMessageService.java (1)
main/src/main/java/org/sopt/makers/crew/main/global/exception/CustomSlackException.java (1)
CustomSlackException(3-11)
⏰ 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 and Test (SpringBoot)
🔇 Additional comments (8)
main/src/main/java/org/sopt/makers/crew/main/slack/SlackEventListener.java (1)
34-36: 로그 레벨 다운그레이드의 적절성을 재검토해 주세요.Slack 리액션 이벤트 처리 실패를 경고(warn)로 처리하는 것이 적절한지 확인이 필요합니다. 이벤트 처리 실패가 시스템 문제를 나타낼 수 있고, 사용자 경험에 영향을 줄 수 있다면 error 레벨 유지가 더 적절할 수 있습니다.
다음 사항을 고려해 검토해 주세요:
- 이벤트 처리 실패가 사용자에게 미치는 영향
- 모니터링/알림 시스템에서 warn vs error의 구분 기준
- 실패 빈도 및 복구 가능성
main/src/main/java/org/sopt/makers/crew/main/slack/SlackSocketModeStarter.java (1)
51-53: 종료 시 예외 처리는 적절합니다.애플리케이션 종료 과정에서의 소켓 모드 정리 실패는 warn 레벨이 적절합니다. 이미 예외 정보도 함께 로깅하고 있어 좋습니다.
main/src/main/java/org/sopt/makers/crew/main/slack/SlackMessageService.java (6)
40-40: LGTM!IllegalArgumentException을 CustomSlackException으로 교체한 것은 명확한 의도를 전달합니다.
49-50: LGTM!예외 타입 변경이 적절합니다.
58-58: LGTM!예외 타입 변경이 적절합니다.
93-95: LGTM!예외 타입 변경이 적절합니다. 검증 오류를 명확히 전달합니다.
102-104: LGTM!예외 타입 변경이 적절합니다.
114-123: LGTM!예외 타입 변경이 적절합니다.
main/src/main/java/org/sopt/makers/crew/main/global/exception/CustomSlackException.java
Show resolved
Hide resolved
main/src/main/java/org/sopt/makers/crew/main/slack/SlackSocketModeStarter.java
Show resolved
Hide resolved
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
👩💻 Contents
IllegalArgumentException을 CustomSlackException으로 변경
📝 Review Note
📣 Related Issue
✅ 점검사항