Repository navigation
fix: 안정성 하드닝 (코드 검토 발견 결함 일괄) - #107
Conversation
- ToastCenter: 취소된 dismiss 태스크가 새로 표시된 토스트를 숨기던 버그 수정 - NetworkProvider: 요청 30s/리소스 60s 타임아웃 설정(무한 스톨 방지) - ErrorMapper: 연결 실패(cannotConnectToHost/cannotFindHost/dns/connectionLost)를 serverError(0)가 아닌 noInternet으로 매핑 - NetworkLoggerPlugin: 요청/응답 본문 로깅을 #if DEBUG로 게이팅 - KeychainItem: save/delete OSStatus 검사 + 실패 로깅, @discardableResult Bool 반환 - DatabaseKit: SwiftData fetch의 try? 에러 삼킴 제거 → do/catch + os.Logger - RecoveryViewModel/PhoneUpdateViewModel: 인증 타이머 retain cycle 해소([weak self]) - PagingScrollView: LazyHStack 인덱스 out-of-range 방어 가드
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces several improvements across multiple modules, including structured logging with os.Logger in DatabaseKit, network timeout configurations, debug-only network logging, and improved error handling in Keychain operations. It also addresses potential memory leaks in timer tasks and crash issues in a SwiftUI scroll view. Feedback focuses on optimizing the timer tasks in PhoneUpdateViewModel and RecoveryViewModel to prevent delayed deallocation caused by strong self-binding during Task.sleep, and refactoring PagingScrollView to use Identifiable models instead of iterating over indices, which is a known SwiftUI anti-pattern.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| timerTask = Task { [weak self] in | ||
| while !Task.isCancelled, let self, self.timerRemaining > 0 { | ||
| do { | ||
| try await Task.sleep(for: .seconds(1)) | ||
| } catch { | ||
| break | ||
| } | ||
| guard !Task.isCancelled else { break } | ||
| timerRemaining -= 1 | ||
| } | ||
| if timerRemaining <= 0 { | ||
| showError = true | ||
| self.timerRemaining -= 1 | ||
| } | ||
| guard let self, self.timerRemaining <= 0 else { return } | ||
| self.showError = true | ||
| } |
There was a problem hiding this comment.
Task 내부에서 [weak self]를 사용했으나, while 루프의 조건식에서 let self로 강한 참조를 바인딩한 상태로 try await Task.sleep을 호출하고 있습니다.\n\n이로 인해 대기(sleep)하는 동안 self가 강하게 유지되어 화면이 이탈하더라도 즉시 해제되지 못하고 최대 1초간 해제가 지연(delayed deallocation)되는 현상이 발생합니다.\n\n대기를 먼저 수행한 후, 동기적으로 상태를 확인하고 변경할 때만 self를 강하게 바인딩하도록 루프 구조를 개선하면 뷰모델이 즉각적으로 메모리에서 해제될 수 있습니다.
timerTask = Task { [weak self] in\n while !Task.isCancelled {\n do {\n try await Task.sleep(for: .seconds(1))\n } catch {\n break\n }\n \n guard let self else { break }\n guard self.timerRemaining > 0 else { break }\n \n self.timerRemaining -= 1\n \n if self.timerRemaining <= 0 {\n self.showError = true\n break\n }\n }\n }| timerTask = Task { [weak self] in | ||
| while !Task.isCancelled, let self, self.timerRemaining > 0 { | ||
| do { | ||
| try await Task.sleep(for: .seconds(1)) | ||
| } catch { | ||
| break | ||
| } | ||
| guard !Task.isCancelled else { break } | ||
| timerRemaining -= 1 | ||
| self.timerRemaining -= 1 | ||
| } | ||
| if timerRemaining <= 0 { | ||
| isTimerActive = false | ||
| // 3회 모두 사용한 상태에서 만료되면 팝업 | ||
| if resendCount >= maxResends { | ||
| isShowPopup = true | ||
| } | ||
| guard let self, self.timerRemaining <= 0 else { return } | ||
| self.isTimerActive = false | ||
| // 3회 모두 사용한 상태에서 만료되면 팝업 | ||
| if self.resendCount >= self.maxResends { | ||
| self.isShowPopup = true | ||
| } | ||
| } |
There was a problem hiding this comment.
Task 내부에서 [weak self]를 사용했으나, while 루프의 조건식에서 let self로 강한 참조를 바인딩한 상태로 try await Task.sleep을 호출하고 있습니다.\n\n이로 인해 대기(sleep)하는 동안 self가 강하게 유지되어 화면이 이탈하더라도 즉시 해제되지 못하고 최대 1초간 해제가 지연(delayed deallocation)되는 현상이 발생합니다.\n\n대기를 먼저 수행한 후, 동기적으로 상태를 확인하고 변경할 때만 self를 강하게 바인딩하도록 루프 구조를 개선하면 뷰모델이 즉각적으로 메모리에서 해제될 수 있습니다.
timerTask = Task { [weak self] in\n while !Task.isCancelled {\n do {\n try await Task.sleep(for: .seconds(1))\n } catch {\n break\n }\n \n guard let self else { break }\n guard self.timerRemaining > 0 else { break }\n \n self.timerRemaining -= 1\n \n if self.timerRemaining <= 0 {\n self.isTimerActive = false\n if self.resendCount >= self.maxResends {\n self.isShowPopup = true\n }\n break\n }\n }\n }| ForEach(dynamicItems.indices, id: \.self) { index in | ||
| let newsletter = dynamicItems[index] | ||
| RecommendedNewsLetterView(recommendation: newsletter) | ||
| .frame(width: itemWidth, height: itemHeight) | ||
| .id(index) | ||
| .contentShape(Rectangle()) | ||
| .onTapGesture { | ||
| onItemTap(newsletter.id) | ||
| } | ||
| // LazyHStack이 배열 축소 후 stale 인덱스를 실현하며 subscript 크래시가 | ||
| // 나는 것을 방어한다. | ||
| if index < dynamicItems.count { | ||
| let newsletter = dynamicItems[index] | ||
| RecommendedNewsLetterView(recommendation: newsletter) | ||
| .frame(width: itemWidth, height: itemHeight) | ||
| .id(index) | ||
| .contentShape(Rectangle()) | ||
| .onTapGesture { | ||
| onItemTap(newsletter.id) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
SwiftUI에서 동적으로 변경되는 배열(dynamicItems)을 순회할 때 indices와 id: \\.self를 사용하는 것은 안티패턴입니다.\n\n배열의 크기가 줄어들거나 변경될 때, SwiftUI는 뷰의 고유 식별자(Identity)를 데이터 자체가 아닌 '인덱스'로 인식하므로, 이전 렌더링 상태의 stale 인덱스가 LazyHStack 뷰 재사용 과정에서 참조되어 out-of-bounds 크래시가 발생하게 됩니다.\n\n현재 추가된 if index < dynamicItems.count 가드는 크래시를 임시방편으로 막아주지만, 레이아웃 오작동이나 잘못된 데이터 렌더링 등의 부작용을 유발할 수 있습니다.\n\n가장 근본적인 해결책은 dynamicItems를 고유한 ID를 가진 모델 래퍼(예: Identifiable을 준수하는 구조체)로 구성하고, ForEach(dynamicItems) 형태로 직접 순회하도록 리팩토링하는 것입니다.
개요
소셜 로그인 작업 중 진행한 코드 검토에서 발견한 안정성/보안 결함 일괄 수정. 동작 변화 없이 결함만 보강, App 스킴 빌드 검증 완료.
변경
serverError(0)대신noInternet으로 매핑#if DEBUG게이팅(릴리스 토큰/PII 로그 차단)@discardableResult Booltry?에러 삼킴 →do/catch+os.Logger[weak self]테스트