-
Notifications
You must be signed in to change notification settings - Fork 0
[feat] #111 디스코드 에러 알림 연동 #112
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
Walkthrough서버 5xx 응답을 감지하면 사용자 정보를 수집해 Discord 웹훅으로 비동기 전송하는 로깅 기능과 관련 DI/네트워크 구성을 추가합니다. 빌드 설정에 웹훅 URL을 추가하고, Retrofit 서비스·로거·인터셉터 통합을 포함합니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as HTTP Client
participant Interceptor as ErrorTrackingInterceptor
participant Logger as DiscordLogger
participant Repo as UserInfoRepository
participant Service as WebhookService
participant Discord as Discord API
Client->>Interceptor: 요청 → 응답(5xx)
rect rgb(200,220,240)
note over Interceptor: 5xx 감지 및 비동기 로깅 트리거
Interceptor->>Logger: logServerError(code, method, url)
end
rect rgb(220,240,200)
note over Logger: 사용자 정보 수집 및 메시지 생성
Logger->>Repo: getUserInfo()
Repo-->>Logger: nickname, deviceId
Logger->>Logger: DiscordLogBody 생성
end
rect rgb(240,220,200)
note over Logger,Service: 웹훅 전송
Logger->>Service: sendLog(webhookUrl, body)
Service->>Discord: POST webhookUrl
Discord-->>Service: 200 OK (또는 오류)
end
예상 코드 리뷰 규모🎯 3 (중간) | ⏱️ ~25분 개요이 PR은 서버 에러를 Discord 웹훅으로 추적할 수 있도록 Discord 알림 통합 기능을 추가합니다. 새로운 웹훅 서비스, 사용자 정보를 수집하고 메시지를 포맷하는 로거, 기존 에러 추적 인터셉터로의 통합, 그리고 필요한 의존성 주입 설정을 포함합니다. 변경사항
시퀀스 다이어그램sequenceDiagram
participant Client as HTTP Client
participant Interceptor as ErrorTrackingInterceptor
participant Logger as DiscordLogger
participant Repo as UserInfoRepository
participant Service as WebhookService
participant Discord as Discord API
Client->>Interceptor: HTTP 5xx Response
rect rgb(200, 220, 240)
note over Interceptor: 에러 감지
Interceptor->>Logger: logServerError(code, method, url)
end
rect rgb(220, 240, 200)
note over Logger: 정보 수집 및 포매팅
Logger->>Repo: getUserInfo()
Repo-->>Logger: nickname, deviceId
Logger->>Logger: DiscordLogBody 생성
end
rect rgb(240, 220, 200)
note over Service: 웹훅 전송
Logger->>Service: sendLog(webhookUrl, body)
Service->>Discord: POST webhookUrl
Discord-->>Service: 200 OK
end
예상 코드 리뷰 규모🎯 3 (중간) | ⏱️ ~25분 시
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
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 |
There was a problem hiding this 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
🧹 Nitpick comments (3)
app/src/main/java/com/kuit/findu/data/dataremote/util/ErrorTrackingInterceptor.kt (1)
44-52: 비구조화된 코루틴 스코프 사용 검토 필요
CoroutineScope(Dispatchers.IO).launch는 비구조화된 스코프를 생성하여 코루틴 누수 가능성이 있습니다. 로깅이 실패하더라도 주요 기능에는 영향이 없지만, 앱 생명주기와 연결된 스코프 사용을 고려해보세요.🔎 개선 방안
Option 1:
GlobalScope명시적 사용 (로깅용으로 적합):- CoroutineScope(Dispatchers.IO).launch { + @OptIn(DelicateCoroutinesApi::class) + GlobalScope.launch(Dispatchers.IO) { discordLogger.logServerError(Option 2: Application 스코프 주입 (더 권장):
// ErrorTrackingInterceptor에 CoroutineScope 주입 class ErrorTrackingInterceptor @Inject constructor( private val firebaseCrashlytics: FirebaseCrashlytics, private val discordLogger: DiscordLogger, @ApplicationScope private val appScope: CoroutineScope, ) : Interceptor { // ... appScope.launch(Dispatchers.IO) { discordLogger.logServerError(...) } }app/src/main/java/com/kuit/findu/data/dataremote/util/DiscordLogger.kt (1)
14-40: Discord 로깅 실패 시 가시성 부족
runCatching으로 에러를 조용히 처리하고 있어, Discord 웹훅 전송 실패 시 이를 감지할 방법이 없습니다. 로깅 실패 자체는 주요 기능에 영향을 주지 않지만, 디버깅이나 모니터링 목적으로 실패를 추적하는 것도 고려해볼 만합니다.🔎 개선 방안
runCatching { webhookService.sendLog(webhookUrl, body) + }.onFailure { error -> + // Logcat에 기록하거나 Firebase Analytics로 추적 + Log.w("DiscordLogger", "Failed to send Discord log", error) }app/src/main/java/com/kuit/findu/di/LogNetworkModule.kt (1)
27-34: 하드코딩된 Discord 베이스 URLDiscord 베이스 URL이 하드코딩되어 있습니다. Discord API는 안정적이지만, 향후 변경 가능성이나 테스트 환경을 고려하면 설정으로 분리하는 것도 고려해볼 수 있습니다. 다만 현재로서는 큰 문제는 아닙니다.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
app/build.gradle.ktsapp/src/main/java/com/kuit/findu/data/dataremote/service/WebhookService.ktapp/src/main/java/com/kuit/findu/data/dataremote/util/DiscordLogger.ktapp/src/main/java/com/kuit/findu/data/dataremote/util/ErrorTrackingInterceptor.ktapp/src/main/java/com/kuit/findu/di/LogModule.ktapp/src/main/java/com/kuit/findu/di/LogNetworkModule.ktapp/src/main/java/com/kuit/findu/di/NetworkModule.ktapp/src/main/java/com/kuit/findu/domain/model/DiscordLogBody.kt
⏰ 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: ci-build
🔇 Additional comments (6)
app/src/main/java/com/kuit/findu/domain/model/DiscordLogBody.kt (1)
3-5: LGTM!Discord 웹훅 페이로드를 위한 깔끔한 데이터 클래스입니다.
app/src/main/java/com/kuit/findu/di/LogModule.kt (1)
13-27: LGTM!DI 설정이 올바르게 구성되어 있고, 싱글톤 스코프도 적절합니다.
app/src/main/java/com/kuit/findu/di/NetworkModule.kt (1)
87-95: LGTM!DiscordLogger 의존성이 올바르게 주입되었고, named parameter 사용으로 가독성도 좋습니다.
app/src/main/java/com/kuit/findu/data/dataremote/service/WebhookService.kt (1)
8-14: LGTM!동적 웹훅 URL 사용을 위한
@Url어노테이션 활용이 적절합니다. 에러 처리는 호출부(DiscordLogger)에서runCatching으로 처리하고 있어 문제없습니다.app/src/main/java/com/kuit/findu/data/dataremote/util/ErrorTrackingInterceptor.kt (1)
32-36: LGTM!로컬 변수
code를 사용하도록 개선되어 일관성이 향상되었습니다.app/src/main/java/com/kuit/findu/di/LogNetworkModule.kt (1)
18-42: 웹훅 전용 네트워크 스택 분리 - 좋은 설계메인 API와 웹훅 로깅을 별도의 OkHttpClient/Retrofit으로 분리한 것은 좋은 아키텍처 결정입니다. 웹훅 실패가 메인 API에 영향을 주지 않고, 각각 독립적으로 설정 가능합니다.
| buildConfigField("String", "NAVER_CLIENT_SECRET", properties["NAVER_CLIENT_SECRET"].toString()) | ||
| buildConfigField("String", "KAKAO_NATIVE_APP_KEY", properties["kakao.native.app.key"].toString()) | ||
| manifestPlaceholders["KAKAO_NATIVE_APP_KEY_MANIFEST"] = properties["kakao.native.app.key.manifest"].toString() | ||
| buildConfigField("String", "DISCORD_WEBHOOK_URL", properties["DISCORD_WEBHOOK_URL"].toString()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
속성 누락 시 빌드 실패 가능성 확인 필요
local.properties에 DISCORD_WEBHOOK_URL이 없을 경우 빌드가 실패할 수 있습니다. 다른 개발자나 CI/CD 환경에서 문제가 발생할 수 있으니, 속성 존재 여부 확인 후 기본값 제공 또는 명확한 에러 메시지 추가를 고려해보세요.
🔎 제안하는 안전한 처리 방법
- buildConfigField("String", "DISCORD_WEBHOOK_URL", properties["DISCORD_WEBHOOK_URL"].toString())
+ buildConfigField(
+ "String",
+ "DISCORD_WEBHOOK_URL",
+ properties.getProperty("DISCORD_WEBHOOK_URL")
+ ?: throw GradleException("DISCORD_WEBHOOK_URL must be defined in local.properties")
+ )또는 개발 환경에서 빈 문자열을 기본값으로 사용:
- buildConfigField("String", "DISCORD_WEBHOOK_URL", properties["DISCORD_WEBHOOK_URL"].toString())
+ buildConfigField(
+ "String",
+ "DISCORD_WEBHOOK_URL",
+ properties.getProperty("DISCORD_WEBHOOK_URL", "\"\"")
+ )📝 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.
| buildConfigField("String", "DISCORD_WEBHOOK_URL", properties["DISCORD_WEBHOOK_URL"].toString()) | |
| buildConfigField( | |
| "String", | |
| "DISCORD_WEBHOOK_URL", | |
| properties.getProperty("DISCORD_WEBHOOK_URL") | |
| ?: throw GradleException("DISCORD_WEBHOOK_URL must be defined in local.properties") | |
| ) |
🤖 Prompt for AI Agents
In app/build.gradle.kts around line 38, the buildConfigField uses
properties["DISCORD_WEBHOOK_URL"] directly which can throw or fail the build
when the property is missing; update the script to first check for the property
key and either supply a safe default (e.g., empty string) or fail fast with a
clear error message indicating the missing property and how to set it
(local.properties or CI secret). Ensure the check happens before calling
toString(), and choose one approach (default empty value for dev builds or an
explicit Gradle exception with instructions for CI) so builds behave
predictably.
ikseong00
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
좋습니다! 고생하셨어요!
Related issue 🛠
Work Description 📝
Screenshot 📸
Uncompleted Tasks 😅
N/A
To Reviewers 📢
Summary by CodeRabbit
릴리스 노트
✏️ Tip: You can customize this high-level summary in your review settings.