Conversation
|
Note
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Description check | PR 설명에서 주요 변경사항(작업 내용) 섹션이 미작성 상태로 남아있습니다. 관련 이슈 링크만 작성되어 있고 구체적인 구현 내용이 빈칸으로 방치되어 있습니다. | 작업 내용과 주요 변경사항 섹션을 작성해주세요. 예를 들어, Firebase 로깅 브릿지 구현, LoggerImpl 클래스 추가, DI 모듈 구성 변경 등을 기술하면 됩니다. | |
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | 제목이 PR의 주요 변경사항을 명확하게 반영하고 있습니다. iOS 네이티브 로거 구현이라는 핵심 내용을 간결하게 전달하고 있습니다. |
| Linked Issues check | ✅ Passed | 코드 변경사항(FirebaseLoggingBridge 인터페이스, LoggerImpl 구현, DI 모듈 재구성 등)이 iOS 네이티브 로거 구현이라는 이슈 #1182의 목표와 기술적으로 일치합니다. |
| Out of Scope Changes check | ✅ Passed | 모든 변경사항이 iOS 로거 네이티브 구현이라는 명확한 범위 내에 있습니다. 로거 인터페이스 정의, Swift 구현, Kotlin DI 연결 등이 모두 해당 목표를 지원하고 있습니다. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
- 📝 Generate docstrings (stacked PR)
- 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
- Commit unit tests in branch
feat/1182
📝 Coding Plan
- Generate coding plan for human review comments
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 help to get the list of available commands and usage tips.
Tip
CodeRabbit can enforce grammar and style rules using `languagetool`.
Configure the reviews.tools.languagetool setting to enable/disable rules and categories. Refer to the LanguageTool Community to learn more.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@mobile/iosApp/iosApp/FirebaseLoggingBridgeImpl.swift`:
- Around line 6-12: logEvent 함수에서 userId를 평문으로 이벤트 파라미터로 전송하는 문제를 해결하세요:
logEvent(eventName: String, level: String, message: String, userId: String?) 내에서
userId를 그대로 params에 넣지 말고(1) 완전히 제거하거나, (2) 전송 전 해시/비식별화 처리(예: 안전한 해시 함수 또는 난수
매핑)하여 params["user_id"]에 넣거나, 또는 (3) Firebase의 의도적 분리 방식으로
Analytics.setUserId(...)를 사용하도록 변경해 message 마스킹(SensitiveInfoSanitizer) 정책과
일치시키세요; 선택한 접근법을 적용한 뒤 Analytics.logEvent(eventName, parameters: params) 호출과
userId 처리 로직을 일관되게 갱신하세요.
In `@mobile/shared/src/iosMain/kotlin/com/mulkkam/data/logger/LoggerImpl.kt`:
- Around line 21-35: The log implementation currently overwrites per-entry user
context by always using the logger's userId; update LoggerImpl.log to prefer the
entry's userId when present (use entry.userId ?: this.userId) for both the
formatted message (formatMessage call) and the firebaseBridge.logEvent call
(event payload userId), preserving per-call user context while falling back to
the logger-level user; keep sanitization and isDebug behavior unchanged.
- Around line 51-62: LoggerImpl.logToNSLog currently passes a concatenated
string into NSLog which is treated as a format string and can misinterpret `%`
tokens in LogEntry.message; change the call so the format string is constant and
message is supplied as an argument (e.g. use a constant format like
"[$DEFAULT_LOGGING_TAG][%s] %@" or split into separate arguments) or otherwise
structure the log fields (tag, prefix, message) so NSLog never interprets user
data as a format string; update the NSLog invocation in LoggerImpl.logToNSLog
and ensure DEFAULT_LOGGING_TAG / LogLevel prefix remain constants while
LogEntry.message is passed as a parameter to avoid format-string injection.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: af388189-97f4-4e47-a600-1fa0296d566c
📒 Files selected for processing (8)
mobile/iosApp/iosApp/FirebaseLoggingBridgeImpl.swiftmobile/iosApp/iosApp/MulKkamApp.swiftmobile/shared/src/commonMain/kotlin/com/mulkkam/domain/logger/FirebaseLoggingBridge.ktmobile/shared/src/iosMain/kotlin/com/mulkkam/data/logger/LoggerImpl.ktmobile/shared/src/iosMain/kotlin/com/mulkkam/di/Helper.ktmobile/shared/src/iosMain/kotlin/com/mulkkam/di/IosSharedModule.ktmobile/shared/src/iosMain/kotlin/com/mulkkam/di/LoggerModule.ktmobile/shared/src/iosMain/kotlin/com/mulkkam/di/LoggingModule.kt
💤 Files with no reviewable changes (1)
- mobile/shared/src/iosMain/kotlin/com/mulkkam/di/LoggerModule.kt
| func logEvent(eventName: String, level: String, message: String, userId: String?) { | ||
| var params: [String: Any] = [ | ||
| "level": level, | ||
| "message": message, | ||
| ] | ||
| if let userId { params["user_id"] = userId } | ||
| Analytics.logEvent(eventName, parameters: params) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# 파일 크기 확인 및 전체 내용 읽기
wc -l mobile/iosApp/iosApp/FirebaseLoggingBridgeImpl.swift && cat -n mobile/iosApp/iosApp/FirebaseLoggingBridgeImpl.swiftRepository: woowacourse-teams/2025-mul-kkam
Length of output: 958
🏁 Script executed:
# 공유 logger 찾기
fd -t f "Logger\|logger" mobile/iosApp --type f | head -20Repository: woowacourse-teams/2025-mul-kkam
Length of output: 57
🏁 Script executed:
# shared logger나 message sanitization 패턴 검색
rg -i "sanitize|sanitiz|sanitise" mobile/iosApp --type swift -A 3Repository: woowacourse-teams/2025-mul-kkam
Length of output: 57
🏁 Script executed:
# userId 관련 사용 패턴 확인
rg "userId|user_id" mobile/iosApp --type swift -B 2 -A 2Repository: woowacourse-teams/2025-mul-kkam
Length of output: 975
🏁 Script executed:
# FirebaseLoggingBridge 인터페이스 정의 찾기
rg "FirebaseLoggingBridge" --type swift --type kotlin -A 5Repository: woowacourse-teams/2025-mul-kkam
Length of output: 6347
🏁 Script executed:
# shared 모듈의 logger 관련 코드 찾기
fd -t f "Log\|logger" shared/commonMain --type f | head -20Repository: woowacourse-teams/2025-mul-kkam
Length of output: 179
🏁 Script executed:
# 공유 모듈에서 message handling 패턴 확인
rg "message|param|sanitize" shared/commonMain --type kotlin -A 2 | head -50Repository: woowacourse-teams/2025-mul-kkam
Length of output: 132
🏁 Script executed:
# userId 파라미터 사용 패턴 확인
rg "userId|user_id" shared/commonMain --type kotlin -B 2 -A 2Repository: woowacourse-teams/2025-mul-kkam
Length of output: 132
🏁 Script executed:
# FirebaseLoggingBridge 전체 정의 보기
cat -n mobile/shared/src/commonMain/kotlin/com/mulkkam/domain/logger/FirebaseLoggingBridge.ktRepository: woowacourse-teams/2025-mul-kkam
Length of output: 395
🏁 Script executed:
# LoggerImpl 파일 크기 확인 후 내용 읽기
wc -l mobile/shared/src/iosMain/kotlin/com/mulkkam/data/logger/LoggerImpl.kt && cat -n mobile/shared/src/iosMain/kotlin/com/mulkkam/data/logger/LoggerImpl.ktRepository: woowacourse-teams/2025-mul-kkam
Length of output: 2673
🏁 Script executed:
# SensitiveInfoSanitizerImpl 찾기 및 읽기
fd "SensitiveInfoSanitizerImpl" && wc -l mobile/shared/src/commonMain/kotlin/com/mulkkam/data/logger/SensitiveInfoSanitizerImpl.kt && cat -n mobile/shared/src/commonMain/kotlin/com/mulkkam/data/logger/SensitiveInfoSanitizerImpl.ktRepository: woowacourse-teams/2025-mul-kkam
Length of output: 2933
🏁 Script executed:
# userId 사용 패턴 확인 - 다른 로거 구현에서는 어떻게 처리하는지
fd "Logger" mobile/shared --type f | grep -E "Impl|Test" | head -10Repository: woowacourse-teams/2025-mul-kkam
Length of output: 220
🏁 Script executed:
# Android 로거 구현 확인
find mobile/shared -name "*Logger*Impl*" -type f | head -5Repository: woowacourse-teams/2025-mul-kkam
Length of output: 220
🏁 Script executed:
# Android LoggerImpl 확인
cat -n mobile/shared/src/androidMain/kotlin/com/mulkkam/data/logger/LoggerImpl.ktRepository: woowacourse-teams/2025-mul-kkam
Length of output: 3530
🏁 Script executed:
# Android의 FirebaseLoggingBridge 구현 찾기
fd "FirebaseLoggingBridge" mobile --type f | grep -i androidRepository: woowacourse-teams/2025-mul-kkam
Length of output: 57
🏁 Script executed:
# Android 앱의 Firebase 구현
find mobile/androidApp -name "*Firebase*" -o -name "*Logging*" | grep -i bridgeRepository: woowacourse-teams/2025-mul-kkam
Length of output: 57
이벤트 파라미터로 전송되는 userId 보호가 필요합니다.
공유 로거는 message를 SensitiveInfoSanitizer로 마스킹하지만, userId는 iOS(Line 11)와 Android 모두 동일하게 Firebase Analytics에 그대로 전송하고 있습니다. 현재 userId가 내부 식별자라도, 향후 이메일·전화번호·외부 계정 키로 변경되면 모든 릴리즈 이벤트에 직접 식별 가능한 개인정보가 노출되는 구조입니다.
선택지는 다음 세 가지입니다:
-
이벤트 파라미터에서 제거: 가장 간단하고 정책 리스크가 없습니다. userId 추적이 불필요한 이벤트 분석 대부분이 이 방식입니다.
-
비식별화 처리 후 전송: userId를 해시하거나 난수화하면 운영상 추적성을 유지하면서도 노출 범위를 제한할 수 있습니다. 사용자별 유입 경로 분석 같은 용도에 적합합니다.
-
Firebase User ID API로 역할 분리:
setUserId()메서드로 사용자 식별을 명시적으로 분리하면 의도가 명확하지만, 플랫폼 정책 검토가 필요할 수 있습니다.
현재 형태는 메시지 보호와의 불일치로 인한 보안 부채입니다. message 마스킹과의 일관성을 맞추거나, userId 처리 정책을 명확히 정의해야 합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mobile/iosApp/iosApp/FirebaseLoggingBridgeImpl.swift` around lines 6 - 12,
logEvent 함수에서 userId를 평문으로 이벤트 파라미터로 전송하는 문제를 해결하세요: logEvent(eventName: String,
level: String, message: String, userId: String?) 내에서 userId를 그대로 params에 넣지
말고(1) 완전히 제거하거나, (2) 전송 전 해시/비식별화 처리(예: 안전한 해시 함수 또는 난수 매핑)하여 params["user_id"]에
넣거나, 또는 (3) Firebase의 의도적 분리 방식으로 Analytics.setUserId(...)를 사용하도록 변경해 message
마스킹(SensitiveInfoSanitizer) 정책과 일치시키세요; 선택한 접근법을 적용한 뒤
Analytics.logEvent(eventName, parameters: params) 호출과 userId 처리 로직을 일관되게 갱신하세요.
| override fun log(entry: LogEntry) { | ||
| val rawMessage = formatMessage(entry.copy(userId = userId)) | ||
| val safeMessage = sanitizer.sanitize(rawMessage) | ||
| val safePayloadMessage = sanitizer.sanitize(entry.message) | ||
|
|
||
| logToNSLog(entry.level, safeMessage) | ||
|
|
||
| if (isDebug) return | ||
|
|
||
| firebaseBridge.logEvent( | ||
| eventName = entry.event.name, | ||
| level = entry.level.name, | ||
| message = safePayloadMessage, | ||
| userId = userId, | ||
| ) |
There was a problem hiding this comment.
LogEntry.userId를 항상 덮어써서 엔트리별 사용자 컨텍스트가 사라집니다.
Line 22와 Line 34가 필드 userId만 사용해서, 호출자가 LogEntry(userId = ...)를 넘겨도 그대로 반영되지 않습니다. 그래서 초기화 전 로그는 null로 떨어지고, 특정 사용자 컨텍스트를 명시한 로그도 현재 로거 상태로 오염됩니다. 선택지는 두 가지입니다.
entry.userId ?: this.userId로 엔트리 값을 우선: 기존 모델을 가장 덜 깨고 의도도 자연스럽습니다.- 전역 사용자만 허용: 대신
LogEntry.userId가 무시된다는 사실을 API에 드러내거나 필드를 정리하는 편이 맞습니다.
지금 모델을 유지할 거라면 첫 번째 쪽이 더 안전해 보입니다.
🛠️ 제안하는 수정안
override fun log(entry: LogEntry) {
- val rawMessage = formatMessage(entry.copy(userId = userId))
+ val resolvedUserId = entry.userId ?: userId
+ val rawMessage = formatMessage(entry.copy(userId = resolvedUserId))
val safeMessage = sanitizer.sanitize(rawMessage)
val safePayloadMessage = sanitizer.sanitize(entry.message)
logToNSLog(entry.level, safeMessage)
if (isDebug) return
firebaseBridge.logEvent(
eventName = entry.event.name,
level = entry.level.name,
message = safePayloadMessage,
- userId = userId,
+ userId = resolvedUserId,
)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mobile/shared/src/iosMain/kotlin/com/mulkkam/data/logger/LoggerImpl.kt`
around lines 21 - 35, The log implementation currently overwrites per-entry user
context by always using the logger's userId; update LoggerImpl.log to prefer the
entry's userId when present (use entry.userId ?: this.userId) for both the
formatted message (formatMessage call) and the firebaseBridge.logEvent call
(event payload userId), preserving per-call user context while falling back to
the logger-level user; keep sanitization and isDebug behavior unchanged.
| private fun logToNSLog( | ||
| level: LogLevel, | ||
| message: String, | ||
| ) { | ||
| val prefix = | ||
| when (level) { | ||
| LogLevel.ERROR -> "❌ ERROR" | ||
| LogLevel.WARN -> "⚠️ WARN" | ||
| LogLevel.INFO -> "ℹ️ INFO" | ||
| LogLevel.DEBUG -> "🐛 DEBUG" | ||
| } | ||
| NSLog("[$DEFAULT_LOGGING_TAG][$prefix] $message") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "LoggerImpl.kt" | head -5Repository: woowacourse-teams/2025-mul-kkam
Length of output: 224
🏁 Script executed:
cat -n ./mobile/shared/src/iosMain/kotlin/com/mulkkam/data/logger/LoggerImpl.ktRepository: woowacourse-teams/2025-mul-kkam
Length of output: 2599
🏁 Script executed:
find . -type f -name "*Sanitizer*" | grep -i logRepository: woowacourse-teams/2025-mul-kkam
Length of output: 246
🏁 Script executed:
cat -n ./mobile/shared/src/commonMain/kotlin/com/mulkkam/domain/logger/LogSanitizer.ktRepository: woowacourse-teams/2025-mul-kkam
Length of output: 278
🏁 Script executed:
cat -n ./mobile/shared/src/commonMain/kotlin/com/mulkkam/data/logger/SensitiveInfoSanitizerImpl.ktRepository: woowacourse-teams/2025-mul-kkam
Length of output: 2750
🏁 Script executed:
# Check if there are any tests or examples showing how user messages might look
rg "LogEntry" --type kotlin -A 3 | head -30Repository: woowacourse-teams/2025-mul-kkam
Length of output: 2635
NSLog에 동적 문자열을 포맷 문자열로 직접 전달하지 마세요.
62번 라인에서 조합된 메시지 전체를 NSLog의 첫 번째 인자로 전달하고 있습니다. NSLog는 첫 번째 인자를 포맷 문자열로 해석하기 때문에, 사용자 입력이나 외부 데이터에 포함된 %@, %d, %% 같은 토큰이 있으면 의도하지 않은 포맷 지정자로 처리됩니다. 현재 코드에서 message는 LogEntry.message를 거쳐오는데, SensitiveInfoSanitizer는 민감 정보 마스킹만 수행하고 포맷 문자열 특수문자를 제거하지 않습니다.
문제 상황:
- 사용자가 실수로 또는 의도적으로 메시지에
%s,%x등을 포함하면 NSLog가 스택 메모리에 접근하려 시도 - 예:
"가격: %d원"과 같은 메시지는 런타임에 예기치 않은 값을 출력하거나 앱 크래시 유발
개선 방안:
-
상수 포맷 문자열 사용 (권장) - 가장 간단하고 안전
-NSLog("[$DEFAULT_LOGGING_TAG][$prefix] $message") +NSLog("%@", "[$DEFAULT_LOGGING_TAG][$prefix] $message")
장점: 별도 로직 추가 없음, 명확한 의도 표현
단점: 구조화된 정보 검색이 어려울 수 있음 -
구조화된 포맷으로 분리 (권장) - 더 안전하고 파싱 가능
-NSLog("[$DEFAULT_LOGGING_TAG][$prefix] $message") +NSLog("%@ %@ %@", DEFAULT_LOGGING_TAG, prefix, message)
장점: 각 부분을 명확히 분리, 로그 파싱 용이, 포맷 문자열 안전성 보장
단점: 약간 더 verbose -
포맷 문자열 제거 필터 추가 - 과도할 수 있음
-NSLog("[$DEFAULT_LOGGING_TAG][$prefix] $message") +val safePath = message.replace("%".toRegex(), "%%") +NSLog("%@", "[$DEFAULT_LOGGING_TAG][$prefix] $safePath")
장점: 기존 출력 형식 유지
단점: 추가 전처리 필요,%%로 이스케이프하면 출력에%표시됨
방안 2번(구조화된 포맷)이 가장 권장되는데, iOS 로깅 관례상 각 정보를 명확히 구분하는 것이 디버깅 효율을 높이기 때문입니다.
📝 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.
| private fun logToNSLog( | |
| level: LogLevel, | |
| message: String, | |
| ) { | |
| val prefix = | |
| when (level) { | |
| LogLevel.ERROR -> "❌ ERROR" | |
| LogLevel.WARN -> "⚠️ WARN" | |
| LogLevel.INFO -> "ℹ️ INFO" | |
| LogLevel.DEBUG -> "🐛 DEBUG" | |
| } | |
| NSLog("[$DEFAULT_LOGGING_TAG][$prefix] $message") | |
| private fun logToNSLog( | |
| level: LogLevel, | |
| message: String, | |
| ) { | |
| val prefix = | |
| when (level) { | |
| LogLevel.ERROR -> "❌ ERROR" | |
| LogLevel.WARN -> "⚠️ WARN" | |
| LogLevel.INFO -> "ℹ️ INFO" | |
| LogLevel.DEBUG -> "🐛 DEBUG" | |
| } | |
| NSLog("%@", "[$DEFAULT_LOGGING_TAG][$prefix] $message") | |
| } |
| private fun logToNSLog( | |
| level: LogLevel, | |
| message: String, | |
| ) { | |
| val prefix = | |
| when (level) { | |
| LogLevel.ERROR -> "❌ ERROR" | |
| LogLevel.WARN -> "⚠️ WARN" | |
| LogLevel.INFO -> "ℹ️ INFO" | |
| LogLevel.DEBUG -> "🐛 DEBUG" | |
| } | |
| NSLog("[$DEFAULT_LOGGING_TAG][$prefix] $message") | |
| private fun logToNSLog( | |
| level: LogLevel, | |
| message: String, | |
| ) { | |
| val prefix = | |
| when (level) { | |
| LogLevel.ERROR -> "❌ ERROR" | |
| LogLevel.WARN -> "⚠️ WARN" | |
| LogLevel.INFO -> "ℹ️ INFO" | |
| LogLevel.DEBUG -> "🐛 DEBUG" | |
| } | |
| NSLog("[%@] [%@] %@", DEFAULT_LOGGING_TAG, prefix, message) | |
| } |
| private fun logToNSLog( | |
| level: LogLevel, | |
| message: String, | |
| ) { | |
| val prefix = | |
| when (level) { | |
| LogLevel.ERROR -> "❌ ERROR" | |
| LogLevel.WARN -> "⚠️ WARN" | |
| LogLevel.INFO -> "ℹ️ INFO" | |
| LogLevel.DEBUG -> "🐛 DEBUG" | |
| } | |
| NSLog("[$DEFAULT_LOGGING_TAG][$prefix] $message") | |
| private fun logToNSLog( | |
| level: LogLevel, | |
| message: String, | |
| ) { | |
| val prefix = | |
| when (level) { | |
| LogLevel.ERROR -> "❌ ERROR" | |
| LogLevel.WARN -> "⚠️ WARN" | |
| LogLevel.INFO -> "ℹ️ INFO" | |
| LogLevel.DEBUG -> "🐛 DEBUG" | |
| } | |
| val safePath = message.replace("%".toRegex(), "%%") | |
| NSLog("%@", "[$DEFAULT_LOGGING_TAG][$prefix] $safePath") | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mobile/shared/src/iosMain/kotlin/com/mulkkam/data/logger/LoggerImpl.kt`
around lines 51 - 62, LoggerImpl.logToNSLog currently passes a concatenated
string into NSLog which is treated as a format string and can misinterpret `%`
tokens in LogEntry.message; change the call so the format string is constant and
message is supplied as an argument (e.g. use a constant format like
"[$DEFAULT_LOGGING_TAG][%s] %@" or split into separate arguments) or otherwise
structure the log fields (tag, prefix, message) so NSLog never interprets user
data as a format string; update the NSLog invocation in LoggerImpl.logToNSLog
and ensure DEFAULT_LOGGING_TAG / LogLevel prefix remain constants while
LogEntry.message is passed as a parameter to avoid format-string injection.
junseo511
left a comment
junseo511
left a comment
There was a problem hiding this comment.
큰 문제 없어보이네요~~ 고생하셨습니다 👍
| import Shared | ||
|
|
||
| class FirebaseLoggingBridgeImpl: FirebaseLoggingBridge { | ||
| func logEvent(eventName: String, level: String, message: String, userId: String?) { |
There was a problem hiding this comment.
p3) 함수 이름이 xxxEvent 라는게 적절한지에 대해서 안드라면 논의를 해보겠다만,,, 스위프트는 그냥 대충 살게요
There was a problem hiding this comment.
refactor: FirebaseLoggingBridge의 logEvent 함수명 변경
ㅎㅎ log로 바꿨습니다...
| func recordException(message: String) { | ||
| Crashlytics.crashlytics().record(error: NSError( | ||
| domain: "com.mulkkam", | ||
| code: -1, |
There was a problem hiding this comment.
p3) -1 이 Exception을 관장하는 코드일까요?
There was a problem hiding this comment.
안드로이드에서는 Exception만 던져서 에러를 기록하는데,
iOS에서 쓰는 NSError는 domain + code + userInfo 이렇게 정보가 들어가야 하더라구요
플랫폼 별로 에러 처리 컨벤션이 달라서 우선 의미 없는 매직넘버로 넣어 둔 건데,
enum AppErrorCode {
static let recordedException = -1
static let networkError = -2
// ...
}이런 식으로 Code를 정의할 순 있을 것 같아요
도메인으로 추가해 두는 게 나을까요?
개인적으로는... 지금은 굳이... 라는 생각............
공백의 의견도 궁금합니다
There was a problem hiding this comment.
p3) 여기 파일명이 그냥 Helper 예요! HelperModule 로 일관성 지키기 어떠세요
There was a problem hiding this comment.
| @@ -0,0 +1,12 @@ | |||
| package com.mulkkam.domain.logger | |||
|
|
|||
| interface FirebaseLoggingBridge { | |||
There was a problem hiding this comment.
p3) 기존의 로거로 안되는 브릿지가 필요한 무언가가 있었나요?
There was a problem hiding this comment.
Firebase를iosMain에서 직접 호출 + 구현하려면cinterop설정이 필요함cinterop설정이 꽤나 복잡함
-> Swift에서 FirebaseSDK를 직접 사용해서 구현해야겠다!
여기서 Swift 직접 구현과 commonMain을 연결해 주는 다리가 Bridge 입니다...
FirebaseAnalytics, FirebaseCrashlytics를 직접 이용해서 swift로 구현하고,
구현된 다리를 koin init할 때 넘겨 주는 방식이에용
There was a problem hiding this comment.
🧹 Nitpick comments (1)
mobile/shared/src/iosMain/kotlin/com/mulkkam/di/HelperModule.kt (1)
11-13: 현재 구조에서는 재초기화 위험이 낮지만, 방어적 코딩을 위해 멱등성 추가를 권장합니다.현재
initKoin은@main앱의init()에서 정확히 한 번만 호출되므로, 실제 재초기화 문제는 발생하지 않습니다. 다만 코드베이스가 성장하면서 테스트나 다른 초기화 경로에서 실수로 다시 호출될 수 있기에, 다음과 같이 방어적 가드를 추가하면 더욱 견고합니다.가드 추가 방식:
- 장점: 미래의 코드 변경에서 실수를 방지하고, 테스트/프리뷰 시 안전성 확보
- 단점: 약간의 추가 복잡도 (다만 무시할 수 있는 수준)
🔧 제안 diff
import com.mulkkam.domain.logger.FirebaseLoggingBridge +import org.koin.core.context.GlobalContext import org.koin.core.context.startKoin fun initKoin( baseUrl: String, isDebug: Boolean, firebaseBridge: FirebaseLoggingBridge, ) { + if (GlobalContext.getOrNull() != null) return startKoin { modules(iosSharedModule(baseUrl, isDebug, firebaseBridge)) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mobile/shared/src/iosMain/kotlin/com/mulkkam/di/HelperModule.kt` around lines 11 - 13, Add an idempotency guard to initKoin so startKoin(iosSharedModule(...)) is only executed once: detect an existing Koin instance (e.g., via Koin's GlobalContext.getOrNull() or a private thread-safe flag like an AtomicBoolean) and return early if Koin is already started; if using a flag, make it private to the HelperModule and set it just before/after calling startKoin to ensure thread-safety and avoid double initialization of iosSharedModule.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@mobile/shared/src/iosMain/kotlin/com/mulkkam/di/HelperModule.kt`:
- Around line 11-13: Add an idempotency guard to initKoin so
startKoin(iosSharedModule(...)) is only executed once: detect an existing Koin
instance (e.g., via Koin's GlobalContext.getOrNull() or a private thread-safe
flag like an AtomicBoolean) and return early if Koin is already started; if
using a flag, make it private to the HelperModule and set it just before/after
calling startKoin to ensure thread-safety and avoid double initialization of
iosSharedModule.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7a8e82d6-cca9-4586-9aab-e2244d8e0ea9
📒 Files selected for processing (5)
mobile/iosApp/iosApp/FirebaseLoggingBridgeImpl.swiftmobile/iosApp/iosApp/MulKkamApp.swiftmobile/shared/src/commonMain/kotlin/com/mulkkam/domain/logger/FirebaseLoggingBridge.ktmobile/shared/src/iosMain/kotlin/com/mulkkam/data/logger/LoggerImpl.ktmobile/shared/src/iosMain/kotlin/com/mulkkam/di/HelperModule.kt
🚧 Files skipped from review as they are similar to previous changes (3)
- mobile/iosApp/iosApp/MulKkamApp.swift
- mobile/iosApp/iosApp/FirebaseLoggingBridgeImpl.swift
- mobile/shared/src/iosMain/kotlin/com/mulkkam/data/logger/LoggerImpl.kt
2 participants
🔗 관련 이슈
📝 작업 내용
주요 변경사항
📸 스크린샷 (Optional)
🔗 관련 이슈
📝 작업 내용
iOS에서 기존의 더미(FakeLogger) 기반 로깅을 실제 네이티브 Firebase 연동 로거로 교체합니다. 기존에는 로컬 콘솔 출력에만 머물렀던 로그를 Firebase Analytics(이벤트)와 Crashlytics(예외)로 전송해 원격 모니터링과 오류 수집을 가능하게 하며, 민감정보 자동 마스킹을 로깅 파이프라인에 통합해 개인정보 노출 위험을 줄입니다. 또한 iOS 네이티브 구현과 Kotlin 공용 모듈 간 느슨한 결합을 유지하도록 브리지 패턴과 DI 구성을 변경했습니다.
기존 한계:
주요 변경사항
플랫폼 비의존적 계약 추가
실제 로거 구현과 동작 흐름
민감정보 보호 통합
DI/초기화 경로 변경
운영·개발 모드 분기
리뷰 포인트 / 트레이드오프
📊 변경 효과 요약
정량적 성과(테스트 커버리지, 빌드 시간 등)는 이번 PR에서 측정하지 않음.