Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[Hotfix] #430 - 솝탬프 디테일뷰 크래시 이슈 해결 #431

Merged
merged 6 commits into from
Nov 7, 2024
Merged

[Hotfix] #430 - 솝탬프 디테일뷰 크래시 이슈 해결 #431

merged 6 commits into from
Nov 7, 2024

Conversation

dlwogus0128
Copy link
Contributor

@dlwogus0128 dlwogus0128 commented Nov 7, 2024 •
edited
Loading

🌴 PR 요약

🌱 작업한 브랜치

🌱 PR Point

  • 지속되고 있는 솝탬프 디테일뷰 크래시 이슈를 해결하고자 합니다.

    • 현재 크래시가 발생하고 있는 분들의 IPS 파일을 받아보았을 때, KF을 사용하는 부분에서 EXC_BAD_ACCESS가 발생하는 것을 확인했습니다.
    • 이에 해당 라이브러리의 버전을 업데이트하고 재배포했으나, 테플에서는 이슈가 해결되었지만 앱스토어에서 설치할 경우 (일부 사용자에게) 여전히 크래시가 발생하고 있는 상황입니다.
    • 따라서, 2.6.2 버전으로, 이슈의 원인으로 추정할 수 있는 여러 부분들을 수정해 재배포하려 합니다.

  • 수정한 부분

    • kf.setImage -> 랩핑해서 사용하고 있는 setImge로 수정 (한글 URL 인코딩)
    • 솝탬프 디테일뷰 바인딩 -> 강한참조 부분 수정

📌 참고 사항

생략

📸 스크린샷

생략

📮 관련 이슈

@dlwogus0128 dlwogus0128 added bug Something isn't working 재현✦ labels Nov 7, 2024
@dlwogus0128 dlwogus0128 self-assigned this Nov 7, 2024
@height Height
Copy link

height bot commented Nov 7, 2024

Link Height tasks by mentioning a task ID in the pull request title or commit messages, or description and comments with the keyword link (e.g. "Link T-123").

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

yungu0010
yungu0010 reviewed Nov 7, 2024
View reviewed changes
Copy link
Contributor

@yungu0010 yungu0010 left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

솝탬프를 책임지는 재현이 . . 이번에 꼬옥 해결되면 좋겠어요..🫶🫶🫶
그리고 머지하는 브랜치 release로 변경해주세요!!

...S/Projects/Features/StampFeature/Sources/ListDetailScene/ViewModel/ListDetailViewModel.swift Outdated
@@ -154,19 +158,22 @@ extension ListDetailViewModel {

listDetailModel.asDriver()
.compactMap {
self.uploadedUrl = $0.image
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

release브랜치에 반영되었던 부분인데 혹시 release 브랜치에서 새 브랜치 파신게 맞는지 확인 한 번 더 부탁드립니다!_!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

그리고 이 부분에서도 self로 뷰모델을 참조하는데 다시 self의 cancelBag에 저장되기 때문에 순환참조가 발생할 것 같습니다 !

Suggested change
self.uploadedUrl = $0.image
listDetailModel.asDrivaer()
.withUnretained(self)
.compactMap { owner, model in
self.uploadedUrl = model.image
self.stampId = model.stampId
return model
}

Copy link
Contributor Author

@dlwogus0128 dlwogus0128 Nov 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

release브랜치에 반영되었던 부분인데 혹시 release 브랜치에서 새 브랜치 파신게 맞는지 확인 한 번 더 부탁드립니다!_!

🥹 실수입니다 .. 잘 되돌려놓겠습니다..

SOPT-iOS/Projects/Features/StampFeature/Sources/ListDetailScene/VC/ListDetailVC.swift
@@ -174,50 +178,54 @@ extension ListDetailVC {

output.$listDetailModel
.compactMap { $0 }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

withUnretained 내부에서 compactMap으로 nil을 필터링 해주고 있기 때문에 해당 코드는 지워주세요@!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

withUnretained 내부에 있는 compactMap은 self만 옵셔널 해제를 해주고 있고, model 자체가 옵셔널로 선언되어 있어서 이 친구에 대한 해제가 필요할 것 같다구 생각했습니다!.!

compactMap { [weak object] output in
    guard let object = object else {
        return nil
    }
    return (object, output)
}
@Published var listDetailModel: ListDetailModel?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

헉!! 제가 self만 확인하고 listDetailModel은 생각을 못했네요..!_! 정정 감사합니다🙌

...S/Projects/Features/StampFeature/Sources/ListDetailScene/ViewModel/ListDetailViewModel.swift Outdated
@@ -75,6 +75,7 @@ extension ListDetailViewModel {
.filter { owner, _ in
owner.sceneType == .completed
}
.withUnretained(self)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

혹시 한 번 더 withUnretained를 해준 이유가 있을까요?

Copy link
Contributor Author

@dlwogus0128 dlwogus0128 Nov 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

약한 참조가 필요한 부분마다 .withUnretained(self) 를 해줘야 한다고 잘못 이해했었어요..! 지금 살펴보니까 중복해서 할 필요가 업네요.. 지우겠습니다 !!

@dlwogus0128 dlwogus0128 force-pushed the hotfix/#430-soptamp-detail-view-crash branch from 8a4aff3 to b4fa968 Compare November 7, 2024 14:46
@pull-request-size pull-request-size bot added size/S and removed size/M labels Nov 7, 2024
@pull-request-size pull-request-size bot added size/M and removed size/S labels Nov 7, 2024
@dlwogus0128 dlwogus0128 changed the base branch from develop to release November 7, 2024 14:59
@dlwogus0128 dlwogus0128 merged commit e167c41 into release Nov 7, 2024
@dlwogus0128 dlwogus0128 deleted the hotfix/#430-soptamp-detail-view-crash branch November 7, 2024 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yungu0010 yungu0010 yungu0010 left review comments

Assignees

@dlwogus0128 dlwogus0128

Labels
bug Something isn't working size/M 재현✦
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Hotfix] 솝탬프 디테일뷰 크래시 이슈
2 participants
@dlwogus0128 @yungu0010