[Refactor] 개발자 모드 개선 & 네트워크 예외 처리 오류 수정 (FlowCallAdapter) #349
Conversation
* 네트워크 요청에서 예외가 발생하면 flow의 catch 연산자로 잡히지 않음 * Flow로 변환되기 전에 Interceptor에서 예외가 발생하기 때문 * 따라서, Retrofit에서 Flow로 반환하도록 변경 * 네트워크 요청에서 발생하는 예외도 flow로 보내도록 수정
| @GET | ||
| fun checkServerStatus( | ||
| @Url url: String | ||
| ): Flow<Result<ResponseServerStatus>> |
There was a problem hiding this comment.
여기서는 사용자 정의 타입 FlowResult 를 사용하지 않은 특별한 이유가 있나요??
There was a problem hiding this comment.
@leeeha 회의록에 적어둔대로 적용할 지 의견을 들어본 후 수정할 예정이라 다 고치진 않았습니다~!
| import com.runnect.runnect.developer.data.service.ServerStatusService | ||
| import javax.inject.Inject | ||
|
|
||
| class RemoteServerStatusDataSource @Inject constructor( |
There was a problem hiding this comment.
전부터 느꼈지만 remote가 상위 패키지 이름이므로 클래스 앞에 매번 Remote라는 접두사는 안 붙여도 될 거 같다는 생각이 듭니다! 사소하지만 코멘트 남겨봅니다 ㅎ.ㅎ
| private val clipboardManager: ClipboardManager? by lazy { | ||
| context?.getSystemService(Context.CLIPBOARD_SERVICE) as? ClipboardManager | ||
| } | ||
|
|
||
| override fun onCreatePreferences(savedInstanceState: Bundle?, rootKey: String?) { | ||
| setPreferencesFromResource(R.xml.preferences_developer_menu, rootKey) | ||
| activity?.apply { | ||
| setStatusBarColor(window = window, true, R.color.white) |
There was a problem hiding this comment.
named argument 모든 인자에 대해 적용해주면 더 좋을 거 같습니다 : )
There was a problem hiding this comment.
@leeeha 오 네네 수정해두겠습니다~!
| } | ||
|
|
||
| private fun initUserInfo() { | ||
| val ctx: Context = context ?: return | ||
| val accessToken = ctx.getAccessToken() | ||
| val refreshToken = ctx.getNewToken() | ||
| val combinedToken = "${ApiMode.getCurrentApiMode(ctx).name} 서버\n[Access Token]: $accessToken\n\n---\n\n[Refresh Token]: $refreshToken" |
| Intent.makeRestartActivityTask(component).apply { | ||
| startActivity(this) | ||
| exitProcess(0) | ||
| } |
| fun checkProdServerStatus() { | ||
| val prodServerUrl = "${BuildConfig.RUNNECT_PROD_URL}/actuator/health" | ||
| checkServerStatus(prodServerUrl, _prodStatus) | ||
| } | ||
|
|
||
| fun checkTestServerStatus() { | ||
| val testServerUrl = "${BuildConfig.RUNNECT_DEV_URL}/actuator/health" | ||
| checkServerStatus(testServerUrl, _testStatus) | ||
| } |
There was a problem hiding this comment.
두 함수의 동작이 굉장히 유사해서 인자를 추가하면 하나로 통일할 수 있을 거 같아요!
There was a problem hiding this comment.
@leeeha
오호 더 좋은 방법 생각 나신게 있다면 코드 제안 부탁드립니다~
| 503 -> ServerState.Degraded | ||
| else -> ServerState.Error | ||
| }.let(state::tryEmit) | ||
| } |
| import com.runnect.runnect.R | ||
| import com.runnect.runnect.developer.enum.ServerStatus | ||
|
|
||
| class ServerStatusPreference @JvmOverloads constructor( |
There was a problem hiding this comment.
오호 @JvmOverloads 이걸 붙이면 생성자 오버로딩을 자동으로 생성해주는군요!
| import com.runnect.runnect.R | ||
| import com.runnect.runnect.developer.presentation.RunnectDeveloperViewModel.ServerState | ||
|
|
||
| enum class ServerStatus( |
There was a problem hiding this comment.
@leeeha 서버팀 레포 구경하다가 Actuator 설정 되어있길래 바로 적용 했슴니다😎
*PR 대응
|
|
||
| private fun parseResponse(response: Response<T>): Result<T> { | ||
| val nullBodyException by lazy { | ||
| RunnectException(response.code(), ERROR_MSG_RESPONSE_IS_NULL) |
There was a problem hiding this comment.
RunnectException은 어떤 상황에 발생하는 exception인가요?
There was a problem hiding this comment.
@unam98 그냥 프로젝트 공용으로 커스텀 Exception을 만들어야할 때 쓸 목적으로 만들어 뒀습니다
|
|
||
| private suspend fun flowApiCall(call: Call<T>): Result<T> { | ||
| return suspendCancellableCoroutine { continuation -> | ||
| runCatching { |
There was a problem hiding this comment.
그때 여기 runCatching으로 감싼 이유가 머라고 하셨었죵?
There was a problem hiding this comment.
@unam98 call.enqueue의 onFailure로 내려오지 않는 예외까지 처리하기 위함이었으나 예외가 발생하면 call.enqueue의 onFailure로 내려오네요
예외 자체를 flow로 보내도록 변경하였으니 runCatching을 굳이 안써도 될 것 같습니다!
| } | ||
| }) | ||
| }.onFailure { | ||
| continuation.resumeWithException(it) |
There was a problem hiding this comment.
@unam98 timeout 등의 예외가 call.enqueue에서 안잡히는 줄 알았는데 지금 확인해보니까 call.enqueue의 onFailure로 잘 내려오네요
runCatching을 제거해도 이제는 flow로 감싸져있으니까 예외가 잘 잡히네요
runCatching 제거해도 될 것 같습니다
There was a problem hiding this comment.
@unam98 테스트를 해보긴했는데 runCatching 없어도 크래시 발생하지 않는지 크로스 체크 부탁드립니다~
| } | ||
|
|
||
| continuation.invokeOnCancellation { | ||
| call.cancel() |
| gson.fromJson(it, ErrorResponse::class.java) | ||
| } | ||
|
|
||
| val errorMessage = errorResponse?.message ?: errorResponse?.error ?: ERROR_MSG_COMMON |
There was a problem hiding this comment.
여기는 에러 메세지 분기를 3단으로 나눠주신 특별한 이유가 있나요?
There was a problem hiding this comment.
- errorResponse?.message : 러넥트 서버 커스텀 에러의 메세지
- errorResponse?.error : Http 기본 에러 메세지
러넥트 서버의 커스텀 에러와 Http 기본 에러 형식을 모두 받을 수 있게 ErrorResponse를 작성해두었습니다.
러넥트 서버의 커스텀 에러를 우선적으로 수신하기 위해 위와 같이 분기하였습니다
| fun <R, D> FlowResult<R>.toEntityResult(mapper: (R) -> D): FlowResult<D> { | ||
| fun <R, D> Result<R>.parseResult(mapper: (R) -> D): Result<D> = when { |
There was a problem hiding this comment.
함수 안에 함수를 넣어주신 이유가 있나요? 개인적인 생각으론 여기 안에서만 쓰인다면 굳이 함수화까진 안 해도 되지 않을까 싶어서용.
There was a problem hiding this comment.
@unam98
개인적으로 가독성이 좀 더 좋다고 생각되어 내부 함수로 만들었습니다
더 깔끔한 방법이 있으면 코드 제안 부탁드립니다~
📌 개요
✨ 작업 내용
네트워크 예외 처리 오류 수정 (FlowCallAdapter로 변경)
🌟 개발자 모드 기능 개선 🌟
✨ PR 포인트
FlowCallAdapter 동작 확인
네트워크 예외 테스트 케이스
Case 1.
Case 2.
Case 3.
Case 4.