Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions core/network/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ android {
}

dependencies {
implementation(projects.core.result)

implementation(libs.bundles.ktor)
implementation(libs.ktorfit.lib)
ksp(libs.ktorfit.ksp)
Expand Down
50 changes: 50 additions & 0 deletions core/network/src/main/java/com/twix/network/execute/ApiCall.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package com.twix.network.execute

import com.twix.network.model.error.ErrorResponse
import com.twix.result.AppError
import com.twix.result.AppResult
import io.ktor.client.plugins.ResponseException
import io.ktor.client.statement.bodyAsText
import kotlinx.io.IOException
import kotlinx.serialization.SerializationException
import kotlinx.serialization.json.Json
import java.net.SocketTimeoutException
import kotlin.coroutines.cancellation.CancellationException

val errorJson =
Json {
ignoreUnknownKeys = true
isLenient = true
}

suspend inline fun <T> safeApiCall(crossinline call: suspend () -> T): AppResult<T> =
try {
AppResult.Success(call())
} catch (e: CancellationException) {
throw e
} catch (e: ResponseException) {
val status = e.response.status.value
val raw = runCatching { e.response.bodyAsText() }.getOrNull()

val parsed =
runCatching {
raw?.let { errorJson.decodeFromString<ErrorResponse>(it) }
}.getOrNull()
Comment on lines +27 to +32
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

find . -type f -name "ApiCall.kt" | head -20

Repository: YAPP-Github/Twix-Android

Length of output: 132


🏁 Script executed:

git ls-files | grep -E "network|ApiCall" | head -20

Repository: YAPP-Github/Twix-Android

Length of output: 1001


🏁 Script executed:

cat -n core/network/src/main/java/com/twix/network/execute/ApiCall.kt | head -50

Repository: YAPP-Github/Twix-Android

Length of output: 2016


🌐 Web query:

Kotlin runCatching CancellationException coroutine cancellation propagation

💡 Result:

runCatching { ... } catches all Throwable from its block—this includes CancellationException. In coroutines, CancellationException is the mechanism used to stop work and propagate cancellation through suspension points; swallowing it breaks cancellation propagation (the coroutine may keep running until the next cancellable suspension/check, and parent/child cancellation behavior becomes surprising). (kotlinlang.org)

What to do instead

If you use runCatching inside coroutines, rethrow cancellation:

val result = runCatching {
    doWork()
}.onFailure { e ->
    if (e is CancellationException) throw e
}

or equivalently:

val result = try {
    Result.success(doWork())
} catch (e: CancellationException) {
    throw e
} catch (t: Throwable) {
    Result.failure(t)
}

Kotlin’s coroutine docs explicitly warn that catching CancellationException can break cancellation propagation and that you should rethrow it. (kotlinlang.org)

Why it matters (rule of thumb)

  • CancellationException is treated specially by coroutine exception handling (it’s generally ignored by handlers) because it represents normal cancellation, not an error. (kotlinlang.org)
  • Therefore, any “catch-all” (catch (Throwable) / runCatching) in coroutine code should always let CancellationException escape. (medium.com)

Citations:


🏁 Script executed:

rg "runCatching" --type kotlin -A 3 | head -60

Repository: YAPP-Github/Twix-Android

Length of output: 1164


🏁 Script executed:

cat -n core/network/src/main/java/com/twix/network/execute/ApiCall.kt | sed -n '20,42p'

Repository: YAPP-Github/Twix-Android

Length of output: 980


runCatching이 코루틴 취소를 막을 수 있어요

runCatchingCancellationException을 포함한 모든 Throwable을 잡기 때문에, bodyAsText()나 JSON 파싱 중에 취소가 발생해도 예외가 getOrNull()로 인해 null로 변환되어 취소 신호가 전파되지 않습니다. 이는 코루틴 취소 의도를 무효화하고 상위 코루틴까지 취소가 전파되지 않는 문제를 일으킵니다.

Kotlin 공식 문서에서도 코루틴 내에서 runCatching을 사용할 때는 반드시 CancellationException을 재throw할 것을 권장하고 있습니다. 두 runCatching 호출 모두에 다음과 같이 취소 예외 처리를 추가해 주세요.

🔧 제안 변경
-        val raw = runCatching { e.response.bodyAsText() }.getOrNull()
+        val raw =
+            runCatching { e.response.bodyAsText() }
+                .onFailure { if (it is CancellationException) throw it }
+                .getOrNull()
 ...
-        val parsed =
-            runCatching {
-                raw?.let { errorJson.decodeFromString<ErrorResponse>(it) }
-            }.getOrNull()
+        val parsed =
+            runCatching {
+                raw?.let { errorJson.decodeFromString<ErrorResponse>(it) }
+            }
+                .onFailure { if (it is CancellationException) throw it }
+                .getOrNull()
📝 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.

Suggested change
val raw = runCatching { e.response.bodyAsText() }.getOrNull()
val parsed =
runCatching {
raw?.let { errorJson.decodeFromString<ErrorResponse>(it) }
}.getOrNull()
val raw =
runCatching { e.response.bodyAsText() }
.onFailure { if (it is CancellationException) throw it }
.getOrNull()
val parsed =
runCatching {
raw?.let { errorJson.decodeFromString<ErrorResponse>(it) }
}
.onFailure { if (it is CancellationException) throw it }
.getOrNull()
🤖 Prompt for AI Agents
In `@core/network/src/main/java/com/twix/network/execute/ApiCall.kt` around lines
27 - 32, The two runCatching blocks that compute raw (calling
e.response.bodyAsText()) and parsed (calling
errorJson.decodeFromString<ErrorResponse>(it)) swallow CancellationException and
prevent coroutine cancellation from propagating; update those runCatching usages
so they rethrow CancellationException (or explicitly check and throw if caught)
instead of converting it to null, ensuring CancellationException is not
swallowed when reading bodyAsText() or decoding JSON in the raw/parsed logic.


AppResult.Error(
AppError.Http(
status = status,
code = parsed?.code,
message = parsed?.message,
rawBody = raw,
),
)
} catch (e: SocketTimeoutException) {
AppResult.Error(AppError.Timeout(e))
} catch (e: IOException) {
AppResult.Error(AppError.Network(e))
Comment on lines +8 to +45
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# Find AppError definition
fd -t f -e kt -e java | xargs rg "class AppError|sealed.*AppError|data class Network" -B 2 -A 5 | head -100

Repository: YAPP-Github/Twix-Android

Length of output: 1353


🏁 Script executed:

# Search for AppError definition more specifically
find . -type f \( -name "*.kt" -o -name "*.java" \) -exec rg "AppError" {} + -l | head -20

Repository: YAPP-Github/Twix-Android

Length of output: 96


🏁 Script executed:

# Look for the AppError class definition
rg "class AppError|sealed.*AppError" -t kt -t java -A 10

Repository: YAPP-Github/Twix-Android

Length of output: 94


🏁 Script executed:

cat core/result/src/main/java/com/twix/result/AppError.kt

Repository: YAPP-Github/Twix-Android

Length of output: 827


🏁 Script executed:

# Check if there's any usage of kotlinx.io.IOException in the codebase
rg "kotlinx\.io\.IOException|java\.io\.IOException" --color=never

Repository: YAPP-Github/Twix-Android

Length of output: 241


🏁 Script executed:

# Check the imports and structure of ApiCall.kt
cat core/network/src/main/java/com/twix/network/execute/ApiCall.kt

Repository: YAPP-Github/Twix-Android

Length of output: 1666


kotlinx.io.IOExceptionjava.io.IOException으로 변경 필요

현재 코드의 문제점:

  • AppError.Networkjava.io.IOException을 기대합니다 (AppError.kt에서 java.io.IOException import)
  • 그런데 ApiCall.kt에서는 kotlinx.io.IOException을 catch하고 있습니다
  • 이 둘은 서로 다른 타입이므로, Ktor에서 발생하는 java.io.IOException은 catch 블록에 걸리지 않습니다
  • 결과적으로 네트워크 오류가 catch (e: Throwable) 블록으로 빠져 AppError.Unknown으로 처리됩니다

import를 다음과 같이 수정하면 네트워크 오류가 올바르게 AppError.Network로 처리됩니다:

-import kotlinx.io.IOException
+import java.io.IOException
📝 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.

Suggested change
import kotlinx.io.IOException
import kotlinx.serialization.SerializationException
import kotlinx.serialization.json.Json
import java.net.SocketTimeoutException
import kotlin.coroutines.cancellation.CancellationException
val errorJson =
Json {
ignoreUnknownKeys = true
isLenient = true
}
suspend inline fun <T> safeApiCall(crossinline call: suspend () -> T): AppResult<T> =
try {
AppResult.Success(call())
} catch (e: CancellationException) {
throw e
} catch (e: ResponseException) {
val status = e.response.status.value
val raw = runCatching { e.response.bodyAsText() }.getOrNull()
val parsed =
runCatching {
raw?.let { errorJson.decodeFromString<ErrorResponse>(it) }
}.getOrNull()
AppResult.Error(
AppError.Http(
status = status,
code = parsed?.code,
message = parsed?.message,
rawBody = raw,
),
)
} catch (e: SocketTimeoutException) {
AppResult.Error(AppError.Timeout(e))
} catch (e: IOException) {
AppResult.Error(AppError.Network(e))
import java.io.IOException
import kotlinx.serialization.SerializationException
import kotlinx.serialization.json.Json
import java.net.SocketTimeoutException
import kotlin.coroutines.cancellation.CancellationException
val errorJson =
Json {
ignoreUnknownKeys = true
isLenient = true
}
suspend inline fun <T> safeApiCall(crossinline call: suspend () -> T): AppResult<T> =
try {
AppResult.Success(call())
} catch (e: CancellationException) {
throw e
} catch (e: ResponseException) {
val status = e.response.status.value
val raw = runCatching { e.response.bodyAsText() }.getOrNull()
val parsed =
runCatching {
raw?.let { errorJson.decodeFromString<ErrorResponse>(it) }
}.getOrNull()
AppResult.Error(
AppError.Http(
status = status,
code = parsed?.code,
message = parsed?.message,
rawBody = raw,
),
)
} catch (e: SocketTimeoutException) {
AppResult.Error(AppError.Timeout(e))
} catch (e: IOException) {
AppResult.Error(AppError.Network(e))
🤖 Prompt for AI Agents
In `@core/network/src/main/java/com/twix/network/execute/ApiCall.kt` around lines
8 - 45, The catch block in safeApiCall is importing and catching
kotlinx.io.IOException which differs from the java.io.IOException that
AppError.Network expects (see AppError.kt); update ApiCall.kt to import
java.io.IOException instead of kotlinx.io.IOException so the catch (e:
IOException) in safeApiCall correctly captures Ktor/Java IO errors and maps them
to AppError.Network.

} catch (e: SerializationException) {
AppResult.Error(AppError.Serialization(e))
} catch (e: Throwable) {
AppResult.Error(AppError.Unknown(e))
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package com.twix.network.model.error

import kotlinx.serialization.Serializable

@Serializable
data class ErrorResponse(
val status: Int? = null,
val code: String? = null,
val message: String? = null,
)
1 change: 1 addition & 0 deletions core/result/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
/build
7 changes: 7 additions & 0 deletions core/result/build.gradle.kts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
plugins {
alias(libs.plugins.twix.android.library)
}

android {
namespace = "com.twix.result"
}
Empty file added core/result/consumer-rules.pro
Empty file.
21 changes: 21 additions & 0 deletions core/result/proguard-rules.pro
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# Add project specific ProGuard rules here.
# You can control the set of applied configuration files using the
# proguardFiles setting in build.gradle.
#
# For more details, see
# http://developer.android.com/guide/developing/tools/proguard.html

# If your project uses WebView with JS, uncomment the following
# and specify the fully qualified class name to the JavaScript interface
# class:
#-keepclassmembers class fqcn.of.javascript.interface.for.webview {
# public *;
#}

# Uncomment this to preserve the line number information for
# debugging stack traces.
#-keepattributes SourceFile,LineNumberTable

# If you keep the line number information, uncomment this to
# hide the original source file name.
#-renamesourcefileattribute SourceFile
4 changes: 4 additions & 0 deletions core/result/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<?xml version="1.0" encoding="utf-8"?>
<manifest xmlns:android="http://schemas.android.com/apk/res/android">

</manifest>
33 changes: 33 additions & 0 deletions core/result/src/main/java/com/twix/result/AppError.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package com.twix.result

import java.io.IOException

sealed interface AppError {
/** HTTP 4xx/5xx */
data class Http(
val status: Int, // HTTP status (e.code())
val code: String? = null, // 서버에서 반환하는 커스텀 코드 ex) G5000
val message: String? = null, // 서버에서 반환하는 message
val rawBody: String? = null, // Http 에러 원문
) : AppError

/** 네트워크 끊김/불가 */
data class Network(
val cause: IOException? = null,
) : AppError

/** 타임아웃 */
data class Timeout(
val cause: Throwable? = null,
) : AppError

/** 파싱/직렬화 */
data class Serialization(
val cause: Throwable? = null,
) : AppError

/** 그 외 */
data class Unknown(
val cause: Throwable,
) : AppError
}
11 changes: 11 additions & 0 deletions core/result/src/main/java/com/twix/result/AppResult.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package com.twix.result

sealed interface AppResult<out T> {
data class Success<out T>(
val data: T,
) : AppResult<T>

data class Error(
val error: AppError,
) : AppResult<Nothing>
}
1 change: 1 addition & 0 deletions core/ui/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,5 @@ android {

dependencies {
implementation(projects.domain)
implementation(projects.core.result)
}
49 changes: 35 additions & 14 deletions core/ui/src/main/java/com/twix/ui/base/BaseViewModel.kt
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package com.twix.ui.base
import androidx.lifecycle.ViewModel
import androidx.lifecycle.viewModelScope
import co.touchlab.kermit.Logger
import com.twix.result.AppError
import com.twix.result.AppResult
import kotlinx.coroutines.Job
import kotlinx.coroutines.channels.Channel
import kotlinx.coroutines.flow.Flow
Expand Down Expand Up @@ -79,33 +81,52 @@ abstract class BaseViewModel<S : State, I : Intent, SE : SideEffect>(
onStart: (() -> Unit)? = null, // 비동기 시작 전 처리해야 할 로직 ex) 로딩
onFinally: (() -> Unit)? = null, // 비동기 종료 후 리소스 정리
onSuccess: (D) -> Unit, // 비동기 메서드 호출이 성공했을 때 처리해야 할 로직
onError: ((Throwable) -> Unit)? = null, // 비동기 메서드 호출에 실패했을 때 처리해야 할 로직
block: suspend () -> Result<D>, // 비동기 메서드 ex) 서버 통신 메서드
onError: ((AppError) -> Unit)? = null, // 비동기 메서드 호출에 실패했을 때 처리해야 할 로직
block: suspend () -> AppResult<D>, // 비동기 메서드 ex) 서버 통신 메서드
): Job =
viewModelScope.launch {
try {
onStart?.invoke()

val result = block.invoke()
result.fold(
onSuccess = { data -> onSuccess(data) },
onFailure = { t ->
if (t is CancellationException) throw t

handleError(t)
onError?.invoke(t)
},
)
when (val result = block()) {
is AppResult.Success -> onSuccess(result.data)
is AppResult.Error -> {
// 공통 처리: 로깅
handleError(result.error)
// 메서드별 처리: 특정 화면만의 UX ex) 다이얼로그/토스트
onError?.invoke(result.error)
}
}
} catch (e: CancellationException) {
// 코루틴 취소는 에러로 취급하지 않기
throw e
} finally {
onFinally?.invoke()
}
}

/**
* 에러 핸들링 메서드
* Throwable용 핸들러 ex) Intent 처리 중 발생한 예외
* */
protected open fun handleError(t: Throwable) {
logger.e { "에러 발생: ${t.stackTraceToString()}" }
// TODO: 크래시 리포트
logger.e(t) { "Unhandled error while handling intent" }
}

/**
* AppError용 핸들러 ex) 서버통신에서 발생한 에러
* */
protected open fun handleError(error: AppError) {
when (error) {
is AppError.Http ->
logger.e {
"HTTP error: status=${error.status}, code=${error.code}, message=${error.message}"
}
is AppError.Network -> logger.e(error.cause) { "Network error" }
is AppError.Timeout -> logger.e(error.cause) { "Timeout error" }
is AppError.Serialization -> logger.e(error.cause) { "Serialization error" }
is AppError.Unknown -> logger.e(error.cause) { "Unknown error" }
}
}

// 리소스 정리
Expand Down
1 change: 1 addition & 0 deletions settings.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,4 @@ include(":core:network")
include(":core:analytics")
include(":feature:main")
include(":feature:task-certification")
include(":core:result")