Skip to content

NetworkKit : SessionDeinitialized 이슈 해결#22

Open
ericKwon95 wants to merge 5 commits intodevelopfrom
feature/network-improvement
Open

NetworkKit : SessionDeinitialized 이슈 해결#22
ericKwon95 wants to merge 5 commits intodevelopfrom
feature/network-improvement

Conversation

@ericKwon95
Copy link
Contributor

📓 Overview

  • 홈 화면에서 AFError.sessionDeinitialzied 에러가 발생했습니다.
  • 세션의 선언 방식을 변경해 에러를 해결하였습니다.

🤔 고민 내용

  • 해당 에러는 세션에서 요청을 보냈을 때 요청 수행이 완료되기 전에 세션이 메모리에서 사라질 경우 발생합니다.
  • APIClient는 싱글턴 패턴으로 선언되어 있어 한 번 생성될 경우 계속해서 메모리에 상주합니다.
  • 다만 이전 로직의 경우 세션이 lazy var로 선언되어 있어 여러 스레드에서 첫 초기화를 동시에 시도했을 때 여러 번 초기화될 수 있는 가능성이 존재합니다.
  • 홈 화면 등장시 10개 이상의 API가 각각 다른 Task에서 동시적으로 호출되기 때문에, 하나의 세션이 요청을 수행중이던 도중 다른 세션으로 초기화되는 상황이 발생 가능합니다.
  • 이를 방지하기 위해 APIClient의 이니셜라이저에서 세션을 한 번만 초기화하도록 변경해 문제를 해결하였습니다.

- reqeust -> request
- 실패 시 에러 localizedDescription 출력
- 가독성 저해 이슈로 응답 데이터는 출력하지 않도록 변경
- lazy var로 선언되어 있던 세션 프로퍼티를 여러 스레드에서 동시에 초기화하며 문제 발생
- 초기화 과정에서 한 번만 초기화하도록 구조 변경하여 문제 해결
session 초기화 문제가 있을 예감은 하였지만 이래도 되겠지란 마음으로... 했던게 들킴 추가 보완해서 올립니당
@HenryVoid HenryVoid force-pushed the feature/network-improvement branch from 45b65f8 to f799307 Compare December 18, 2024 12:25

private init() { }

private lazy var session: Session = {
Copy link
Contributor

Choose a reason for hiding this comment

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

session memory 가 해제될 가능성이 있는건 인지하고 있었지만
lazy var로 사용시 생성되는 걸 이용하여 해결할 거라 생각하여..
안일하게 작업하였습니다. ㅜ

return Session(
configuration: configuration,
interceptor: tokenRefresher.map { APIInterceptor(tokenRefresher: $0) },
public init(session: Session, tokenRefresher: TokenRefreshable) {
Copy link
Contributor

Choose a reason for hiding this comment

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

session을 밖에서 설정하여 주입하는 방식으로 변경하였습니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

안녕하세요 오랜만입니다~
shared 인스턴스를 생성할 때 이니셜라이저에서 세션 설정해주었는데
외부에서 주입하는 방식으로 변경해주신 이유가 궁금합니다~

Copy link
Contributor

Choose a reason for hiding this comment

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

#22 (comment)

안녕하세요 오랜만입니다~ shared 인스턴스를 생성할 때 이니셜라이저에서 세션 설정해주었는데 외부에서 주입하는 방식으로 변경해주신 이유가 궁금합니다~

import NetworkKit

extension APIClient {
static let shared: APIClient = APIClient(session: .default, tokenRefresher: TokenRefresher())
Copy link
Contributor

Choose a reason for hiding this comment

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

shared 싱글톤 쓰는것보다 더 좋은 방법이 있을지 고민인데 같이 생각해보면 좋을것 같아요!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

저는 네트워크 객체 특성상 싱글턴 패턴이 적절해 보입니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

세션 객체는 하나만 유지하는 게 좋을 것 같아서요~

Copy link
Contributor

Choose a reason for hiding this comment

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

아래 추가한 내용처럼 session 설정을 달리할 필요가 있을때가 있어요!
예를 들어 이미지 혹은 동영상을 왕창 보내거나 받거나 할때 session의 timeout 설정을 다르게 할때도 있고요!
오늘 같은 경우엔 저는 login API 를 보내는데 실제 token이 필요하지않았고 그 token이 없을때 Interceptor를 괜히
설정할 필요가 없었고요!
이 외에도 더 있을 것 같은데... 지금은 생각이 안나네요ㅜ

세션 객체는 하나만 유지하는 게 좋을 것 같아서요~

Copy link
Contributor Author

Choose a reason for hiding this comment

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

그렇다면 싱글턴 API 클라이언트가 필요한 세션들을 가지고 있고, request 형태에 따라 적절한 세션을 통해 요청을 수행하는 방식은 어떤가요?
필요에 따라 다른 세션을 세션을 주입하는 의도는 이해했으나, API 카테고리마다 모두 다른 클라이언트를 가지고 있는 현재 구조에서는
필요한 API를 요청할 때 마다 서로 다른 세션이 중복 생성되는 상황이 발생할 수 있다고 생각합니다~~
혹시 더 좋은 방법이 있을까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Screenshot 2024-12-20 at 5 24 14 PM

요청마다 세션을 생성하는 것이 안티패턴인 이유도 함께 첨부드립니다!
애플 개발자 포럼

Copy link
Contributor

Choose a reason for hiding this comment

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

외부에서 Session 주입할 때 유의하여 사용하고 APIClient는 보통 싱글톤을 사용하되 예외케이스 시 주입하여 사용하는 방식으로 수정하였습니다!

login: { param in
let endPoint = ClimberEndPoint.login(param)
return try await APIClient.shared.request(endPoint, decode: SignResponse.self)
return try await APIClient(session: .default, tokenRefresher: nil).request(endPoint, decode: SignResponse.self)
Copy link
Contributor

Choose a reason for hiding this comment

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

login API는 token 없이 network해야하므로 기존 shared(공통)으로 빼놓은 경우랑 달리 해야합니다.

interceptor: APIInterceptor(tokenRefresher: tokenRefresher),
eventMonitors: [APILogger()]
)
public init(session: Session, tokenRefresher: TokenRefreshable?) {
Copy link
Contributor

Choose a reason for hiding this comment

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

token 없이 network할 경우 retry method가 필요가 없습니다!
그래서 tokenRefresher 없이 APIClient를 사용할 경우를 위해 작업하였습니다.


func retry(_ request: Request, for session: Session, dueTo error: any Error, completion: @escaping @Sendable (RetryResult) -> Void) {

guard let response = request.task?.response as? HTTPURLResponse,
Copy link
Contributor

Choose a reason for hiding this comment

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

테스트해보니 Token이 없으면 무조건 retry를 타고 무조건 retry API를 돌리는건 비효율적이라
판단하여 추가하였습니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

고생하셨습니다~
불필요한 세션 발생 방지를 위해 APIClient는 싱글턴으로 유지하면 좋을 것 같은데요, request 자체에 토큰 관련 검증 로직을 넣는 방식은 어떨까요?

Copy link
Contributor

Choose a reason for hiding this comment

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

request 자체에 토큰 관련 검증 로직이 무엇일까요?
(현재 tokenRefresher, interceptor 그리고 urlRequest 에서 token이 있는지 없는지 체크하는 로직이 있는데..)
싱글턴으로 유지하면서 session 설정을 다르게 할 수 있는 방법 알려주시면 적용해보겠습니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

그러네요 request 자체에서는 할 수 없겠군요...
위에서 말씀드린것처럼 서로 다른 세션을 싱글턴 내부에 두고
request 메소드를 다르게 해 재사용하는 방식은 어떨지 궁금합니다~!

Copy link
Contributor

Choose a reason for hiding this comment

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

싱글톤 내부에서 세션을 여러개 사용하는 것이 아닌 싱글톤을 여러개 생성하는 방식으로 회의 때 결정하였습니다.

https://www.notion.so/climbers/24-12-21-iOS-27-e5ff159050534540b6575120e13d5d19?pvs=4

Copy link
Contributor

@HenryVoid HenryVoid left a comment

Choose a reason for hiding this comment

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

제가 놓친 부분을 잡아내주셔서 감사합니다!
진행해나가면서 더 좋은 방안이 있으면 의논하면서 추가해보아요!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants