[Refactor] Response Handler 적용 #224 #235
Conversation
- HealthService의 getDailyHealth 함수 반환 타입을 `Response<HealthResponseDto>`로 수정 - Ktorfit의 `Response`를 사용하여 API 호출의 성공/실패 여부를 명확하게 처리하도록 변경
WalkthroughHTTP 응답 처리를 중앙화하는 리팩토링: 여러 서비스 호출의 수동 Changes
Sequence Diagram(s)(생성 조건 미충족 — 생략) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
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
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 |
ikseong00
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In
`@app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/EldersHealthInfoRepositoryImpl.kt`:
- Around line 45-51: The call to elderRegisterService.postElderHealthInfo(...)
uses handleResponse(), which fails because postElderHealthInfo returns
Response<Unit> (204 No Content with null body); replace the call to
handleResponse() with handleNullableResponse() so a null body is treated as a
successful response, i.e. update the invocation on the result of
elderRegisterService.postElderHealthInfo(elderInfo.elderId, elder) to use
handleNullableResponse(), keeping the surrounding code
(ElderHealthRegisterRequestDto construction and refresh() call) unchanged.
In
`@app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/SetCallRepositoryImpl.kt`:
- Around line 15-20: saveForElder is calling
service.saveCareCallTimes(...).handleResponse(), which throws on 204 No Content
because ResponseHandler expects a non-null body; change the call to use
handleNullableResponse() instead (i.e., replace handleResponse() with
handleNullableResponse() in saveForElder) or update the ResponseHandler to treat
Unit/empty bodies as success (special-case when T is Unit or
response.code()==204) so a null body doesn't raise error; specifically look for
saveForElder, service.saveCareCallTimes, handleResponse, handleNullableResponse
and adjust the call or ResponseHandler logic accordingly.
In `@app/src/main/java/com/konkuk/medicarecall/data/util/ResponseHandler.kt`:
- Around line 6-11: handleResponse() currently throws when body() is null, which
breaks endpoints that return Response<Unit> on 204 No Content; update
handleResponse (or replace calls with handleNullableResponse) to treat null body
as valid when the response code is 204 or when T is Unit: in the success branch,
if body() != null return it, otherwise if (code() == 204 || T represents
kotlin.Unit) return Unit cast to T, else throw the existing error; apply this
change in the handleResponse function used by
SetCallService.saveCareCallTimes(), HomeService.requestImmediateCareCall(),
EldersInfoService.deleteElderSettings(), ElderRegisterService methods,
AuthService.requestCertificationCode()/logout(), and FcmService methods (or
switch those callers to handleNullableResponse()).
In `@REFACTORING_ANALYSIS.md`:
- Around line 41-62: The markdown code fences in REFACTORING_ANALYSIS.md are
missing language identifiers (violates MD040); update each fenced block (e.g.,
the project tree blocks shown in the diff) to add an appropriate language tag
such as ```text or ```kotlin/```gradle where applicable, ensuring opening fences
include the language and closing fences remain ```; apply the same change to the
other occurrences called out in the comment (around the blocks at lines ~71-74,
~171-181, and ~282-286) so all fenced code blocks have language specifiers.
- Around line 26-33: The Markdown table block lacks surrounding blank lines,
violating MD058; add one empty line before the table start ("| 항목 | 수치 |") and
one empty line after the table end ("| 테스트 코드 | 3개 (극히 부족) |") so the table is
separated from surrounding paragraphs and complies with markdownlint.
| val elder = ElderHealthRegisterRequestDto( | ||
| diseaseNames = elderInfo.diseases, | ||
| medicationSchedules = medicationSchedule, | ||
| notes = elderInfo.notes, | ||
| ) | ||
| elderRegisterService.postElderHealthInfo(elderInfo.elderId, elder).handleResponse() | ||
| refresh() |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# ResponseHandler.kt 내용 확인
find . -name "ResponseHandler.kt" -type f | head -5
# 경로가 보이면 내용 확인
find . -path "*/data/util/ResponseHandler.kt" -type fRepository: Medicare-Call/Medicare-Call-Android
Length of output: 224
🏁 Script executed:
#!/bin/bash
# 서비스 메서드 시그니처 찾기
find . -name "*.kt" -path "*/api/*" | xargs grep -l "postElderHealthInfo" | head -3Repository: Medicare-Call/Medicare-Call-Android
Length of output: 162
🏁 Script executed:
cat -n app/src/main/java/com/konkuk/medicarecall/data/util/ResponseHandler.ktRepository: Medicare-Call/Medicare-Call-Android
Length of output: 711
🏁 Script executed:
cat -n app/src/main/java/com/konkuk/medicarecall/data/api/elders/ElderRegisterService.ktRepository: Medicare-Call/Medicare-Call-Android
Length of output: 1681
🏁 Script executed:
# 현재 EldersHealthInfoRepositoryImpl.kt 에서 postElderHealthInfo 호출 부분 확인
cat -n app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/EldersHealthInfoRepositoryImpl.kt | sed -n '40,55p'Repository: Medicare-Call/Medicare-Call-Android
Length of output: 886
postElderHealthInfo의 Unit 응답에서 handleResponse() 사용 시 버그 발생
postElderHealthInfo는 Response을 반환하는데, 성공적인 204 No Content 응답은 body가 null입니다. 현재 handleResponse()는 body() ?: error("응답값이 null 입니다.")로 구현되어 있어서, 성공한 요청이 오류로 떨어지게 됩니다. handleNullableResponse()로 변경하면 null body를 안전하게 처리할 수 있습니다.
수정 예시
- elderRegisterService.postElderHealthInfo(elderInfo.elderId, elder).handleResponse()
+ elderRegisterService.postElderHealthInfo(elderInfo.elderId, elder).handleNullableResponse()📝 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.
| val elder = ElderHealthRegisterRequestDto( | |
| diseaseNames = elderInfo.diseases, | |
| medicationSchedules = medicationSchedule, | |
| notes = elderInfo.notes, | |
| ) | |
| elderRegisterService.postElderHealthInfo(elderInfo.elderId, elder).handleResponse() | |
| refresh() | |
| val elder = ElderHealthRegisterRequestDto( | |
| diseaseNames = elderInfo.diseases, | |
| medicationSchedules = medicationSchedule, | |
| notes = elderInfo.notes, | |
| ) | |
| elderRegisterService.postElderHealthInfo(elderInfo.elderId, elder).handleNullableResponse() | |
| refresh() |
🤖 Prompt for AI Agents
In
`@app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/EldersHealthInfoRepositoryImpl.kt`
around lines 45 - 51, The call to elderRegisterService.postElderHealthInfo(...)
uses handleResponse(), which fails because postElderHealthInfo returns
Response<Unit> (204 No Content with null body); replace the call to
handleResponse() with handleNullableResponse() so a null body is treated as a
successful response, i.e. update the invocation on the result of
elderRegisterService.postElderHealthInfo(elderInfo.elderId, elder) to use
handleNullableResponse(), keeping the surrounding code
(ElderHealthRegisterRequestDto construction and refresh() call) unchanged.
| override suspend fun saveForElder( | ||
| elderId: Int, | ||
| body: SetCallTimeRequestDto, | ||
| ): Result<Unit> = | ||
| runCatching { | ||
| val response = service.saveCareCallTimes(elderId, body) | ||
| if (!response.isSuccessful) { | ||
| Log.e("SetCallRepository", "HTTP ${response.code} ${response.message}") | ||
| Log.e("SetCallRepository", "ErrorBody=${response.errorBody()?.toString()}") | ||
| throw HttpException(response) | ||
| } | ||
| } | ||
| ): Result<Unit> = runCatching { | ||
| service.saveCareCallTimes(elderId, body).handleResponse() | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat app/src/main/java/com/konkuk/medicarecall/data/util/ResponseHandler.ktRepository: Medicare-Call/Medicare-Call-Android
Length of output: 564
🏁 Script executed:
# Verify that saveCareCallTimes is expected to return 204
rg -n "saveCareCallTimes" app/src/main/java/com/konkuk/medicarecall/data/ -A 3 -B 3Repository: Medicare-Call/Medicare-Call-Android
Length of output: 1690
handleResponse() 사용하면 204 응답에서 에러 발생함!
saveCareCallTimes가 204 No Content를 반환할 때 handleResponse()는 확실히 터진다. ResponseHandler를 보니 body() ?: error("응답값이 null 입니다.")라고 되어 있는데, 204는 성공(isSuccessful = true)이지만 body가 null이라서 무조건 에러가 던져진다.
handleNullableResponse()로 바꾸거나 ResponseHandler에서 Unit 타입을 특별 처리해야 한다.
🤖 Prompt for AI Agents
In
`@app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/SetCallRepositoryImpl.kt`
around lines 15 - 20, saveForElder is calling
service.saveCareCallTimes(...).handleResponse(), which throws on 204 No Content
because ResponseHandler expects a non-null body; change the call to use
handleNullableResponse() instead (i.e., replace handleResponse() with
handleNullableResponse() in saveForElder) or update the ResponseHandler to treat
Unit/empty bodies as success (special-case when T is Unit or
response.code()==204) so a null body doesn't raise error; specifically look for
saveForElder, service.saveCareCallTimes, handleResponse, handleNullableResponse
and adjust the call or ResponseHandler logic accordingly.
| fun <T> Response<T>.handleResponse(): T { | ||
| if (isSuccessful) { | ||
| return body() ?: error("응답값이 null 입니다.") | ||
| } else { | ||
| throw HttpException(this) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "*.kt" | grep -E "(Response|API|Service)" | head -20Repository: Medicare-Call/Medicare-Call-Android
Length of output: 1640
🏁 Script executed:
cd app/src/main/java/com/konkuk/medicarecall && find . -type f -name "*.kt" | head -30Repository: Medicare-Call/Medicare-Call-Android
Length of output: 1155
🏁 Script executed:
cat -n app/src/main/java/com/konkuk/medicarecall/data/util/ResponseHandler.ktRepository: Medicare-Call/Medicare-Call-Android
Length of output: 711
🏁 Script executed:
# Search for usages of handleResponse
rg "handleResponse" app/src/main/java/com/konkuk/medicarecall/ -A 2 -B 2Repository: Medicare-Call/Medicare-Call-Android
Length of output: 24562
🏁 Script executed:
# Search for API interfaces and response definitions
fd ".*API.kt|.*Service.kt|.*Repository.kt" app/src/main/java/com/konkuk/medicarecall/Repository: Medicare-Call/Medicare-Call-Android
Length of output: 3605
🏁 Script executed:
# Look for handleNullableResponse mentioned in the review
rg "handleNullableResponse" app/src/main/java/com/konkuk/medicarecall/Repository: Medicare-Call/Medicare-Call-Android
Length of output: 200
🏁 Script executed:
# Check services that use Unit return type
rg "Response<Unit>" app/src/main/java/com/konkuk/medicarecall/data/api/Repository: Medicare-Call/Medicare-Call-Android
Length of output: 1165
🏁 Script executed:
# Look at SetCallService since it returns Unit
cat -n app/src/main/java/com/konkuk/medicarecall/data/api/elders/SetCallService.ktRepository: Medicare-Call/Medicare-Call-Android
Length of output: 694
🏁 Script executed:
# Check HomeService for the immediate care call endpoint
cat -n app/src/main/java/com/konkuk/medicarecall/data/api/elders/HomeService.ktRepository: Medicare-Call/Medicare-Call-Android
Length of output: 940
🏁 Script executed:
# Look at EldersInfoService
cat -n app/src/main/java/com/konkuk/medicarecall/data/api/elders/EldersInfoService.ktRepository: Medicare-Call/Medicare-Call-Android
Length of output: 1901
🌐 Web query:
Ktorfit Response<Unit> body() null behavior 204 No Content
💡 Result:
With HTTP 204 No Content, there is no response payload, so in Ktorfit a Response<Unit> often ends up with body() being null (or a conversion error if something tries to deserialize an empty body). Ktor’s response-body pipeline explicitly allows returning null when there’s nothing to transform/deserialize. [1]
What to do:
- Don’t read the body for 204: treat success based on
response.status/response.isSuccessfuland ignorebody(). - If you want to represent “maybe there is a body”, change the type to
Response<Unit?>(nullable). - If you’re on a recent Ktor, prefer a nullable-body API:
bodyNullable(...)returnsAny?explicitly (useful for empty/204 bodies). [2] - If you have a custom Ktorfit converter/wrapper that calls
response.body(...), make it 204-safe (skip parsing or use a nullable-body approach), because Ktorfit converters commonly delegate toresponse.body(typeInfo). [3]
Sources:
[1] Ktor transformResponseBody docs (transformers may return null) (api.ktor.io)
[2] Ktor bodyNullable API (api.ktor.io)
[3] Ktorfit converter example calling response.body(info) (foso.github.io)
Citations:
- 1: https://api.ktor.io/ktor-client/ktor-client-core/io.ktor.client.plugins.api/-client-plugin-builder/transform-response-body.html?utm_source=openai
- 2: https://api.ktor.io/ktor-client/ktor-client-core/io.ktor.client.call/-http-client-call/body-nullable.html?utm_source=openai
- 3: https://foso.github.io/Ktorfit/responseconverter/?utm_source=openai
Response 엔드포인트에서 204 No Content 처리 필수
handleResponse()를 사용하는 여러 API 엔드포인트가 Response<Unit>을 반환합니다. 서버가 204 No Content로 응답하면 Ktorfit의 body()는 null을 반환하기 때문에 line 8에서 예외가 발생합니다.
현재 코드가 영향을 받는 엔드포인트:
- SetCallService.saveCareCallTimes()
- HomeService.requestImmediateCareCall()
- EldersInfoService.deleteElderSettings()
- ElderRegisterService (여러 메서드)
- AuthService.requestCertificationCode(), logout()
- FcmService 관련 메서드들
기존의 handleNullableResponse()를 사용하거나, Unit 타입을 명시적으로 허용하는 로직을 추가해야 합니다.
🤖 Prompt for AI Agents
In `@app/src/main/java/com/konkuk/medicarecall/data/util/ResponseHandler.kt`
around lines 6 - 11, handleResponse() currently throws when body() is null,
which breaks endpoints that return Response<Unit> on 204 No Content; update
handleResponse (or replace calls with handleNullableResponse) to treat null body
as valid when the response code is 204 or when T is Unit: in the success branch,
if body() != null return it, otherwise if (code() == 204 || T represents
kotlin.Unit) return Unit cast to T, else throw the existing error; apply this
change in the handleResponse function used by
SetCallService.saveCareCallTimes(), HomeService.requestImmediateCareCall(),
EldersInfoService.deleteElderSettings(), ElderRegisterService methods,
AuthService.requestCertificationCode()/logout(), and FcmService methods (or
switch those callers to handleNullableResponse()).
| | 항목 | 수치 | | ||
| |------|------| | ||
| | 총 코드 라인 | 22,855줄 | | ||
| | Kotlin 파일 | 293개 | | ||
| | ViewModel | 18개 | | ||
| | Repository | 22개 | | ||
| | 테스트 코드 | 3개 (극히 부족) | | ||
|
|
There was a problem hiding this comment.
테이블 전후에 빈 줄을 추가해주세요.
Markdownlint(MD058) 규칙 위반이라 렌더링/툴 호환성에 걸릴 수 있어요.
🛠️ 제안 수정
-### 1.1 프로젝트 규모
-| 항목 | 수치 |
-|------|------|
-| 총 코드 라인 | 22,855줄 |
-| Kotlin 파일 | 293개 |
-| ViewModel | 18개 |
-| Repository | 22개 |
-| 테스트 코드 | 3개 (극히 부족) |
+### 1.1 프로젝트 규모
+
+| 항목 | 수치 |
+|------|------|
+| 총 코드 라인 | 22,855줄 |
+| Kotlin 파일 | 293개 |
+| ViewModel | 18개 |
+| Repository | 22개 |
+| 테스트 코드 | 3개 (극히 부족) |
+🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
26-26: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
🤖 Prompt for AI Agents
In `@REFACTORING_ANALYSIS.md` around lines 26 - 33, The Markdown table block lacks
surrounding blank lines, violating MD058; add one empty line before the table
start ("| 항목 | 수치 |") and one empty line after the table end ("| 테스트 코드 | 3개 (극히
부족) |") so the table is separated from surrounding paragraphs and complies with
markdownlint.
| ``` | ||
| ├── data | ||
| │ ├── api # Retrofit/Ktorfit 서비스 | ||
| │ ├── di # Dependency Injection | ||
| │ ├── dto # Request/Response DTO | ||
| │ ├── network # Interceptor, Authenticator | ||
| │ ├── repository # 인터페이스 (22개) | ||
| │ ├── repositoryimpl # 구현체 (22개) | ||
| │ └── util | ||
| ├── domain | ||
| │ └── usecase | ||
| ├── ui | ||
| │ ├── feature # 화면별 모듈 | ||
| │ │ ├── home | ||
| │ │ ├── login | ||
| │ │ ├── statistics | ||
| │ │ ├── settings | ||
| │ │ └── homedetail | ||
| │ ├── common # 공통 컴포넌트 | ||
| │ ├── navigation # NavGraph | ||
| │ └── theme | ||
| ``` |
There was a problem hiding this comment.
코드 펜스에 언어 지정해주세요.
Markdownlint(MD040) 위반입니다. 예: kotlin, gradle, ```text 등.
🛠️ 예시 수정
-```
+```text
├── data
│ ├── api # Retrofit/Ktorfit 서비스
...
│ └── theme
-```
+```Also applies to: 71-74, 171-181, 282-286
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
41-41: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In `@REFACTORING_ANALYSIS.md` around lines 41 - 62, The markdown code fences in
REFACTORING_ANALYSIS.md are missing language identifiers (violates MD040);
update each fenced block (e.g., the project tree blocks shown in the diff) to
add an appropriate language tag such as ```text or ```kotlin/```gradle where
applicable, ensuring opening fences include the language and closing fences
remain ```; apply the same change to the other occurrences called out in the
comment (around the blocks at lines ~71-74, ~171-181, and ~282-286) so all
fenced code blocks have language specifiers.
- `HealthRepositoryImpl`에서 `handleResponse()` 유틸 함수를 사용하여 API 응답을 처리하도록 수정 - 코드 중복을 줄이고 일관성 있는 오류 처리 방식 적용
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/HealthRepositoryImpl.kt (1)
18-20:symptomList에!!사용 시 NPE 가능성 있어요Line 18에서는
orEmpty()로 null을 안전하게 처리하는데, Line 20에서는 같은 필드에!!를 사용하고 있어서 일관성이 없고symptomList가 null일 경우 NPE가 발생합니다.🔧 제안하는 수정
HealthUiState( symptoms = response.symptomList.orEmpty(), symptomAnalysis = response.analysisComment.orEmpty(), - isRecorded = response.symptomList!!.isNotEmpty() || !response.analysisComment.isNullOrBlank(), + isRecorded = !response.symptomList.isNullOrEmpty() || !response.analysisComment.isNullOrBlank(), )
✏️ 변경사항
📷 스크린샷
N/A
✍️ 사용법
🎸 기타
Summary by CodeRabbit
릴리스 노트
Refactor
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.