refactor: 네트워크 레이어 Gson → Kotlin Serialization 전환#381
Closed
Conversation
ResponseInterceptor, ResultCall, FlowCallAdapter에서 Gson()을 직접 생성하여 BaseResponse/ErrorResponse를 파싱하던 코드를 kotlinx.serialization.json으로 교체.
📝 WalkthroughWalkthroughThis pull request migrates JSON parsing from Gson to Kotlinx Serialization across three network utility files. Error response handling and JSON extraction logic were refactored to use Kotlinx Serialization's JSON parsing API with unknown key ignorance enabled, while maintaining equivalent error handling behavior and fallback mechanisms. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
app/src/main/java/com/runnect/runnect/data/network/interceptor/ResponseInterceptor.kt (1)
47-49: 파싱 실패를 완전히 무시하지 말고 최소한의 진단 로그를 남겨주세요.Line 47-49는 릴리즈에서 재발 시 원인 추적이 어렵습니다. 응답 본문 전체는 제외하고 예외 타입/메시지만 debug 레벨로 남기면 운영 가시성이 좋아집니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/runnect/runnect/data/network/interceptor/ResponseInterceptor.kt` around lines 47 - 49, The catch block in ResponseInterceptor (inside the intercept method of class ResponseInterceptor) currently swallows exceptions by returning null; instead log the exception type and message at debug level before returning null to preserve minimal diagnostic visibility (do not log the full response body). Update the catch (e: Exception) branch to call your logger/debug utility (e.g., using the interceptor's logger or Android Log/Timbre) to record e::class.simpleName and e.message, then return null as before.app/src/main/java/com/runnect/runnect/data/network/calladapter/flow/FlowCallAdapter.kt (1)
62-73:ResultCall과 예외 코드 결정 규칙을 맞추는 것을 권장합니다.현재는 여기서 HTTP status만 사용하고,
ResultCall은 payloadstatus를 반영합니다. 동일 API가 호출 경로(Flow vs Result)에 따라 다른RunnectException.code를 만들 수 있어 처리 분기가 흔들릴 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/runnect/runnect/data/network/calladapter/flow/FlowCallAdapter.kt` around lines 62 - 73, parseErrorResponse currently uses only the HTTP status to set RunnectException.code, which diverges from ResultCall's behavior that prefers the payload "status"; update parseErrorResponse to parse the errorBody JSON for a numeric "status" field and use that value as the RunnectException code when present and valid, falling back to response.code() otherwise, and still extract "message"/"error" for the exception message; touch the parseErrorResponse function and RunnectException creation logic so FlowCall and ResultCall produce consistent exception codes.app/src/main/java/com/runnect/runnect/data/network/calladapter/ResultCall.kt (1)
50-63:FlowCallAdapter와 에러 파싱 정책을 공통화하는 것을 권장합니다.현재
ResultCall은 payloadstatus를 사용하고,FlowCallAdapter는 HTTP status를 사용해 동일 API라도 예외 코드가 달라질 수 있습니다. 공통 유틸로 합치면 정책 드리프트를 막을 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/runnect/runnect/data/network/calladapter/ResultCall.kt` around lines 50 - 63, The parseErrorResponse implementation in ResultCall (function parseErrorResponse) uses the payload "status" while FlowCallAdapter uses the HTTP status, causing policy drift; extract this parsing logic into a shared utility (e.g., an ErrorParser or ErrorResponseUtil with a function like parseError(response: Response<*>) : RunnectException) and update both ResultCall.parseErrorResponse and FlowCallAdapter to call that single utility so they share the same precedence (decide and document whether payload.status overrides HTTP status or vice versa) and return a consistent RunnectException; ensure the util handles null/invalid bodies the same way as the current getOrElse fallback (use response.code() and ERROR_MSG_COMMON).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@app/src/main/java/com/runnect/runnect/data/network/calladapter/flow/FlowCallAdapter.kt`:
- Around line 67-69: The code in FlowCallAdapter.kt that sets val message from
jsonObject["message"] or jsonObject["error"] can yield "null" or empty strings;
normalize by extracting the raw content, trimming it, and only using it if
non-blank and not the literal "null", otherwise fall back to the other field or
ERROR_MSG_COMMON — update the selection logic around
jsonObject["message"]?.jsonPrimitive?.content and
jsonObject["error"]?.jsonPrimitive?.content to apply .trim(), a check for
isNotBlank() and != "null" before accepting the value so that MESSAGE uses a
sanitized string or ERROR_MSG_COMMON as fallback.
In
`@app/src/main/java/com/runnect/runnect/data/network/calladapter/ResultCall.kt`:
- Around line 54-59: The current parsing of errorString via
json.parseToJsonElement and subsequent extraction of status/message is brittle:
any parsing error or a single malformed field causes the whole getOrElse to
swallow server details; also values like "null" or empty strings can be returned
as real messages. Update the block around
json.parseToJsonElement(errorString.orEmpty())/jsonObject and the status/message
extraction so you first try-catch JSON parsing, verify the element is a
JsonObject, then individually extract status by checking the presence of
"status" and using jsonPrimitive.intOrNull or toIntOrNull with a fallback to
response.code(), and extract message by checking "message" then "error",
ensuring the jsonPrimitive is a non-empty, non-"null" string before using it; if
parsing fails or values are invalid, fall back to ERROR_MSG_COMMON, and
construct RunnectException(code = status, message = message) with those robustly
derived values.
In
`@app/src/main/java/com/runnect/runnect/data/network/interceptor/ResponseInterceptor.kt`:
- Around line 45-46: The current isBaseResponse check in ResponseInterceptor.kt
incorrectly requires the "data" key, which skips unwrapping when a valid
BaseResponse has a null/omitted data field; update isBaseResponse to detect
BaseResponse by its other identifiers (e.g., status/code/message fields) and not
require "data", and in the unwrapping logic (where jsonObject["data"].toString()
is used) safely read the data with a nullable-safe/optional access (e.g.,
opt/getNullable) so if "data" is absent you treat it as null/JsonNull (returning
a null payload) rather than skipping unwrapping—ensure references:
isBaseResponse(...) and the jsonObject["data"] unwrap point are adjusted
accordingly.
---
Nitpick comments:
In
`@app/src/main/java/com/runnect/runnect/data/network/calladapter/flow/FlowCallAdapter.kt`:
- Around line 62-73: parseErrorResponse currently uses only the HTTP status to
set RunnectException.code, which diverges from ResultCall's behavior that
prefers the payload "status"; update parseErrorResponse to parse the errorBody
JSON for a numeric "status" field and use that value as the RunnectException
code when present and valid, falling back to response.code() otherwise, and
still extract "message"/"error" for the exception message; touch the
parseErrorResponse function and RunnectException creation logic so FlowCall and
ResultCall produce consistent exception codes.
In
`@app/src/main/java/com/runnect/runnect/data/network/calladapter/ResultCall.kt`:
- Around line 50-63: The parseErrorResponse implementation in ResultCall
(function parseErrorResponse) uses the payload "status" while FlowCallAdapter
uses the HTTP status, causing policy drift; extract this parsing logic into a
shared utility (e.g., an ErrorParser or ErrorResponseUtil with a function like
parseError(response: Response<*>) : RunnectException) and update both
ResultCall.parseErrorResponse and FlowCallAdapter to call that single utility so
they share the same precedence (decide and document whether payload.status
overrides HTTP status or vice versa) and return a consistent RunnectException;
ensure the util handles null/invalid bodies the same way as the current
getOrElse fallback (use response.code() and ERROR_MSG_COMMON).
In
`@app/src/main/java/com/runnect/runnect/data/network/interceptor/ResponseInterceptor.kt`:
- Around line 47-49: The catch block in ResponseInterceptor (inside the
intercept method of class ResponseInterceptor) currently swallows exceptions by
returning null; instead log the exception type and message at debug level before
returning null to preserve minimal diagnostic visibility (do not log the full
response body). Update the catch (e: Exception) branch to call your logger/debug
utility (e.g., using the interceptor's logger or Android Log/Timbre) to record
e::class.simpleName and e.message, then return null as before.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0d716246-d522-4c0a-993e-1df3b37a371e
📒 Files selected for processing (3)
app/src/main/java/com/runnect/runnect/data/network/calladapter/ResultCall.ktapp/src/main/java/com/runnect/runnect/data/network/calladapter/flow/FlowCallAdapter.ktapp/src/main/java/com/runnect/runnect/data/network/interceptor/ResponseInterceptor.kt
Comment on lines
+54
to
+59
| val jsonObject = json.parseToJsonElement(errorString.orEmpty()).jsonObject | ||
| val status = jsonObject["status"]?.jsonPrimitive?.int ?: response.code() | ||
| val message = jsonObject["message"]?.jsonPrimitive?.content | ||
| ?: jsonObject["error"]?.jsonPrimitive?.content | ||
| ?: ERROR_MSG_COMMON | ||
| RunnectException(code = status, message = message) |
There was a problem hiding this comment.
에러 필드 추출을 조금 더 견고하게 처리해주세요.
Line 55-58에서 status/message 파싱 중 하나만 어긋나도 전체가 getOrElse로 빠져 실제 서버 메시지를 잃을 수 있습니다. "null"/빈 문자열도 그대로 메시지로 노출될 수 있습니다.
🔧 Proposed fix
- val status = jsonObject["status"]?.jsonPrimitive?.int ?: response.code()
- val message = jsonObject["message"]?.jsonPrimitive?.content
- ?: jsonObject["error"]?.jsonPrimitive?.content
- ?: ERROR_MSG_COMMON
+ val status = jsonObject["status"]?.jsonPrimitive?.content
+ ?.toIntOrNull()
+ ?: response.code()
+ val message = sequenceOf(
+ jsonObject["message"]?.jsonPrimitive?.content,
+ jsonObject["error"]?.jsonPrimitive?.content
+ ).firstOrNull { !it.isNullOrBlank() && it != "null" }
+ ?: ERROR_MSG_COMMON
RunnectException(code = status, message = message)📝 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 jsonObject = json.parseToJsonElement(errorString.orEmpty()).jsonObject | |
| val status = jsonObject["status"]?.jsonPrimitive?.int ?: response.code() | |
| val message = jsonObject["message"]?.jsonPrimitive?.content | |
| ?: jsonObject["error"]?.jsonPrimitive?.content | |
| ?: ERROR_MSG_COMMON | |
| RunnectException(code = status, message = message) | |
| val status = jsonObject["status"]?.jsonPrimitive?.content | |
| ?.toIntOrNull() | |
| ?: response.code() | |
| val message = sequenceOf( | |
| jsonObject["message"]?.jsonPrimitive?.content, | |
| jsonObject["error"]?.jsonPrimitive?.content | |
| ).firstOrNull { !it.isNullOrBlank() && it != "null" } | |
| ?: ERROR_MSG_COMMON | |
| RunnectException(code = status, message = message) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/com/runnect/runnect/data/network/calladapter/ResultCall.kt`
around lines 54 - 59, The current parsing of errorString via
json.parseToJsonElement and subsequent extraction of status/message is brittle:
any parsing error or a single malformed field causes the whole getOrElse to
swallow server details; also values like "null" or empty strings can be returned
as real messages. Update the block around
json.parseToJsonElement(errorString.orEmpty())/jsonObject and the status/message
extraction so you first try-catch JSON parsing, verify the element is a
JsonObject, then individually extract status by checking the presence of
"status" and using jsonPrimitive.intOrNull or toIntOrNull with a fallback to
response.code(), and extract message by checking "message" then "error",
ensuring the jsonPrimitive is a non-empty, non-"null" string before using it; if
parsing fails or values are invalid, fall back to ERROR_MSG_COMMON, and
construct RunnectException(code = status, message = message) with those robustly
derived values.
Collaborator
Author
There was a problem hiding this comment.
현재 코드 유지
1번 코멘트와 동일한 사유입니다. 기존 Gson 코드의 에러 파싱 동작을 그대로 유지하는 것이 이 PR의 목적이며, 정규화 로직 추가는 범위 밖입니다.
status 파싱에 대해: jsonPrimitive.int는 JSON 값이 숫자가 아닐 때 예외를 던지고, runCatching이 이를 잡아서 response.code()로 폴백합니다. 기존 Gson 코드에서 errorBody.status가 실패할 때 getOrElse로 빠지는 것과 동일한 동작입니다.
Collaborator
Author
|
bugfix/replace-gson-with-kotlinx 브랜치로 대체 (kotlin.Result 규칙 포함) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
작업 배경
R8 난독화(#379) 적용 후 릴리즈 빌드에서 모든 API 응답이 실패하는 버그가 발생했다.
원인:
ResponseInterceptor,ResultCall,FlowCallAdapter에서Gson()을 직접 생성하여BaseResponse/ErrorResponse를 역직렬화하고 있었는데, 이 DTO 클래스들은@Serializable+@SerialName(Kotlin Serialization)으로만 선언되어 있어 Gson이 인식하는@SerializedName이 없었다. R8이 필드명을 난독화하면 Gson의 리플렉션 기반 매핑이 깨져서 모든 응답 파싱이 실패했다.근본 원인: 하나의 DTO 클래스를 두 개의 직렬화 라이브러리(Gson + Kotlin Serialization)에서 혼용하는 구조적 문제.
변경 사항
ResponseInterceptor.ktGson().fromJson(body, BaseResponse::class.java)→Json.parseToJsonElement(body).jsonObject로 교체. BaseResponse의data필드를 JsonObject에서 직접 추출ResultCall.ktGson().fromJson(errorJson, ErrorResponse::class.java)→Json.parseToJsonElement()로 에러 응답 파싱 교체FlowCallAdapter.kt솔루션 선택 근거
3가지 대안 중 Kotlin Serialization 전환을 선택:
@SerializedName이중 부착-keepclassmembers class ...dto.** { <fields>; }장점 및 기대효과
1. R8 난독화 안전성 확보
2. 직렬화 라이브러리 단일화
new Gson()) 제거@SerializedName/@SerialName이중 선언 불필요3. 성능 개선
4. 유지보수 비용 절감
영향 범위
ResponseInterceptor— 모든 성공 응답의 BaseResponse 언래핑에 영향ResultCall—RetrofitV2인스턴스를 사용하는 API의 에러 응답 파싱에 영향FlowCallAdapter—RetrofitFlow인스턴스를 사용하는 API의 에러 응답 파싱에 영향GsonConverterFactory자체는 그대로 유지 (DTO 본문 직렬화는 변경 없음)Test Plan
./gradlew assembleRelease빌드 성공🤖 Generated with Claude Code
Summary by CodeRabbit