-
Notifications
You must be signed in to change notification settings - Fork 0
[Refactor] 온보딩 Ktorfit 마이그레이션 #216 #221
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
TODO: HttpException -> 커스텀 Exception? or else...
WalkthroughRetrofit에서 Ktorfit로 마이그레이션: API 인터페이스의 HTTP 어노테이션·Response 임포트가 Ktorfit로 변경되고, 리포지토리의 Retrofit HttpException 처리 경로가 제거되어 일반/커스텀 예외로 대체됩니다. 빌드 의존성에서 Ktor/Ktorfit 항목이 삭제되었습니다. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In
@app/src/main/java/com/konkuk/medicarecall/data/api/elders/EldersInfoService.kt:
- Around line 14-15: EldersInfoService.kt에 불필요한 연속 공백(빈) 라인으로 인해 린트 오류가 발생하고
있습니다; 파일의 14번째 빈 줄을 삭제하여 연속된 공백을 제거하고 파일 내 클래스/인터페이스 선언(EldersInfoService) 주위의
여분 공백이 남아있지 않은지 확인해 주세요.
In @app/src/main/java/com/konkuk/medicarecall/data/api/elders/SetCallService.kt:
- Around line 4-7: Several repository implementations still use HttpException
and inconsistent try/runCatching patterns; migrate them to the same
error-handling approach used by SetCallRepositoryImpl and
ElderRegisterRepositoryImpl. Update EldersHealthInfoRepositoryImpl,
EldersInfoRepositoryImpl, GlucoseRepositoryImpl, HomeRepositoryImpl to stop
importing/throwing de.ktor.client.features.HttpException, instead catch
network/IO exceptions (or let them surface) and wrap/map errors to the shared
repository error strategy (e.g., use runCatching { ... } .getOrElse { throw
RepositoryException(it) } or return Result types consistently), remove
HttpException imports, and standardize on the single pattern (runCatching +
mapping or unified RepositoryException) across all repository classes.
In
@app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/MemberRegisterRepositoryImpl.kt:
- Around line 36-37: Replace the generic Exception throw and the unused
errorBody assignment with a custom ApiException: create a class ApiException(val
statusCode: Int, val errorBody: String?, message: String) : Exception(message),
then in MemberRegisterRepositoryImpl use the response code and the actual error
body (e.g. val errorBody = response.errorBody()?.string() ?: "Unknown error")
and throw ApiException(response.code(), errorBody, "API error:
${response.message()}") instead of throw Exception(response.toString()); ensure
errorBody is consumed (not left unused) and provide meaningful message and
statusCode to callers.
In
@app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/SetCallRepositoryImpl.kt:
- Around line 21-23: In SetCallRepositoryImpl, replace the incorrect
errorBody()?.toString() usage with errorBody()?.string() so the actual HTTP
error payload is logged (update the Log.e call that currently logs
"ErrorBody=..."), and instead of throwing a generic
Exception(response.toString()), throw an HttpException(response) to preserve
HTTP error details; locate the logging and throw site in the method inside
SetCallRepositoryImpl where response is checked and update those two uses
accordingly.
In
@app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/SubscribeRepositoryImpl.kt:
- Line 18: The error handling in SubscribeRepositoryImpl uses
response.errorBody()?.toString() and throws a generic Exception; change it to
extract the body as a string like val error = response.errorBody()?.string() ?:
"Unknown error", log it with Log.e("SubscribeRepository", "응답 실패: $error"), and
throw an HttpException(response) instead of Exception to match the pattern used
in EldersInfoRepositoryImpl/GlucoseRepositoryImpl and ensure consistent error
handling.
In
@app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/VerificationRepositoryImpl.kt:
- Line 26: Replace the generic Exception thrown in VerificationRepositoryImpl
(the place that currently does throw Exception(response.toString())) with an
HttpException constructed from the retrofit Response object (throw
HttpException(response)) and add the necessary import for
retrofit2.HttpException so callers can inspect HTTP status codes consistently
like other repos (EldersInfoRepositoryImpl, UserRepositoryImpl); keep the
existing null-body handling (response.body() ?: error(...)) for successful
responses.
In
@app/src/main/java/com/konkuk/medicarecall/ui/feature/login/info/viewmodel/LoginViewModel.kt:
- Line 81: The onSuccess handler in LoginViewModel is incorrectly accessing
it.message on a Ktorfit Response<Unit>; replace that with a valid Response
property (e.g., it.status) or log a fixed success message (or use it.body() if
returning a body) in the .onSuccess { ... } callback where currently
Log.d("httplog", "인증번호 요청 성공, ${it.message}") is used so it no longer references
a nonexistent message property.
🧹 Nitpick comments (4)
app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/ElderRegisterRepositoryImpl.kt (1)
40-41: 에러 처리 방식의 일관성은 좋으나 개선이 필요합니다.코드 내
// Exception Type?코멘트에서 이미 인지하고 계신 것처럼, 현재 GenericException사용은 임시 방편으로 보입니다.마이그레이션 완료 후 다음을 고려해보세요:
- 도메인 특화 예외 클래스 도입 (예:
ApiException,NetworkException)errorBody()?.toString()대신 실제 에러 내용을 파싱하는 방식 적용- HTTP 상태 코드별 예외 타입 분리 (4xx vs 5xx)
현재 단계에서는 마이그레이션 우선이므로 괜찮지만, 추후 이슈로 트래킹하시는 게 좋을 것 같습니다.
커스텀 예외 클래스 구조를 제안해드릴까요? 혹은 별도 이슈로 생성하는 것을 도와드릴 수 있습니다.
Also applies to: 57-58, 80-80, 105-105
app/build.gradle.kts (3)
8-8: 플러그인 버전을 버전 카탈로그와 일치시키세요.Kotlin serialization 플러그인 버전이 하드코딩되어 있는데,
gradle/libs.versions.toml에도 kotlin 버전이 정의되어 있습니다. 일관성을 위해 카탈로그의 버전을 사용하는 것이 좋습니다.♻️ 제안하는 수정
- kotlin("plugin.serialization") version "2.2.20" + alias(libs.plugins.kotlin.serialization)그리고
gradle/libs.versions.toml의[plugins]섹션에 다음을 추가하세요:kotlin-serialization = { id = "org.jetbrains.kotlin.plugin.serialization", version.ref = "kotlin" }
11-11: Ktorfit 플러그인 버전도 버전 카탈로그로 관리하는 것을 고려해보세요.Ktorfit 플러그인 버전이 하드코딩되어 있습니다. 일관된 의존성 관리를 위해
gradle/libs.versions.toml에 버전을 정의하고 카탈로그를 통해 참조하는 것이 좋습니다.♻️ 제안하는 수정
gradle/libs.versions.toml의[versions]섹션에 추가:ktorfit = "2.7.1"그리고
[plugins]섹션에 추가:ktorfit = { id = "de.jensklingenberg.ktorfit", version.ref = "ktorfit" }그런 다음 이 파일에서:
- id("de.jensklingenberg.ktorfit") version "2.7.1" + alias(libs.plugins.ktorfit)
126-127: Ktorfit 의존성 버전을 버전 카탈로그로 이동하세요.Ktorfit 라이브러리들도 하드코딩된 버전을 사용하고 있습니다. 플러그인과 동일하게 버전 카탈로그로 관리하면 버전 업데이트가 용이해집니다.
♻️ 제안하는 수정
gradle/libs.versions.toml의[libraries]섹션에 추가:ktorfit-lib = { group = "de.jensklingenberg.ktorfit", name = "ktorfit-lib", version.ref = "ktorfit" } ktorfit-converters-response = { group = "de.jensklingenberg.ktorfit", name = "ktorfit-converters-response", version.ref = "ktorfit" }그런 다음 이 파일에서:
- implementation("de.jensklingenberg.ktorfit:ktorfit-lib:2.7.1") - implementation("de.jensklingenberg.ktorfit:ktorfit-converters-response:2.7.1") + implementation(libs.ktorfit.lib) + implementation(libs.ktorfit.converters.response)
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
app/build.gradle.ktsapp/src/main/java/com/konkuk/medicarecall/data/api/auth/AuthService.ktapp/src/main/java/com/konkuk/medicarecall/data/api/auth/RefreshService.ktapp/src/main/java/com/konkuk/medicarecall/data/api/elders/ElderRegisterService.ktapp/src/main/java/com/konkuk/medicarecall/data/api/elders/EldersInfoService.ktapp/src/main/java/com/konkuk/medicarecall/data/api/elders/SetCallService.ktapp/src/main/java/com/konkuk/medicarecall/data/api/elders/SubscribeService.ktapp/src/main/java/com/konkuk/medicarecall/data/api/member/MemberRegisterService.ktapp/src/main/java/com/konkuk/medicarecall/data/api/payments/NaverPayService.ktapp/src/main/java/com/konkuk/medicarecall/data/di/ApiModule.ktapp/src/main/java/com/konkuk/medicarecall/data/repository/VerificationRepository.ktapp/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/ElderRegisterRepositoryImpl.ktapp/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/MemberRegisterRepositoryImpl.ktapp/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/NaverPayRepositoryImpl.ktapp/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/SetCallRepositoryImpl.ktapp/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/SubscribeRepositoryImpl.ktapp/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/VerificationRepositoryImpl.ktapp/src/main/java/com/konkuk/medicarecall/ui/feature/login/info/viewmodel/LoginViewModel.ktgradle/libs.versions.toml
💤 Files with no reviewable changes (1)
- app/src/main/java/com/konkuk/medicarecall/data/di/ApiModule.kt
🧰 Additional context used
🪛 GitHub Actions: Android CI
app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/ElderRegisterRepositoryImpl.kt
[error] 41-41: TooGenericExceptionThrown: Exception is a too generic Exception. Prefer throwing specific exceptions that indicate a specific error case.
[error] 58-58: TooGenericExceptionThrown: Exception is a too generic Exception. Prefer throwing specific exceptions that indicate a specific error case.
[error] 80-80: TooGenericExceptionThrown: Exception is a too generic Exception. Prefer throwing specific exceptions that indicate a specific error case.
[error] 105-105: TooGenericExceptionThrown: Exception is a too generic Exception. Prefer throwing specific exceptions that indicate a specific error case.
app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/NaverPayRepositoryImpl.kt
[error] 21-21: TooGenericExceptionThrown: Exception is a too generic Exception. Prefer throwing specific exceptions that indicate a specific error case.
app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/MemberRegisterRepositoryImpl.kt
[error] 37-37: TooGenericExceptionThrown: Exception is a too generic Exception. Prefer throwing specific exceptions that indicate a specific error case.
app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/VerificationRepositoryImpl.kt
[error] 26-26: TooGenericExceptionThrown: Exception is a too generic Exception. Prefer throwing specific exceptions that indicate a specific error case.
app/src/main/java/com/konkuk/medicarecall/data/api/elders/EldersInfoService.kt
[error] 15-15: NoConsecutiveBlankLines: Needless blank line(s)
app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/SetCallRepositoryImpl.kt
[error] 23-23: TooGenericExceptionThrown: Exception is a too generic Exception. Prefer throwing specific exceptions that indicate a specific error case.
app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/SubscribeRepositoryImpl.kt
[error] 26-26: TooGenericExceptionThrown: Exception is a too generic Exception. Prefer throwing specific exceptions that indicate a specific error case.
🔇 Additional comments (12)
app/src/main/java/com/konkuk/medicarecall/data/api/auth/AuthService.kt (1)
6-9: 임포트 변경 확인됨Retrofit에서 Ktorfit으로의 임포트 변경이 깔끔하게 처리되었습니다. 메서드 시그니처도 그대로 유지되어 호환성 문제가 없습니다.
app/src/main/java/com/konkuk/medicarecall/data/repository/VerificationRepository.kt (1)
4-4: Response 타입 변경 확인됨Ktorfit의 Response 타입으로 변경이 잘 적용되었습니다.
app/src/main/java/com/konkuk/medicarecall/data/api/payments/NaverPayService.kt (1)
5-7: LGTM! 깔끔한 임포트 마이그레이션입니다.Retrofit에서 Ktorfit으로 임포트를 올바르게 변경하셨고, 인터페이스 시그니처는 동일하게 유지되어 호환성이 보장됩니다.
app/src/main/java/com/konkuk/medicarecall/data/api/elders/SubscribeService.kt (1)
4-5: LGTM! Ktorfit 마이그레이션이 올바르게 적용되었습니다.임포트만 변경되고 인터페이스는 그대로 유지되어 기존 호출 코드와의 호환성이 유지됩니다.
app/src/main/java/com/konkuk/medicarecall/data/api/elders/ElderRegisterService.kt (1)
9-12: 깔끔한 Ktorfit 마이그레이션입니다.Retrofit에서 Ktorfit으로의 임포트 교체가 정확하게 이루어졌고, 인터페이스의 공개 API는 그대로 유지되었습니다. KMP 지원을 위한 좋은 첫 걸음이네요.
app/src/main/java/com/konkuk/medicarecall/data/api/auth/RefreshService.kt (1)
4-6: LGTM!Ktorfit 임포트로의 전환이 올바르게 적용되었습니다. 메서드 시그니처도 그대로 유지되어 호환성 문제가 없습니다.
app/src/main/java/com/konkuk/medicarecall/data/api/elders/EldersInfoService.kt (1)
8-13: 임포트 마이그레이션 확인 완료.모든 Retrofit 임포트가 Ktorfit 버전으로 정확하게 교체되었습니다. 인터페이스 정의는 변경사항 없이 유지되었습니다.
app/src/main/java/com/konkuk/medicarecall/data/api/member/MemberRegisterService.kt (1)
5-8: 좋습니다!Ktorfit 라이브러리로의 마이그레이션이 정확하게 완료되었습니다. API 계약은 변경되지 않았으니 기존 호출부에 영향이 없습니다.
app/build.gradle.kts (1)
102-106: 마이그레이션 접근 방식이 합리적입니다.Retrofit 의존성을 주석 처리하는 것은 마이그레이션 중 롤백이 필요할 경우를 대비한 좋은 접근 방식입니다. 마이그레이션이 완전히 완료되고 안정화되면 이 주석들을 제거하면 됩니다.
gradle/libs.versions.toml (3)
3-3: Kotlin 버전 업데이트가 적절합니다.Kotlin 2.2.20으로의 업그레이드는 Ktorfit 지원을 위해 필요한 변경입니다.
26-27: Koin 버전 업데이트가 적절합니다.Koin 및 Koin Annotations의 버전 업데이트는 점진적이며 합리적입니다.
21-21: KSP 버전이 올바릅니다 — 특별한 조치 필요 없습니다.KSP 2.3.2는 KSP2이며, KSP2는 더 이상 특정 Kotlin 컴파일러 버전과 직결되지 않습니다. 원래 우려는 "2.2.20-1.0.XX" 형식의 구 KSP1 버전 관례에 기반한 것이지만, KSP 2.0.0부터는 Kotlin 버전과 독립적으로 작동하도록 재설계되었습니다. KSP 2.3.2는 Kotlin 2.2.20과 완벽하게 호환되므로 현재 설정은 문제없습니다.
Likely an incorrect or invalid review comment.
app/src/main/java/com/konkuk/medicarecall/data/api/elders/EldersInfoService.kt
Outdated
Show resolved
Hide resolved
| import de.jensklingenberg.ktorfit.Response | ||
| import de.jensklingenberg.ktorfit.http.Body | ||
| import de.jensklingenberg.ktorfit.http.POST | ||
| import de.jensklingenberg.ktorfit.http.Path |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Repository 레이어에서 HttpException 사용 여부 및 에러 처리 패턴 확인
echo "=== Checking for HttpException references ==="
rg -n "HttpException" --type=kt -g '!build/' -g '!.gradle/'
echo -e "\n=== Checking error handling patterns in repository implementations ==="
# Repository 구현체들의 에러 처리 패턴 확인
fd ".*\.kt$" app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/ 2>/dev/null | head -20
echo -e "\n=== Searching for catch blocks in Repository implementations ==="
rg -n -A3 "catch" app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/ --type=kt 2>/dev/null || echo "No catch blocks found or directory doesn't exist"
echo -e "\n=== Checking for try-catch patterns and exception handling ==="
rg -n "try\s*{" app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/ --type=kt 2>/dev/null || echo "No try blocks found or directory doesn't exist"Repository: Medicare-Call/Medicare-Call-Android
Length of output: 2233
🏁 Script executed:
#!/bin/bash
# Read SetCallRepositoryImpl.kt to see error handling patterns
echo "=== SetCallRepositoryImpl.kt ==="
cat app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/SetCallRepositoryImpl.kt
echo -e "\n\n=== ElderRegisterRepositoryImpl.kt (sample) ==="
head -80 app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/ElderRegisterRepositoryImpl.kt
echo -e "\n\n=== Checking for try-catch without type filter ==="
grep -n "try\|catch\|throw" app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/*.kt | head -30Repository: Medicare-Call/Medicare-Call-Android
Length of output: 10214
Repository 레이어 에러 처리가 아직 마이그레이션되지 않았습니다.
SetCallService 임포트 변경은 깔끔하게 진행되었는데, Repository 구현체들을 확인해보니 상황이 좀 복잡하네요:
EldersHealthInfoRepositoryImpl,EldersInfoRepositoryImpl,GlucoseRepositoryImpl,HomeRepositoryImpl등에서 여전히HttpException을 사용하고 있습니다SetCallRepositoryImpl,ElderRegisterRepositoryImpl은Exception으로 변경했지만, 다른 곳들은 아직도HttpException을 쓰고 있어요- 에러 처리 패턴도 일관되지 않습니다: 어떤 곳은
runCatching을, 어떤 곳은 직접throw를, 또 다른 곳은 try-catch로 처리하고 있습니다
Repository 전체에서 에러 처리 방식을 통일하고, 남은 HttpException 참조들도 마이그레이션하는 게 좋을 것 같습니다.
🤖 Prompt for AI Agents
In @app/src/main/java/com/konkuk/medicarecall/data/api/elders/SetCallService.kt
around lines 4 - 7, Several repository implementations still use HttpException
and inconsistent try/runCatching patterns; migrate them to the same
error-handling approach used by SetCallRepositoryImpl and
ElderRegisterRepositoryImpl. Update EldersHealthInfoRepositoryImpl,
EldersInfoRepositoryImpl, GlucoseRepositoryImpl, HomeRepositoryImpl to stop
importing/throwing de.ktor.client.features.HttpException, instead catch
network/IO exceptions (or let them surface) and wrap/map errors to the shared
repository error strategy (e.g., use runCatching { ... } .getOrElse { throw
RepositoryException(it) } or return Result types consistently), remove
HttpException imports, and standardize on the single pattern (runCatching +
mapping or unified RepositoryException) across all repository classes.
| val errorBody = response.errorBody()?.toString() ?: "Unknown error" | ||
| throw Exception(response.toString()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
파이프라인 실패: 제네릭 Exception 사용
현재 제네릭 Exception을 던지고 있어서 파이프라인이 실패하고 있습니다. PR 설명에서 언급하신 것처럼, Ktorfit 마이그레이션 시 HttpException 대체 방안이 필요한 부분이네요.
추가로 36번 라인의 errorBody 변수가 할당만 되고 실제로는 사용되지 않고 있습니다.
🔧 제안: 커스텀 예외 클래스 사용
Ktorfit 마이그레이션에 맞춰 커스텀 예외를 만들어서 사용하는 것을 추천드립니다. 예를 들어:
// 새로운 파일: data/exception/ApiException.kt
class ApiException(
val statusCode: Int,
val errorBody: String?,
message: String
) : Exception(message)그리고 이 파일에서는:
} else {
- val errorBody = response.errorBody()?.toString() ?: "Unknown error"
- throw Exception(response.toString())
+ val errorBody = response.errorBody()?.toString()
+ throw ApiException(
+ statusCode = response.code(),
+ errorBody = errorBody,
+ message = "Member registration failed: ${response.code()} - $errorBody"
+ )
}이렇게 하면 호출하는 쪽에서도 에러 정보를 구조적으로 접근할 수 있고, 린터 규칙도 통과할 수 있습니다.
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Actions: Android CI
[error] 37-37: TooGenericExceptionThrown: Exception is a too generic Exception. Prefer throwing specific exceptions that indicate a specific error case.
🤖 Prompt for AI Agents
In
@app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/MemberRegisterRepositoryImpl.kt
around lines 36 - 37, Replace the generic Exception throw and the unused
errorBody assignment with a custom ApiException: create a class ApiException(val
statusCode: Int, val errorBody: String?, message: String) : Exception(message),
then in MemberRegisterRepositoryImpl use the response code and the actual error
body (e.g. val errorBody = response.errorBody()?.string() ?: "Unknown error")
and throw ApiException(response.code(), errorBody, "API error:
${response.message()}") instead of throw Exception(response.toString()); ensure
errorBody is consumed (not left unused) and provide meaningful message and
statusCode to callers.
| Log.e("SetCallRepository", "HTTP ${response.code} ${response.message}") | ||
| Log.e("SetCallRepository", "ErrorBody=${response.errorBody()?.toString()}") | ||
| throw Exception(response.toString()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, let's locate and examine the specific file mentioned
find . -name "SetCallRepositoryImpl.kt" -type fRepository: Medicare-Call/Medicare-Call-Android
Length of output: 167
🏁 Script executed:
# Search for errorBody usage patterns in the codebase
rg -nP --type=kt 'errorBody\(\)' -A 3 -B 2Repository: Medicare-Call/Medicare-Call-Android
Length of output: 105
🏁 Script executed:
# Look for Response object usage patterns
rg -nP --type=kt 'response\.' --max-count=50 | head -50Repository: Medicare-Call/Medicare-Call-Android
Length of output: 105
🏁 Script executed:
# Check for exception handling patterns across repository impl files
fd -e "RepositoryImpl.kt" --type f | head -20Repository: Medicare-Call/Medicare-Call-Android
Length of output: 61
🏁 Script executed:
# Read the target file to see lines 21-23
cat -n ./app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/SetCallRepositoryImpl.kt | head -40Repository: Medicare-Call/Medicare-Call-Android
Length of output: 1798
🏁 Script executed:
# Search for errorBody usage without file type restriction
rg 'errorBody\(\)' -A 3 -B 2Repository: Medicare-Call/Medicare-Call-Android
Length of output: 13388
🏁 Script executed:
# Look for Response class imports to understand the object type
rg 'import.*Response' | head -20Repository: Medicare-Call/Medicare-Call-Android
Length of output: 3092
🏁 Script executed:
# Find all repository impl files to check error handling patterns
find ./app -name "*RepositoryImpl.kt" -type fRepository: Medicare-Call/Medicare-Call-Android
Length of output: 2065
errorBody()?.toString() 대신 .string() 사용하기
errorBody()?.toString()은 실제 에러 내용이 아니라 객체의 메모리 주소 문자열(예: java.nio.channels.Channels$ReadableByteChannelImpl@abc123)을 반환합니다. 다른 레포지토리 파일들(UserRepositoryImpl, EldersInfoRepositoryImpl 등)에서 사용 중인 errorBody()?.string()으로 변경해서 실제 에러 본문을 읽도록 해야 합니다.
추가로 제네릭 Exception 대신 HttpException(response)를 던지는 것이 좋습니다.
🧰 Tools
🪛 GitHub Actions: Android CI
[error] 23-23: TooGenericExceptionThrown: Exception is a too generic Exception. Prefer throwing specific exceptions that indicate a specific error case.
🤖 Prompt for AI Agents
In
@app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/SetCallRepositoryImpl.kt
around lines 21 - 23, In SetCallRepositoryImpl, replace the incorrect
errorBody()?.toString() usage with errorBody()?.string() so the actual HTTP
error payload is logged (update the Log.e call that currently logs
"ErrorBody=..."), and instead of throwing a generic
Exception(response.toString()), throw an HttpException(response) to preserve
HTTP error details; locate the logging and throw site in the method inside
SetCallRepositoryImpl where response is checked and update those two uses
accordingly.
| val response = subscribeService.getElderSubscriptions() | ||
| Log.d("SubscribeRepository", "응답 수신됨: isSuccessful = ${response.isSuccessful}") | ||
| Log.d("SubscribeRepository", "구독 정보 응답 코드: ${response.code()}") | ||
| Log.d("SubscribeRepository", "구독 정보 응답 코드: ${response.code}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "SubscribeRepositoryImpl.kt" | head -5Repository: Medicare-Call/Medicare-Call-Android
Length of output: 169
🌐 Web query:
Ktorfit Response error handling extract error body best practices
💡 Result:
Short answer — recommended approach and checklist:
-
Turn off automatic throwing and read the raw response first
- Configure Ktor HttpClient with expectSuccess = false so non-2xx responses don’t throw before you can read the body. [4]
-
Parse error body centrally into your error model
- Read the body as text (response.bodyAsText() / readText()) or use response.body(ErrorModel::class) with your serializer to convert into a typed error object. Do this inside a single helper so parsing logic is consistent. (Ktor clients can throw if expectSuccess=true; see note). [3][4]
-
Use a Ktorfit response converter to unify success/failure handling
- Implement Converter.SuspendResponseConverter (or the newer Converter.ResponseConverter / SuspendResponseConverter pattern) to wrap Ktorfit results into your domain result type (sealed class, Result, Either, etc.). In the converter you receive KtorfitResult.Success (with HttpResponse) or KtorfitResult.Failure (with throwable), and can parse the error body into your error DTO and return a unified result. This keeps parsing and mapping logic in one place. [1][2]
-
Catch and map Ktor exceptions promptly
- Handle ClientRequestException / ServerResponseException (thrown when expectSuccess=true) and extract response content from exception.response when needed. Prefer expectSuccess=false to avoid losing body because exceptions can be thrown early. See Ktor client issue history where reading body on non-200 caused ClientRequestException. [3]
-
Best-practice implementation pattern (summary)
- HttpClient(expectSuccess = false)
- Single helper: parseError(response) -> ErrorModel (uses kotlinx.serialization / converter)
- ResponseConverter / SuspendResponseConverter in Ktorfit to return sealed Result<T, ErrorModel>
- Map HTTP status → domain errors (401→AuthError, 422→ValidationError with parsed details, 5xx→ServerError)
- Log sanitized error details (avoid sensitive fields)
-
Additional tips
- Prefer typed parsing (JSON → ErrorModel) over fragile string inspection. [1][2]
- Retry/refresh-token logic should run before converting to final domain error; implement in an HttpClient feature or Ktor pipeline. [5]
- Centralize error-to-user-message mapping (so UI code stays simple).
Key docs/issues:
- Ktorfit converters and examples (ResponseConverter / SuspendResponseConverter). [1][2]
- Ktor client behavior for non-2xx responses and reading body (issue/notes). [3][4]
- Ktorfit configuration / builder (to attach converters and custom client). [5]
If you want, I can:
- paste a short Kotlin example showing HttpClient config + a Converter.SuspendResponseConverter that parses error bodies and returns a sealed Result type, or
- review your current Ktorfit client code and show where to add parsing.
🏁 Script executed:
cat -n ./app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/SubscribeRepositoryImpl.ktRepository: Medicare-Call/Medicare-Call-Android
Length of output: 1720
🏁 Script executed:
find . -type f -name "*Exception.kt" -o -name "*Error*.kt" | head -20Repository: Medicare-Call/Medicare-Call-Android
Length of output: 61
🏁 Script executed:
rg "class.*Exception|class.*Error" --type kt | head -30Repository: Medicare-Call/Medicare-Call-Android
Length of output: 105
🏁 Script executed:
find . -path "*/repositoryimpl/*.kt" -type f | head -10Repository: Medicare-Call/Medicare-Call-Android
Length of output: 972
🏁 Script executed:
cat -n ./app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/GlucoseRepositoryImpl.kt | head -40Repository: Medicare-Call/Medicare-Call-Android
Length of output: 1227
🏁 Script executed:
find . -type f -name "*.kt" | xargs rg "sealed class|data class" | grep -i "error\|exception\|result" | head -20Repository: Medicare-Call/Medicare-Call-Android
Length of output: 174
🏁 Script executed:
find . -type f -name "*.kt" -path "*/domain/*" | xargs rg "sealed class\|class.*Error\|class.*Exception" 2>/dev/null | head -30Repository: Medicare-Call/Medicare-Call-Android
Length of output: 61
🏁 Script executed:
cat -n ./app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/EldersInfoRepositoryImpl.ktRepository: Medicare-Call/Medicare-Call-Android
Length of output: 4136
errorBody 추출 방식과 예외 타입을 다른 Repository들과 맞춰주세요.
현재 코드는 errorBody()?.toString()을 사용하고 있는데, 같은 프로젝트의 다른 Repository들(EldersInfoRepositoryImpl, GlucoseRepositoryImpl)은 errorBody()?.string()을 사용하고 있습니다. 또한 generic Exception 대신 HttpException(response)를 throw하도록 통일해야 합니다.
수정 예시
// 현재
val error = response.errorBody()?.toString()
Log.e("SubscribeRepository", "응답 실패: $error")
throw Exception(response.toString())
// 변경 (다른 Repository 패턴과 일치)
val error = response.errorBody()?.string() ?: "Unknown error"
Log.e("SubscribeRepository", "응답 실패: $error")
throw HttpException(response)lines 24, 26에 적용되며, 추후 모든 Repository 구현체에 일관된 에러 처리 패턴을 적용하는 것을 권장합니다.
🤖 Prompt for AI Agents
In
@app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/SubscribeRepositoryImpl.kt
at line 18, The error handling in SubscribeRepositoryImpl uses
response.errorBody()?.toString() and throws a generic Exception; change it to
extract the body as a string like val error = response.errorBody()?.string() ?:
"Unknown error", log it with Log.e("SubscribeRepository", "응답 실패: $error"), and
throw an HttpException(response) instead of Exception to match the pattern used
in EldersInfoRepositoryImpl/GlucoseRepositoryImpl and ensure consistent error
handling.
| response.body() ?: error("Response body is null") | ||
| } else { | ||
| throw HttpException(response) | ||
| throw Exception(response.toString()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's examine the VerificationRepositoryImpl.kt file around line 26
find . -name "VerificationRepositoryImpl.kt" -type fRepository: Medicare-Call/Medicare-Call-Android
Length of output: 172
🏁 Script executed:
#!/bin/bash
# Search for HttpException usage patterns in the codebase
rg -nP --type=kt '\bhrow\s+(HttpException|Exception)' -B 2 -A 1Repository: Medicare-Call/Medicare-Call-Android
Length of output: 105
🏁 Script executed:
#!/bin/bash
# Search for catch blocks handling HttpException or other exceptions
rg -nP --type=kt '\bcatch\s*\([^)]*Exception' -B 1 -A 3Repository: Medicare-Call/Medicare-Call-Android
Length of output: 105
🏁 Script executed:
#!/bin/bash
# Read VerificationRepositoryImpl.kt to see the actual code
cat -n ./app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/VerificationRepositoryImpl.ktRepository: Medicare-Call/Medicare-Call-Android
Length of output: 1475
🏁 Script executed:
#!/bin/bash
# Search for HttpException and Exception throw statements without file type
rg -n 'throw.*Exception|catch.*Exception' --type-not=binary -B 1 -A 2Repository: Medicare-Call/Medicare-Call-Android
Length of output: 109
🏁 Script executed:
#!/bin/bash
# Also search for HttpException imports to understand what's being used
rg -n 'import.*HttpException|import.*retrofit' --type-not=binaryRepository: Medicare-Call/Medicare-Call-Android
Length of output: 109
🏁 Script executed:
#!/bin/bash
# Search for HttpException mentions and catch blocks
rg -in 'HttpException' -B 2 -A 2Repository: Medicare-Call/Medicare-Call-Android
Length of output: 28518
🏁 Script executed:
#!/bin/bash
# Search for catch blocks in the codebase
rg -in 'catch\s*\(' -B 1 -A 3Repository: Medicare-Call/Medicare-Call-Android
Length of output: 38155
🏁 Script executed:
#!/bin/bash
# Check imports to see if HttpException is used anywhere
rg -n 'import' app/src/main/java/com/konkuk/medicarecall/ | grep -i exceptionRepository: Medicare-Call/Medicare-Call-Android
Length of output: 2967
다른 리포지토리와 일치하도록 HttpException 사용하기
Exception(response.toString())을 사용하면 다른 리포지토리와 달라질 뿐 아니라, 호출자가 HTTP 상태 코드를 확인하기 어려워집니다. 코드베이스의 다른 모든 리포지토리가 HttpException(response)을 사용하므로 이에 맞춰주세요.
if (response.isSuccessful) {
response.body() ?: error("Response body is null")
} else {
throw HttpException(response)
}HttpException을 import하고 변경하면, EldersInfoRepositoryImpl, UserRepositoryImpl 등의 다른 리포지토리와 동일한 패턴이 되어 ViewModels에서 일관되게 .code() 메서드로 상태 코드를 확인할 수 있습니다.
🧰 Tools
🪛 GitHub Actions: Android CI
[error] 26-26: TooGenericExceptionThrown: Exception is a too generic Exception. Prefer throwing specific exceptions that indicate a specific error case.
🤖 Prompt for AI Agents
In
@app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/VerificationRepositoryImpl.kt
at line 26, Replace the generic Exception thrown in VerificationRepositoryImpl
(the place that currently does throw Exception(response.toString())) with an
HttpException constructed from the retrofit Response object (throw
HttpException(response)) and add the necessary import for
retrofit2.HttpException so callers can inspect HTTP status codes consistently
like other repos (EldersInfoRepositoryImpl, UserRepositoryImpl); keep the
existing null-body handling (response.body() ?: error(...)) for successful
responses.
| viewModelScope.launch { | ||
| verificationRepository.requestCertificationCode(phone) | ||
| .onSuccess { Log.d("httplog", "인증번호 요청 성공, ${it.message()}") } | ||
| .onSuccess { Log.d("httplog", "인증번호 요청 성공, ${it.message}") } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# Search for Ktorfit Response usage in the codebase
rg -t kt "Response" --max-count=20 -B2 -A2 | head -50Repository: Medicare-Call/Medicare-Call-Android
Length of output: 105
🏁 Script executed:
# Look for imports related to Ktorfit
rg -t kt "import.*Response|import.*ktorfit"Repository: Medicare-Call/Medicare-Call-Android
Length of output: 105
🏁 Script executed:
# Check for other usages of .message in the file
rg -t kt "\.message" app/src/main/java/com/konkuk/medicarecall/ui/feature/login/info/viewmodel/LoginViewModel.ktRepository: Medicare-Call/Medicare-Call-Android
Length of output: 105
🌐 Web query:
Ktorfit Response message property API documentation
💡 Result:
Do you mean the Ktorfit Response (converter) API or the Ktor/Ktorfit HttpResponse/Response object's "message" property? Please clarify which class/property and which documentation you need (Ktorfit site, Ktor API, or GitHub).
🏁 Script executed:
# Find all Kotlin files in the project
find . -type f -name "*.kt" | head -20Repository: Medicare-Call/Medicare-Call-Android
Length of output: 1586
🏁 Script executed:
# Look at the specific file mentioned in the review
cat -n app/src/main/java/com/konkuk/medicarecall/ui/feature/login/info/viewmodel/LoginViewModel.kt | sed -n '70,90p'Repository: Medicare-Call/Medicare-Call-Android
Length of output: 773
🏁 Script executed:
# Search for Response class definition
rg "class Response" --max-count=10 -B2 -A5Repository: Medicare-Call/Medicare-Call-Android
Length of output: 61
🏁 Script executed:
# Check for build.gradle to see dependencies
find . -type f -name "build.gradle*" | xargs cat | grep -i "ktorfit\|retrofit" | head -20Repository: Medicare-Call/Medicare-Call-Android
Length of output: 538
🏁 Script executed:
# Search for .message usage pattern in Kotlin files
rg "\.message\(" . --max-count=15 2>/dev/null | head -20Repository: Medicare-Call/Medicare-Call-Android
Length of output: 1296
🏁 Script executed:
# Find the VerificationRepository definition
find . -type f -name "*VerificationRepository*"Repository: Medicare-Call/Medicare-Call-Android
Length of output: 258
🏁 Script executed:
# Search for requestCertificationCode function definition
rg "requestCertificationCode" -B3 -A5Repository: Medicare-Call/Medicare-Call-Android
Length of output: 4443
🏁 Script executed:
# Look for Result class or extension functions on it
rg "extension.*Result|fun.*Result|class Result" -B2 -A3 | head -30Repository: Medicare-Call/Medicare-Call-Android
Length of output: 3598
🏁 Script executed:
# Check if there's a custom Result/Response wrapper
rg "onSuccess.*onFailure|Result<" -B2 -A2 | head -30Repository: Medicare-Call/Medicare-Call-Android
Length of output: 3487
🏁 Script executed:
# Look at build.gradle for Kotlin Result library
cat app/build.gradle | grep -i "result\|arrow" | head -20Repository: Medicare-Call/Medicare-Call-Android
Length of output: 127
🏁 Script executed:
# Check the imports in VerificationRepository to confirm it's Ktorfit's Response
cat -n app/src/main/java/com/konkuk/medicarecall/data/repository/VerificationRepository.ktRepository: Medicare-Call/Medicare-Call-Android
Length of output: 521
🌐 Web query:
Ktorfit version 2.7.1 Response class message property API
💡 Result:
Short answer: Ktorfit 2.7.1 does not introduce a custom Response.message property. It uses Ktor's HTTP response types (HttpResponse / io.ktor.client.statement.HttpResponse). Read the status via response.status, headers via response.headers, and the server payload (including any JSON field named "message") via response.body(...) or response.call.response.bodyAsText(). See Ktorfit docs on response converters and Ktor client response API for details. [1][2]
Sources:
- Ktorfit response converter docs. [1]
- Ktor client — receiving responses (HttpResponse API: status, headers, body, bodyAsText). [2]
🏁 Script executed:
# Search for how Throwable.message is used (for comparison)
rg "\.message" app/src/main/java/com/konkuk/medicarecall/ui/feature/login/info/viewmodel/LoginViewModel.kt -B2 -A2Repository: Medicare-Call/Medicare-Call-Android
Length of output: 1027
Response<Unit> 객체에 message 프로퍼티가 없음
Line 81의 onSuccess 블록에서 it.message로 접근하는 것은 오류입니다. Ktorfit의 Response 객체에는 message 프로퍼티가 없습니다. 대신 status, headers, body() 등을 사용해야 합니다. 반면 Line 82의 onFailure 블록에서 it.message는 Throwable의 표준 프로퍼티이므로 정상입니다.
🤖 Prompt for AI Agents
In
@app/src/main/java/com/konkuk/medicarecall/ui/feature/login/info/viewmodel/LoginViewModel.kt
at line 81, The onSuccess handler in LoginViewModel is incorrectly accessing
it.message on a Ktorfit Response<Unit>; replace that with a valid Response
property (e.g., it.status) or log a fixed success message (or use it.body() if
returning a body) in the .onSuccess { ... } callback where currently
Log.d("httplog", "인증번호 요청 성공, ${it.message}") is used so it no longer references
a nonexistent message property.
retrofit2.httpexception의 구조와 동일한 새로운 커스텀 Exception을 생성해 기존 코드의 수정 없이 동작할 수 있도록 구현
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @app/src/main/java/com/konkuk/medicarecall/data/exception/HttpException.kt:
- Around line 12-14: HttpException currently defines a method message() that
conflicts with Throwable.message and causes inconsistent usage; rename the
method to a clear property like responseMessage (e.g., add val responseMessage:
String = _message and remove fun message()) in class HttpException, update all
call sites that use exception.message() (LoginElderViewModel, MealViewModel,
etc.) to use exception.responseMessage, and keep uses of exception.message that
rely on the Throwable-formatted string (e.g., CheckLoginStatusUseCase) unchanged
so the API is consistent and unambiguous.
🧹 Nitpick comments (1)
app/src/main/java/com/konkuk/medicarecall/data/exception/HttpException.kt (1)
5-10: Kotlin 스타일로 리팩토링 고려해보세요.언더스코어 prefix(_code, _message)와 getter 메서드 대신 Kotlin의 백킹 프로퍼티 패턴이나 직접 프로퍼티를 사용하는 것이 더 관용적입니다. 또한 response 필드가 nullable로 선언되어 있지만 생성자는 non-null을 받고 있어서 nullable일 이유가 없어 보입니다.
♻️ Kotlin 스타일 리팩토링 제안
-class HttpException(response: Response<*>) : RuntimeException(getMessage(response)) { - private val _code: Int = response.code - private val _message: String = response.message - - @Transient - private val response: Response<*>? = response - - fun code(): Int = _code - fun message(): String = _message - fun response(): Response<*>? = response +class HttpException(private val response: Response<*>) : RuntimeException(getMessage(response)) { + val code: Int get() = response.code + val message: String get() = response.message
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/src/main/java/com/konkuk/medicarecall/data/exception/HttpException.kt
🔇 Additional comments (2)
app/src/main/java/com/konkuk/medicarecall/data/exception/HttpException.kt (2)
1-4: 임포트 확인 완료!Ktorfit의 Response를 사용하는 것이 마이그레이션 목표와 일치합니다.
16-20: 깔끔한 헬퍼 메서드입니다.HTTP 응답을 사람이 읽기 좋은 형식으로 포맷팅하는 로직이 잘 캡슐화되어 있습니다.
| fun code(): Int = _code | ||
| fun message(): String = _message | ||
| fun response(): Response<*>? = response |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
cat -n app/src/main/java/com/konkuk/medicarecall/data/exception/HttpException.ktRepository: Medicare-Call/Medicare-Call-Android
Length of output: 843
🏁 Script executed:
# Search for HttpException usages in the codebase
rg -n "HttpException" --type kotlin -A 3 -B 2 | head -100Repository: Medicare-Call/Medicare-Call-Android
Length of output: 12016
🏁 Script executed:
# Check for .message() method calls
rg -n "\.message\(\)" --type kotlin -C 2 | head -100Repository: Medicare-Call/Medicare-Call-Android
Length of output: 4847
🏁 Script executed:
# Check for .message property access
rg -n "exception\.message[^(]|error\.message[^(]" --type kotlin -C 2 | head -100Repository: Medicare-Call/Medicare-Call-Android
Length of output: 4608
message() 메서드가 Throwable.message 프로퍼티와 충돌합니다.
RuntimeException을 상속받는 이 클래스는 exception.message로 프로퍼티 접근 시 "HTTP 200 OK" 형태의 포맷팅된 메시지를 받습니다. 하지만 message(): String 메서드를 정의했으므로, 실제 응답 메시지는 exception.message()로 명시적으로 호출해야만 접근 가능합니다.
현재 코드베이스를 보면 이 둘이 혼재되어 있습니다:
exception.message(포맷팅된 메시지) - CheckLoginStatusUseCase 등에서 사용exception.message()(실제 응답 메시지) - LoginElderViewModel, MealViewModel 등에서 사용
어느 것이 의도된 동작인지 명확히 하고, 메서드를 responseMessage()로 이름 변경하거나 프로퍼티로 오버라이드하여 API를 단순화하는 것을 권장합니다.
🤖 Prompt for AI Agents
In @app/src/main/java/com/konkuk/medicarecall/data/exception/HttpException.kt
around lines 12 - 14, HttpException currently defines a method message() that
conflicts with Throwable.message and causes inconsistent usage; rename the
method to a clear property like responseMessage (e.g., add val responseMessage:
String = _message and remove fun message()) in class HttpException, update all
call sites that use exception.message() (LoginElderViewModel, MealViewModel,
etc.) to use exception.responseMessage, and keep uses of exception.message that
rely on the Throwable-formatted string (e.g., CheckLoginStatusUseCase) unchanged
so the API is consistent and unambiguous.
…all-Android into refactor/ktorfit-onboarding-#216
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)
gradle/libs.versions.toml (1)
1-67: Ktorfit 의존성을 버전 카탈로그에 추가하세요.Ktorfit은 현재
app/build.gradle.kts에 하드코딩되어 있습니다. 다른 의존성들처럼gradle/libs.versions.toml에서 버전을 관리하는 것이 일관성 있습니다.다음을 추가하세요:
[versions] # ... 기존 버전들 ... ktorfit = "2.7.1" [libraries] # ... 기존 라이브러리들 ... ktorfit-lib = { module = "de.jensklingenberg.ktorfit:ktorfit-lib", version.ref = "ktorfit" } ktorfit-converters-response = { module = "de.jensklingenberg.ktorfit:ktorfit-converters-response", version.ref = "ktorfit" } [plugins] # ... 기존 플러그인들 ... ktorfit = { id = "de.jensklingenberg.ktorfit", version.ref = "ktorfit" }그 다음
app/build.gradle.kts를 업데이트하세요:plugins { // ... 기존 플러그인들 ... alias(libs.plugins.ktorfit) } dependencies { // ... 기존 의존성들 ... implementation(libs.ktorfit.lib) implementation(libs.ktorfit.converters.response) }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/build.gradle.ktsgradle/libs.versions.toml
💤 Files with no reviewable changes (1)
- app/build.gradle.kts
🔇 Additional comments (1)
gradle/libs.versions.toml (1)
2-2: gradle/libs.versions.toml은 새로 추가된 파일이며, 현재 AGP 버전 설정은 적절합니다.파일의
A(Added) 상태에서 볼 수 있듯이 이것은 새로 생성된 파일입니다. 또한 Ktorfit 의존성은 app/build.gradle.kts에 명시적으로 선언되어 있고 실제로 사용 중입니다. 모든 의존성을 버전 카탈로그에 넣을 필요는 없으므로 현재 구조는 문제없습니다.
alswlekk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
| val errorBody = response.errorBody()?.toString() ?: "Unknown error" | ||
| throw Exception(errorBody) // Exception Type? | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기는 HttpException으로 원상복구해야하는 거 맞을까요 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
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/FcmRepositoryImpl.kt (1)
61-71: 응답 상태 확인이 필요하고 타입 표기가 부정확함
runCatching으로 감싼updateFcmToken()은Response<Unit>을 반환하므로, 실제 타입은Result<Response<Unit>>입니다.Result<Unit>으로 표기한 것은 부정확합니다.더 중요한 건, 다른 리포지토리 구현들처럼
response.isSuccessful을 확인하고 실패 시 예외를 던져야 합니다:참고할 수 있는 패턴
val response: Result<Unit> = runCatching { val response = fcmUpdateService.updateFcmToken( header = "Bearer $jwtToken", body = mapOf("fcmToken" to newToken), ) if (response.isSuccessful) { response.body() ?: error("Response body is null") } else { throw HttpException(response) } }
🤖 Fix all issues with AI agents
In @app/src/main/java/com/konkuk/medicarecall/App.kt:
- Around line 39-44: modules(...) 호출 블록의 들여쓰기가 CI에서 요구하는 12칸이 아니라 16칸으로 되어 있어 포맷
오류가 발생합니다; App.kt에서 modules( 호출 내부 항목들인 defaultModule, ApiModule().module,
NetworkModule().module을 유지하되 닫는 괄호와 콤마 위치의 들여쓰기를 호출 시작 열과 맞춰 12칸으로 조정해 전체 블록의
들여쓰기를 일관되게 맞춰 주세요.
In @app/src/main/java/com/konkuk/medicarecall/data/api/fcm/FcmUpdateService.kt:
- Around line 7-8: Remove the unnecessary consecutive blank line reported by CI
in FcmUpdateService.kt: open the file around the class or top-of-file
declarations (look for the FcmUpdateService class or its surrounding
imports/package header) and delete the extra empty line between lines 7 and 8 so
there are no consecutive blank lines; ensure formatting follows the project
style (single blank line where needed).
🧹 Nitpick comments (1)
app/src/main/java/com/konkuk/medicarecall/data/di/ApiModule.kt (1)
50-52: Qualifier 이름 불일치 고려
@AuthRetrofitqualifier가 이제Ktorfit타입을 받고 있어요. 혼란을 방지하기 위해@AuthKtorfit로 이름을 변경하는 것을 고려해보세요. 당장 필수는 아니지만 코드 가독성에 도움이 될 거예요.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
app/src/main/java/com/konkuk/medicarecall/App.ktapp/src/main/java/com/konkuk/medicarecall/data/api/elders/HealthService.ktapp/src/main/java/com/konkuk/medicarecall/data/api/fcm/FcmUpdateService.ktapp/src/main/java/com/konkuk/medicarecall/data/api/fcm/FcmValidationService.ktapp/src/main/java/com/konkuk/medicarecall/data/di/ApiModule.ktapp/src/main/java/com/konkuk/medicarecall/data/di/KtorApiModule.ktapp/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/FcmRepositoryImpl.kt
💤 Files with no reviewable changes (1)
- app/src/main/java/com/konkuk/medicarecall/data/di/KtorApiModule.kt
🧰 Additional context used
🪛 GitHub Actions: Android CI
app/src/main/java/com/konkuk/medicarecall/App.kt
[error] 44-44: Unexpected indentation (16) (should be 12) [Indentation]
app/src/main/java/com/konkuk/medicarecall/data/api/fcm/FcmUpdateService.kt
[error] 8-8: Needless blank line(s) [NoConsecutiveBlankLines]
🔇 Additional comments (5)
app/src/main/java/com/konkuk/medicarecall/App.kt (1)
26-27: LGTM!
@KoinApplication어노테이션 적용과 클래스 시그니처 변경이 적절하게 이루어졌어요.app/src/main/java/com/konkuk/medicarecall/data/api/elders/HealthService.kt (1)
4-6: LGTM!Retrofit에서 Ktorfit으로 HTTP 어노테이션 임포트가 깔끔하게 마이그레이션 되었네요.
app/src/main/java/com/konkuk/medicarecall/data/api/fcm/FcmValidationService.kt (1)
3-4: LGTM!Ktorfit Response 및 POST 어노테이션으로 잘 마이그레이션 되었어요.
app/src/main/java/com/konkuk/medicarecall/data/api/fcm/FcmUpdateService.kt (1)
3-6: LGTM!Ktorfit 어노테이션으로 임포트가 잘 변경되었네요.
app/src/main/java/com/konkuk/medicarecall/data/di/ApiModule.kt (1)
54-128: LGTM!모든 서비스 프로바이더 메서드가 Retrofit에서 Ktorfit으로 일관성 있게 마이그레이션 되었네요.
createXService()확장 함수 패턴 사용이 깔끔해요. 👍
| modules( | ||
| defaultModule, | ||
| ) | ||
| modules( | ||
| ApiModule().module, | ||
| NetworkModule().module, | ||
| ) | ||
|
|
||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
파이프라인 실패: 들여쓰기 수정 필요
CI에서 Line 44의 들여쓰기가 잘못되었다고 보고하고 있어요. 12칸이어야 하는데 16칸으로 되어 있네요.
🔧 수정 제안
modules(
defaultModule,
ApiModule().module,
NetworkModule().module,
-
- )
+ )📝 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.
| modules( | |
| defaultModule, | |
| ) | |
| modules( | |
| ApiModule().module, | |
| NetworkModule().module, | |
| ) | |
| ) | |
| modules( | |
| defaultModule, | |
| ApiModule().module, | |
| NetworkModule().module, | |
| ) |
🧰 Tools
🪛 GitHub Actions: Android CI
[error] 44-44: Unexpected indentation (16) (should be 12) [Indentation]
🤖 Prompt for AI Agents
In @app/src/main/java/com/konkuk/medicarecall/App.kt around lines 39 - 44,
modules(...) 호출 블록의 들여쓰기가 CI에서 요구하는 12칸이 아니라 16칸으로 되어 있어 포맷 오류가 발생합니다; App.kt에서
modules( 호출 내부 항목들인 defaultModule, ApiModule().module, NetworkModule().module을
유지하되 닫는 괄호와 콤마 위치의 들여쓰기를 호출 시작 열과 맞춰 12칸으로 조정해 전체 블록의 들여쓰기를 일관되게 맞춰 주세요.
|
|
||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
파이프라인 실패: 불필요한 빈 줄 제거 필요
CI에서 연속된 빈 줄이 있다고 보고하고 있어요. Line 7-8 사이의 중복 빈 줄을 제거해주세요.
🔧 수정 제안
import de.jensklingenberg.ktorfit.http.Header
import de.jensklingenberg.ktorfit.http.POST
-
interface FcmUpdateService {📝 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.
| import de.jensklingenberg.ktorfit.http.Header | |
| import de.jensklingenberg.ktorfit.http.POST | |
| interface FcmUpdateService { |
🧰 Tools
🪛 GitHub Actions: Android CI
[error] 8-8: Needless blank line(s) [NoConsecutiveBlankLines]
🤖 Prompt for AI Agents
In @app/src/main/java/com/konkuk/medicarecall/data/api/fcm/FcmUpdateService.kt
around lines 7 - 8, Remove the unnecessary consecutive blank line reported by CI
in FcmUpdateService.kt: open the file around the class or top-of-file
declarations (look for the FcmUpdateService class or its surrounding
imports/package header) and delete the extra empty line between lines 7 and 8 so
there are no consecutive blank lines; ensure formatting follows the project
style (single blank line where needed).
🔗 관련 이슈
📙 작업 설명
💬 추가 설명 or 리뷰 포인트 (선택)
Summary by CodeRabbit
릴리스 노트
Refactor
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.