Conversation
y-eonee
requested review from
soseoyo12,
sum130 and
wotjs020708
and removed request for
a team
Closed
1 task
📝 WalkthroughWalkthroughAlamofire를 SPM으로 추가하고 네트워크 계층(EndPoint 프로토콜, DefaultNetworkService, DTO, 오류 매핑)을 도입했습니다. Data 의존성 주입이 네트워크 서비스로 변경되었고, OSLog.framework 참조 및 Presentation/Global/Component.swift 파일이 제거되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Assembler
participant Repo as DefaultTestRepository
participant NetSvc as DefaultNetworkService
participant EndPoint
participant Alamofire
participant Server
Client->>Assembler: assemble()
Assembler->>Repo: DefaultTestRepository(networkService)
Client->>Repo: repo.test()/요청 호출
Repo->>NetSvc: request(endPoint)
NetSvc->>EndPoint: obtain request info (requestURL, headers, params)
EndPoint-->>NetSvc: return endpoint data
NetSvc->>Alamofire: AF.request(...)
Alamofire->>Server: HTTP 요청 전송
Server-->>Alamofire: HTTP 응답
Alamofire-->>NetSvc: 응답 전달
NetSvc->>NetSvc: 로그, 검증, 상태코드 매핑, BaseResponseDTO 디코딩
NetSvc-->>Repo: 성공 데이터 또는 CherrishError
Repo-->>Client: 결과 반환
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 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: 14
🤖 Fix all issues with AI agents
In @Cherrish-iOS/Cherrish-iOS/Data/DataDependencyAssembler.swift:
- Around line 11-15: DataDependencyAssembler currently instantiates
DefaultNetworkService directly via the networkService property; instead register
NetworkService (conforming to the NetworkService protocol/interface) as a
singleton in your DIContainer and resolve it from the container in
DataDependencyAssembler so callers reuse the same instance. Replace the direct
new DefaultNetworkService() creation in DataDependencyAssembler with a
resolution from DIContainer (or accept NetworkService via init injection),
ensure the DI registration uses singleton scope, and update tests to register a
MockNetworkService for injection where needed.
In @Cherrish-iOS/Cherrish-iOS/Data/Model/BaseResponseDTO.swift:
- Around line 16-19: EmptyResponseDTO currently lacks explicit access control;
update the struct declaration and its properties to use the same access level as
BaseResponseDTO (match the access modifier used on BaseResponseDTO for both the
struct and the let properties) so visibility is consistent across DTOs, and
optionally change conformance from Decodable to Codable if future encoding will
be needed.
- Around line 10-14: BaseResponseDTO의 code 프로퍼티는 현재 사용되거나 검증되지 않으므로 타입을
유지하되(문자열로 둠) 접근 제어를 명확히 하기 위해 struct BaseResponseDTO와 프로퍼티 code에 대해 명시적으로
internal을 선언하고, 향후 API 응답 값이 정해지면 ResponseCode enum으로 교체할 수 있다는 간단한 주석을 추가하세요;
참조 식별자: BaseResponseDTO, code, data.
In @Cherrish-iOS/Cherrish-iOS/Data/Network/CherrishError.swift:
- Around line 20-25: The enum case order for badRequest, unauthorized,
forbidden, notFound, conflict, tooManyRequests does not match the order used in
the errorDescription switch; update the switch inside the errorDescription
computed property so the case labels process notFound before conflict (or
reorder the enum to match the switch), ensuring the sequence of case handlers
matches the enum declaration (referencing the enum cases notFound and conflict
and the errorDescription switch) to improve maintainability.
In @Cherrish-iOS/Cherrish-iOS/Data/Network/EndPoint/EndPoint.swift:
- Line 30: Rename the misspelled property queryParmaters to queryParameters in
the EndPoint protocol declaration and update all references (e.g., where
queryParmaters is accessed/returned around the implementation lines referenced,
including the usages mentioned on lines 46-49) to use queryParameters so the
property name is consistent across the protocol and its consumers.
- Around line 36-59: Change requestURL to return an optional URL to avoid
force-unwrapping crashes: update the EndPoint protocol signature from var
requestURL: URL to var requestURL: URL? and modify the EndPoint extension
implementation (requestURL) to return nil on failures instead of URL(string:
"")!; replace both guarded failure branches that call
CherrishLogger.error(CherrishError.URLError) then force-unwrap with returning
nil, keep logging, ensure URLComponents usage (URLComponents(string:),
urlComponents.url) stays the same, fix the query parameter usage (queryParmaters
-> queryParameters) and set urlComponents.queryItems = queryParameters.map {
URLQueryItem(name: $0, value: $1) }, and then update any call sites that used
EndPoint.requestURL to handle the optional result.
In @Cherrish-iOS/Cherrish-iOS/Data/Network/EndPoint/TestAPI.swift:
- Around line 8-16: The TestAPI enum is missing its EndPoint protocol
conformance (the extension is commented out); implement an extension TestAPI:
EndPoint and provide concrete implementations for required members including
basePath, path, method, headers, parameterEncoding, queryParameters, and
bodyParameters (or return nil/empty where appropriate) so each case (.login,
.logout) returns correct values; ensure you implement all EndPoint protocol
requirements and handle per-case logic inside the TestAPI extension.
In @Cherrish-iOS/Cherrish-iOS/Data/Network/NetworkService.swift:
- Around line 67-94: The non-decoding overload request(_ endPoint: EndPoint)
does not follow the same error logging/mapping policy: it currently throws
AFError directly and omits CherrishLogger.error; update the function so that
whenever response.error exists you (1) try to decode the error body into
EmptyResponseDTO as now but if decoding fails still extract response.data (as
string or hex) and statusCode, (2) call CherrishLogger.error with contextual
info (endpoint, statusCode, response body/error description), (3) map the
observed statusCode/message into a Cherrish error via handleError(statusCode,
message) and throw that mapped error instead of raw AFError, ensuring parity
with the decoding variant (use the same keys/log format as the decoding path).
- Around line 12-18: Change the protocol method signature so the type-only
parameter is explicitly an intentionally unused parameter (e.g. func request<T:
Decodable>(_ endPoint: EndPoint, _: T.Type) async throws -> T), and update any
conforming implementations to match (either name the parameter _ or prefix the
parameter usage with _ in the implementation) so the compiler no longer emits
unused-parameter warnings for decodingType while still forcing callers to
provide the generic type.
- Around line 111-138: The mapping for statusCode == -1 is inconsistent; update
handleError(_ statusCode: Int, _ errorResponse: String) to treat -1 the same as
other network connection failures by explicitly mapping case -1 to
.networkConnect (or alternatively change the default to return .networkConnect
for non-HTTP responses) so that raw network failures, timeouts (408) and 5xx
server errors are classified consistently as .networkConnect rather than leaving
-1 falling through to .networkError.
- Around line 22-65: The request(_:decodingType:) error handling is
inconsistent: when response.error exists you sometimes throw the AF error
instead of mapping to CherrishError via handleError, and decoding failures of
BaseResponseDTO lack logging. Change the error branch that checks response.error
to always obtain statusCode = response.response?.statusCode ?? -1 and try to
decode an error message from response.data (using EmptyResponseDTO or fallback),
call handleError(statusCode, message) and throw that mapped error (and log it
with CherrishLogger.error); additionally, wrap the
JSONDecoder().decode(BaseResponseDTO<T>.self, from: data) in do/catch so that
any decoding error logs CherrishLogger.error with the decoding Error and the raw
response data (string or hex) before rethrowing a clear CherrishError (or the
decoding error) so callers can diagnose failures. Ensure you reference
request(_:decodingType:), EmptyResponseDTO, BaseResponseDTO, handleError, and
CherrishLogger when making these changes.
In @Cherrish-iOS/Cherrish-iOS/Data/Repository/TestRepository.swift:
- Around line 10-16: DefaultTestRepository currently stores the injected
networkService in its initializer but never uses it, which looks like dead code;
either use it in a minimal example method (e.g., add a simple
fetch/testNetwork() method that calls networkService.request or similar) or
document intent with a TODO/comment in the struct and mark the property as used
in tests/examples; locate DefaultTestRepository and the private let
networkService and update by adding a short example method or a clarifying
comment/TODO referencing networkService and the init so the injection is
intentional and not unused.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (9)
Cherrish-iOS/Cherrish-iOS.xcodeproj/project.pbxprojCherrish-iOS/Cherrish-iOS.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolvedCherrish-iOS/Cherrish-iOS/Data/DataDependencyAssembler.swiftCherrish-iOS/Cherrish-iOS/Data/Model/BaseResponseDTO.swiftCherrish-iOS/Cherrish-iOS/Data/Network/CherrishError.swiftCherrish-iOS/Cherrish-iOS/Data/Network/EndPoint/EndPoint.swiftCherrish-iOS/Cherrish-iOS/Data/Network/EndPoint/TestAPI.swiftCherrish-iOS/Cherrish-iOS/Data/Network/NetworkService.swiftCherrish-iOS/Cherrish-iOS/Data/Repository/TestRepository.swift
🧰 Additional context used
🧬 Code graph analysis (3)
Cherrish-iOS/Cherrish-iOS/Data/Network/NetworkService.swift (1)
Cherrish-iOS/Cherrish-iOS/Core/CherrishLogger.swift (3)
error(147-153)data(139-145)network(123-129)
Cherrish-iOS/Cherrish-iOS/Data/DataDependencyAssembler.swift (3)
Cherrish-iOS/Cherrish-iOS/Presentation/PresentationDependencyAssembler.swift (1)
assemble(17-28)Cherrish-iOS/Cherrish-iOS/Domain/DomainDependencyAssembler.swift (1)
assemble(17-27)Cherrish-iOS/Cherrish-iOS/Core/DIContainer.swift (1)
register(22-25)
Cherrish-iOS/Cherrish-iOS/Data/Network/EndPoint/EndPoint.swift (1)
Cherrish-iOS/Cherrish-iOS/Core/CherrishLogger.swift (1)
error(147-153)
🪛 SwiftLint (0.57.0)
Cherrish-iOS/Cherrish-iOS/Data/Network/EndPoint/TestAPI.swift
[Warning] 13-13: A doc comment should be attached to a declaration
(orphaned_doc_comment)
Cherrish-iOS/Cherrish-iOS/Data/Network/EndPoint/EndPoint.swift
[Warning] 38-38: TODOs should be resolved (서버 배포 후 수정)
(todo)
[Warning] 72-72: TODOs should be resolved (서버 구조에 맞춰 수정)
(todo)
🔇 Additional comments (6)
Cherrish-iOS/Cherrish-iOS/Data/Network/CherrishError.swift (1)
47-58: 이 에러 케이스들은 NetworkService에서 올바르게 매핑되어 있습니다.
handleError메서드에서 각 HTTP 상태 코드에 대한 명확한 매핑 전략이 구현되어 있습니다: 400, 401, 403, 404, 409, 429는 각각의 전용 케이스로 매핑되고, 나머지 상태 코드는 제네릭networkError(code:message:)케이스를 사용합니다. 일관된 전략이 잘 적용되어 있으므로 추가 수정은 필요하지 않습니다.Cherrish-iOS/Cherrish-iOS/Data/Network/EndPoint/EndPoint.swift (2)
38-38: TODO 주석 확인서버 배포 및 API 스펙 확정 후 다음 항목들을 업데이트해야 합니다:
- Line 38: baseURL 설정
- Line 72: userID 헤더 구조 확정
개발 단계에서 TODO 주석을 남긴 것은 적절합니다. 향후 서버 연동 시 반드시 업데이트하세요.
Also applies to: 72-72
61-76: HeaderType의 HTTPHeaders 초기화 구문은 올바릅니다.현재 코드의 dictionary literal 초기화 방식은 Alamofire 5.11.0+ 버전에서 완벽히 지원됩니다. HTTPHeaders는
ExpressibleByDictionaryLiteral프로토콜을 준수하므로["key": "value"]형태로 직접 초기화할 수 있습니다. 현재 코드는 올바른 Alamofire 사용 방식입니다.Likely an incorrect or invalid review comment.
Cherrish-iOS/Cherrish-iOS.xcodeproj/project.pbxproj (1)
362-379: Alamofire Swift Package 통합이 올바르게 구성되었습니다.Swift Package Manager를 통한 Alamofire 통합이 적절하게 설정되어 있습니다:
upToNextMajorVersion전략으로 5.11.0 이상 6.0.0 미만 버전 사용- 프레임워크 빌드 단계에 올바르게 추가됨
- 패키지 의존성이 타겟에 연결됨
Cherrish-iOS/Cherrish-iOS/Data/DataDependencyAssembler.swift (1)
19-19: 의존성 주입 패턴이 올바르게 적용되었습니다.DefaultTestRepository에 NetworkService를 주입하는 방식이 적절합니다. 이는 테스트 가능성과 유지보수성을 향상시킵니다.
Cherrish-iOS/Cherrish-iOS.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved (1)
5-11: Alamofire 5.11.0은 유효하며 보안 취약점이 없습니다.검증 결과, Alamofire 5.11.0은 2025년 12월 20일 공식 릴리스되었으며 현재 최신 버전입니다. 해당 버전에 대한 알려진 보안 취약점(CVE)이 없으므로 현재 설정은 안전합니다.
Comment on lines
+11
to
+15
| private let networkService: NetworkService | ||
|
|
||
| init() { | ||
| self.networkService = DefaultNetworkService() | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
NetworkService를 DIContainer에 등록하는 것을 고려하세요.
현재 DataDependencyAssembler의 init에서 NetworkService를 생성하고 있습니다. 이는 작동하지만, 향후 다른 Repository들도 NetworkService가 필요할 경우 재사용성이 떨어질 수 있습니다.
♻️ 개선 제안: NetworkService를 DIContainer에 등록
final class DataDependencyAssembler: DependencyAssembler {
- private let networkService: NetworkService
-
- init() {
- self.networkService = DefaultNetworkService()
- }
-
func assemble() {
+ // NetworkService를 먼저 등록
+ DIContainer.shared.register(type: NetworkService.self) {
+ return DefaultNetworkService()
+ }
+
+ guard let networkService = DIContainer.shared.resolve(type: NetworkService.self) else {
+ return
+ }
+
DIContainer.shared.register(type: TestInterface.self) {
- return DefaultTestRepository(networkService: self.networkService)
+ return DefaultTestRepository(networkService: networkService)
}
}
}이렇게 하면:
- NetworkService가 싱글톤으로 관리됨
- 다른 Repository에서도 동일한 NetworkService 인스턴스를 재사용 가능
- 테스트 시 Mock NetworkService 주입이 용이함
🤖 Prompt for AI Agents
In @Cherrish-iOS/Cherrish-iOS/Data/DataDependencyAssembler.swift around lines 11
- 15, DataDependencyAssembler currently instantiates DefaultNetworkService
directly via the networkService property; instead register NetworkService
(conforming to the NetworkService protocol/interface) as a singleton in your
DIContainer and resolve it from the container in DataDependencyAssembler so
callers reuse the same instance. Replace the direct new DefaultNetworkService()
creation in DataDependencyAssembler with a resolution from DIContainer (or
accept NetworkService via init injection), ensure the DI registration uses
singleton scope, and update tests to register a MockNetworkService for injection
where needed.
Contributor
There was a problem hiding this comment.
저도 이 부분에 동의합니다.내용을 완벽히 이해 하지는 않았지만 내용 확인하면 좋을 거 같아요
Contributor
There was a problem hiding this comment.
찾아보니까 관리하기는 편하다는데 지금 이해하기도 쉽지않았는데 저희가 잘 활용할 수 있을지..! 좀 걱정되긴합니당 한 번 생각해주세요
Comment on lines
+10
to
+14
| struct BaseResponseDTO<T: Decodable>: Decodable { | ||
| let code: String | ||
| let message: String | ||
| let data: T? | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, find and read the actual BaseResponseDTO.swift file
fd "BaseResponseDTO.swift" --type f -x cat -n {}Repository: TEAM-Cherrish/Cherrish-iOS
Length of output: 504
🏁 Script executed:
#!/bin/bash
# Search for all uses of BaseResponseDTO in the codebase
rg -n "BaseResponseDTO" --type=swift -B 2 -A 2Repository: TEAM-Cherrish/Cherrish-iOS
Length of output: 1250
🏁 Script executed:
#!/bin/bash
# Look for API response structures, examples, or documentation
fd -e json -e md -e txt | head -20 | xargs rg -l "response\|api\|code\|message" -i 2>/dev/null | head -10Repository: TEAM-Cherrish/Cherrish-iOS
Length of output: 52
🏁 Script executed:
#!/bin/bash
# Check if ResponseCode or similar enum already exists
rg -n "enum.*Code.*Decodable\|enum.*Response" --type=swiftRepository: TEAM-Cherrish/Cherrish-iOS
Length of output: 52
🏁 Script executed:
#!/bin/bash
# Get more context from NetworkService.swift around line 57
sed -n '40,75p' Cherrish-iOS/Cherrish-iOS/Data/Network/NetworkService.swift | cat -nRepository: TEAM-Cherrish/Cherrish-iOS
Length of output: 1523
🏁 Script executed:
#!/bin/bash
# Search for how the code field is actually used after decoding
rg -n "\.code" --type=swift -B 2 -A 2 | head -50Repository: TEAM-Cherrish/Cherrish-iOS
Length of output: 52
🏁 Script executed:
#!/bin/bash
# Check if BaseResponseDTO is used in other modules (beyond Data module)
rg -n "BaseResponseDTO\|import.*Data\|from.*Data" --type=swift | grep -E "(Presentation|Domain|UI)" | head -20Repository: TEAM-Cherrish/Cherrish-iOS
Length of output: 52
🏁 Script executed:
#!/bin/bash
# Search for error handling or code value comparisons
rg -n "code.*==\|code.*!=\|switch.*code\|if.*code" --type=swift -B 1 -A 1 | head -40Repository: TEAM-Cherrish/Cherrish-iOS
Length of output: 52
code 필드의 사용 패턴을 확인한 후 리팩토링 필요성 재평가
현재 코드에서 code 필드는 접근되거나 검증되지 않으며, 오직 data 필드만 활발하게 사용됩니다. 따라서 String 타입으로 유지해도 무방합니다. 다만 다음을 고려할 수 있습니다:
- 접근 제어: 현재
internal(기본값)로 설정되어 있으며, Data 레이어 내에서만 사용되므로 명시적으로internal을 추가하거나 유지해도 됩니다. - 타입 안정성: API 응답 코드가 정해진 값들(예: "SUCCESS", "FAILURE")이라면 나중에
enum으로 개선할 수 있습니다. 현재로서는 불필요합니다.
💡 향후 개선 사항 (선택 사항)
API 명세에서 code 필드의 가능한 값들이 정의되어 있다면, 다음과 같이 개선할 수 있습니다:
public enum ResponseCode: String, Decodable {
case success = "SUCCESS"
case failure = "FAILURE"
// API 명세에 맞춰 추가
}
struct BaseResponseDTO<T: Decodable>: Decodable {
let code: ResponseCode
let message: String
let data: T?
}🤖 Prompt for AI Agents
In @Cherrish-iOS/Cherrish-iOS/Data/Model/BaseResponseDTO.swift around lines 10 -
14, BaseResponseDTO의 code 프로퍼티는 현재 사용되거나 검증되지 않으므로 타입을 유지하되(문자열로 둠) 접근 제어를 명확히
하기 위해 struct BaseResponseDTO와 프로퍼티 code에 대해 명시적으로 internal을 선언하고, 향후 API 응답 값이
정해지면 ResponseCode enum으로 교체할 수 있다는 간단한 주석을 추가하세요; 참조 식별자: BaseResponseDTO, code,
data.
Comment on lines
+16
to
+19
| struct EmptyResponseDTO: Decodable { | ||
| let code: String | ||
| let message: String | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
명확한 설계입니다.
데이터가 없는 응답을 위한 별도 DTO 정의가 명확하고 이해하기 쉽습니다. BaseResponseDTO와 동일하게 명시적인 접근 제어자를 추가하여 일관성을 유지하는 것을 권장합니다.
💡 제안: 일관성을 위한 접근 제어자 추가
-struct EmptyResponseDTO: Decodable {
- let code: String
- let message: String
+public struct EmptyResponseDTO: Decodable {
+ public let code: String
+ public let message: String
+
+ public init(code: String, message: String) {
+ self.code = code
+ self.message = message
+ }
}선택사항: 향후 인코딩이 필요할 경우를 대비해 Decodable 대신 Codable을 사용하는 것도 고려해 볼 수 있습니다.
📝 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
| struct EmptyResponseDTO: Decodable { | |
| let code: String | |
| let message: String | |
| } | |
| public struct EmptyResponseDTO: Decodable { | |
| public let code: String | |
| public let message: String | |
| public init(code: String, message: String) { | |
| self.code = code | |
| self.message = message | |
| } | |
| } |
🤖 Prompt for AI Agents
In @Cherrish-iOS/Cherrish-iOS/Data/Model/BaseResponseDTO.swift around lines 16 -
19, EmptyResponseDTO currently lacks explicit access control; update the struct
declaration and its properties to use the same access level as BaseResponseDTO
(match the access modifier used on BaseResponseDTO for both the struct and the
let properties) so visibility is consistent across DTOs, and optionally change
conformance from Decodable to Codable if future encoding will be needed.
Comment on lines
+20
to
+25
| case badRequest | ||
| case unauthorized | ||
| case forbidden | ||
| case notFound | ||
| case conflict | ||
| case tooManyRequests |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
서버 에러(5xx) 케이스 추가를 고려하세요.
현재 클라이언트 에러(4xx) 케이스만 추가되었습니다. 네트워크 추상화의 완전성을 위해 internalServerError(500), serviceUnavailable(503) 등 서버 에러 케이스도 추가하는 것을 권장합니다.
♻️ 서버 에러 케이스 추가 제안
case tooManyRequests
+ case internalServerError
+ case serviceUnavailable그리고 errorDescription에도 추가:
case .tooManyRequests:
return "요청 한도 초과"
+ case .internalServerError:
+ return "서버 내부 오류"
+ case .serviceUnavailable:
+ return "서비스 이용 불가"Also applies to: 47-58
케이스 정의 순서와 switch 문 순서를 일치시키세요.
enum 정의에서는 notFound(23번 줄)가 conflict(24번 줄)보다 먼저 나오지만, errorDescription의 switch 문에서는 conflict(53번 줄)가 notFound(55번 줄)보다 먼저 처리됩니다. 유지보수성을 위해 두 순서를 일치시키는 것이 좋습니다.
♻️ 순서를 일치시키는 제안
enum CherrishError: Error, LocalizedError, Equatable {
case DIFailedError
case decodingError
case URLError
case networkRequestFailed
case networkConnect
case networkError(code: Int, message: String)
case noData
case unknownError
case encodingError
case badRequest
case unauthorized
case forbidden
- case notFound
case conflict
+ case notFound
case tooManyRequests🤖 Prompt for AI Agents
In @Cherrish-iOS/Cherrish-iOS/Data/Network/CherrishError.swift around lines 20 -
25, The enum case order for badRequest, unauthorized, forbidden, notFound,
conflict, tooManyRequests does not match the order used in the errorDescription
switch; update the switch inside the errorDescription computed property so the
case labels process notFound before conflict (or reorder the enum to match the
switch), ensuring the sequence of case handlers matches the enum declaration
(referencing the enum cases notFound and conflict and the errorDescription
switch) to improve maintainability.
Comment on lines
+12
to
+18
| protocol NetworkService { | ||
| func request<T: Decodable>( | ||
| _ endPoint: EndPoint, | ||
| decodingType: T.Type | ||
| ) async throws -> T | ||
| func request(_ endPoint: EndPoint) async throws | ||
| } |
There was a problem hiding this comment.
decodingType 미사용 파라미터로 인한 경고/빌드 실패 가능성 제거 필요
구현부에서 decodingType를 전혀 쓰지 않아 Swift 경고가 발생합니다(“treat warnings as errors”면 CI에서 막힐 수 있음). 호출부에서 타입 명시를 강제하려는 의도라면 “의도적 미사용” 형태로 선언을 바꾸는 편이 안전합니다.
제안 diff
protocol NetworkService {
func request<T: Decodable>(
_ endPoint: EndPoint,
- decodingType: T.Type
+ decodingType _: T.Type
) async throws -> T
func request(_ endPoint: EndPoint) async throws
}📝 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
| protocol NetworkService { | |
| func request<T: Decodable>( | |
| _ endPoint: EndPoint, | |
| decodingType: T.Type | |
| ) async throws -> T | |
| func request(_ endPoint: EndPoint) async throws | |
| } | |
| protocol NetworkService { | |
| func request<T: Decodable>( | |
| _ endPoint: EndPoint, | |
| decodingType _: T.Type | |
| ) async throws -> T | |
| func request(_ endPoint: EndPoint) async throws | |
| } |
🤖 Prompt for AI Agents
In @Cherrish-iOS/Cherrish-iOS/Data/Network/NetworkService.swift around lines 12
- 18, Change the protocol method signature so the type-only parameter is
explicitly an intentionally unused parameter (e.g. func request<T: Decodable>(_
endPoint: EndPoint, _: T.Type) async throws -> T), and update any conforming
implementations to match (either name the parameter _ or prefix the parameter
usage with _ in the implementation) so the compiler no longer emits
unused-parameter warnings for decodingType while still forcing callers to
provide the generic type.
Comment on lines
22
to
65
| func request<T: Decodable>(_ endPoint: EndPoint, decodingType: T.Type) async throws -> T { | ||
| requestLogger(endPoint) | ||
|
|
||
| let response = await AF.request( | ||
| endPoint.requestURL, | ||
| method: endPoint.method, | ||
| parameters: endPoint.bodyParameters, | ||
| encoding: endPoint.parameterEncoding, | ||
| headers: endPoint.headers.value | ||
| ) | ||
| .validate() | ||
| .serializingData() | ||
| .response | ||
|
|
||
| responseLogger(response) | ||
|
|
||
| if let error = response.error { | ||
| let statusCode = response.response?.statusCode ?? -1 | ||
|
|
||
| if let data = response.data, | ||
| let errorResponse = try? JSONDecoder().decode(EmptyResponseDTO.self, from: data) { | ||
| let cherrishError = handleError(statusCode, errorResponse.message) | ||
| CherrishLogger.error(cherrishError) | ||
| throw cherrishError | ||
| } else { | ||
| CherrishLogger.error(error) | ||
| throw error | ||
| } | ||
| } | ||
|
|
||
| guard let data = response.data else { | ||
| CherrishLogger.error(CherrishError.noData) | ||
| throw CherrishError.noData | ||
| } | ||
|
|
||
| let decoded = try JSONDecoder().decode(BaseResponseDTO<T>.self, from: data) | ||
|
|
||
| guard let decodedData = decoded.data else { | ||
| CherrishLogger.error(CherrishError.noData) | ||
| throw CherrishError.noData | ||
| } | ||
|
|
||
| return decodedData | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Alamofire async/await serializingData response validate error handling status code
💡 Result:
Short recipe + example.
Key points
- Call validate(...) before serializing so invalid status codes produce an error. [1][2]
- Use serializingData() (or serializingDecodable()) with async/await and access .value or await the DataResponse to inspect response metadata like statusCode. [1][3]
- Catch AFError.responseValidationFailed / AFError.responseSerializationFailed or inspect response.response?.statusCode / error.responseCode to handle specific HTTP codes. [3][4]
Example (async/await)
-
returns Data and throws detailed Alamofire errors you can inspect:
func fetchRawData(_ url: URL) async throws -> Data {
let request = AF.request(url)
.validate(statusCode: 200..<300) // <- validate first
let response = await request.serializingData().response // DataResponse<Data, AFError>// You can inspect HTTP status code even on failure: if let code = response.response?.statusCode, !(200..<300).contains(code) { // handle specific status codes (e.g. refresh token on 401) throw MyNetworkError.http(code) } switch response.result { case .success(let data): return data case .failure(let error): // catch validation vs serialization errors if case let .responseValidationFailed(reason) = error { // reason can be .unacceptableStatusCode(code) etc. // also error.responseCode == HTTP status code (optional) } throw error }}
Alternative concise style using value and try/await:
do {
let data = try await AF.request(url)
.validate(statusCode: 200..<300)
.serializingData()
.value // throws AFError on validation/serialization failure
// success
} catch let af as AFError {
// af.responseCode or if case .responseValidationFailed(reason: .unacceptableStatusCode(let code)) ...
}
Notes
- AFError.responseCode and the DataResponse.response?.statusCode are the typical places to read the HTTP status code. [3][4]
- For JSON -> Decodable use serializingDecodable(_:emptyResponseCodes:) and same error handling. [1][5]
- For custom validation, use .validate { request, response, data in ... } to return custom failures with status-aware logic. [2]
Sources
[1] Alamofire serializing & serializers docs.
[2] Alamofire validate() docs / custom validation.
[3] StackOverflow examples showing inspecting request.response?.statusCode and AFError usage.
[4] Alamofire DataResponse/response structure docs.
[5] DecodableResponseSerializer docs.
에러 핸들링 일관성 + 디코딩 실패 로깅 개선 필요
현재 구현에서는 .validate() 실패 시 EmptyResponseDTO 디코딩 성공 여부에 따라 서로 다른 에러 타입이 던져집니다. 디코딩 성공 시 CherrishError를 던지고, 실패 시 AFError를 그대로 던져 호출부의 에러 처리 로직이 복잡해집니다. Alamofire의 async/await 패턴에서 .validate() 후 실패할 때도 response.response?.statusCode와 response.data가 모두 접근 가능하므로, 모든 에러를 상태코드 기반으로 일관되게 handleError()를 통해 매핑하는 것이 권장됩니다. 또한 BaseResponseDTO<T> 디코딩 실패 시 로깅이 없어 원인 파악이 어렵습니다.
제안 diff(핵심: 상태코드 기반 일관성 있는 에러 매핑 + 디코딩 실패 로깅)
final class DefaultNetworkService: NetworkService {
- func request<T: Decodable>(_ endPoint: EndPoint, decodingType: T.Type) async throws -> T {
+ func request<T: Decodable>(_ endPoint: EndPoint, decodingType _: T.Type) async throws -> T {
requestLogger(endPoint)
let response = await AF.request(
endPoint.requestURL,
method: endPoint.method,
parameters: endPoint.bodyParameters,
encoding: endPoint.parameterEncoding,
headers: endPoint.headers.value
)
.validate()
.serializingData()
.response
responseLogger(response)
if let error = response.error {
- let statusCode = response.response?.statusCode ?? -1
-
- if let data = response.data,
- let errorResponse = try? JSONDecoder().decode(EmptyResponseDTO.self, from: data) {
- let cherrishError = handleError(statusCode, errorResponse.message)
- CherrishLogger.error(cherrishError)
- throw cherrishError
- } else {
- CherrishLogger.error(error)
- throw error
- }
+ let statusCode = response.response?.statusCode ?? -1
+ let message: String = {
+ guard let data = response.data else { return "" }
+ return (try? JSONDecoder().decode(EmptyResponseDTO.self, from: data).message) ?? ""
+ }()
+ let cherrishError = handleError(statusCode, message)
+ CherrishLogger.error(cherrishError)
+ throw cherrishError
}
guard let data = response.data else {
CherrishLogger.error(CherrishError.noData)
throw CherrishError.noData
}
- let decoded = try JSONDecoder().decode(BaseResponseDTO<T>.self, from: data)
+ let decoded: BaseResponseDTO<T>
+ do {
+ decoded = try JSONDecoder().decode(BaseResponseDTO<T>.self, from: data)
+ } catch {
+ CherrishLogger.error(error)
+ throw error
+ }
guard let decodedData = decoded.data else {
CherrishLogger.error(CherrishError.noData)
throw CherrishError.noData
}
return decodedData
}🤖 Prompt for AI Agents
In @Cherrish-iOS/Cherrish-iOS/Data/Network/NetworkService.swift around lines 22
- 65, The request(_:decodingType:) error handling is inconsistent: when
response.error exists you sometimes throw the AF error instead of mapping to
CherrishError via handleError, and decoding failures of BaseResponseDTO lack
logging. Change the error branch that checks response.error to always obtain
statusCode = response.response?.statusCode ?? -1 and try to decode an error
message from response.data (using EmptyResponseDTO or fallback), call
handleError(statusCode, message) and throw that mapped error (and log it with
CherrishLogger.error); additionally, wrap the
JSONDecoder().decode(BaseResponseDTO<T>.self, from: data) in do/catch so that
any decoding error logs CherrishLogger.error with the decoding Error and the raw
response data (string or hex) before rethrowing a clear CherrishError (or the
decoding error) so callers can diagnose failures. Ensure you reference
request(_:decodingType:), EmptyResponseDTO, BaseResponseDTO, handleError, and
CherrishLogger when making these changes.
Comment on lines
67
to
94
| func request(_ endPoint: EndPoint) async throws { | ||
| requestLogger(endPoint) | ||
|
|
||
| let response = await AF.request( | ||
| endPoint.requestURL, | ||
| method: endPoint.method, | ||
| parameters: endPoint.bodyParameters, | ||
| encoding: endPoint.parameterEncoding, | ||
| headers: endPoint.headers.value | ||
| ) | ||
| .validate() | ||
| .serializingData() | ||
| .response | ||
|
|
||
| responseLogger(response) | ||
|
|
||
| if let error = response.error { | ||
| let statusCode = response.response?.statusCode ?? -1 | ||
|
|
||
| if let data = response.data, | ||
| let errorResponse = try? JSONDecoder().decode(EmptyResponseDTO.self, from: data) { | ||
| let cherrishError = handleError(statusCode, errorResponse.message) | ||
| throw cherrishError | ||
| } | ||
|
|
||
| throw error | ||
| } | ||
| } |
There was a problem hiding this comment.
Non-decoding request도 에러 로깅/매핑 정책을 디코딩 버전과 통일 필요
현재 이 메서드는 (1) CherrishLogger.error(...)를 호출하지 않고, (2) 에러 바디 디코딩 실패 시 AFError를 그대로 던집니다. 위의 디코딩 버전과 정책/관측가능성이 달라 운영에서 원인 추적이 어려워집니다.
🤖 Prompt for AI Agents
In @Cherrish-iOS/Cherrish-iOS/Data/Network/NetworkService.swift around lines 67
- 94, The non-decoding overload request(_ endPoint: EndPoint) does not follow
the same error logging/mapping policy: it currently throws AFError directly and
omits CherrishLogger.error; update the function so that whenever response.error
exists you (1) try to decode the error body into EmptyResponseDTO as now but if
decoding fails still extract response.data (as string or hex) and statusCode,
(2) call CherrishLogger.error with contextual info (endpoint, statusCode,
response body/error description), (3) map the observed statusCode/message into a
Cherrish error via handleError(statusCode, message) and throw that mapped error
instead of raw AFError, ensuring parity with the decoding variant (use the same
keys/log format as the decoding path).
Comment on lines
+96
to
+109
| private func requestLogger(_ endPoint: EndPoint) { | ||
| CherrishLogger.network("[Request Start]") | ||
| CherrishLogger.network("URL: \(endPoint.requestURL)") | ||
| CherrishLogger.network("Method: \(endPoint.method.rawValue)") | ||
| CherrishLogger.network("Headers: \(endPoint.headers.value)") | ||
| CherrishLogger.network("Parameters: \(String(describing: endPoint.bodyParameters))") | ||
| } | ||
|
|
||
| private func responseLogger<T>(_ response: DataResponse<T, AFError>) { | ||
| CherrishLogger.network("[Response Start]") | ||
| CherrishLogger.network("StatusCode: \(String(describing: response.response?.statusCode))") | ||
| CherrishLogger.network("Header: \(String(describing: response.response?.headers))") | ||
| CherrishLogger.network("Description: \(String(describing: response.response?.description))") | ||
| } |
There was a problem hiding this comment.
요청/응답 로깅에 민감정보(Authorization 등) 노출 위험 — 마스킹 필요
Headers: \(endPoint.headers.value)를 그대로 찍으면 토큰/세션/개인정보가 로그로 남을 수 있습니다. 최소한 Authorization/Cookie 등은 마스킹하거나 화이트리스트 기반으로 로깅하는 편이 안전합니다.
제안 diff(간단 마스킹 예시)
private func requestLogger(_ endPoint: EndPoint) {
CherrishLogger.network("[Request Start]")
CherrishLogger.network("URL: \(endPoint.requestURL)")
CherrishLogger.network("Method: \(endPoint.method.rawValue)")
- CherrishLogger.network("Headers: \(endPoint.headers.value)")
+ CherrishLogger.network("Headers: \(String(describing: endPoint.headers.value))") // TODO: Authorization/Cookie 등 마스킹 적용
CherrishLogger.network("Parameters: \(String(describing: endPoint.bodyParameters))")
}📝 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
| private func requestLogger(_ endPoint: EndPoint) { | |
| CherrishLogger.network("[Request Start]") | |
| CherrishLogger.network("URL: \(endPoint.requestURL)") | |
| CherrishLogger.network("Method: \(endPoint.method.rawValue)") | |
| CherrishLogger.network("Headers: \(endPoint.headers.value)") | |
| CherrishLogger.network("Parameters: \(String(describing: endPoint.bodyParameters))") | |
| } | |
| private func responseLogger<T>(_ response: DataResponse<T, AFError>) { | |
| CherrishLogger.network("[Response Start]") | |
| CherrishLogger.network("StatusCode: \(String(describing: response.response?.statusCode))") | |
| CherrishLogger.network("Header: \(String(describing: response.response?.headers))") | |
| CherrishLogger.network("Description: \(String(describing: response.response?.description))") | |
| } | |
| private func requestLogger(_ endPoint: EndPoint) { | |
| CherrishLogger.network("[Request Start]") | |
| CherrishLogger.network("URL: \(endPoint.requestURL)") | |
| CherrishLogger.network("Method: \(endPoint.method.rawValue)") | |
| CherrishLogger.network("Headers: \(String(describing: endPoint.headers.value))") // TODO: Authorization/Cookie 등 마스킹 적용 | |
| CherrishLogger.network("Parameters: \(String(describing: endPoint.bodyParameters))") | |
| } | |
| private func responseLogger<T>(_ response: DataResponse<T, AFError>) { | |
| CherrishLogger.network("[Response Start]") | |
| CherrishLogger.network("StatusCode: \(String(describing: response.response?.statusCode))") | |
| CherrishLogger.network("Header: \(String(describing: response.response?.headers))") | |
| CherrishLogger.network("Description: \(String(describing: response.response?.description))") | |
| } |
Comment on lines
10
to
+16
| struct DefaultTestRepository: TestInterface { | ||
| private let networkService: NetworkService | ||
|
|
||
| init(networkService: NetworkService) { | ||
| self.networkService = networkService | ||
| } | ||
|
|
There was a problem hiding this comment.
주입한 networkService 미사용(의도 명시 또는 최소 사용 예시 권장)
networkService가 현재 로직에서 전혀 사용되지 않아 “추후 사용 예정”임을 주석/TODO로라도 남기지 않으면 코드가 방치된 것으로 보일 수 있습니다(특히 PR에 “임시 예시 파일”이 포함된 상황이라 더더욱).
🤖 Prompt for AI Agents
In @Cherrish-iOS/Cherrish-iOS/Data/Repository/TestRepository.swift around lines
10 - 16, DefaultTestRepository currently stores the injected networkService in
its initializer but never uses it, which looks like dead code; either use it in
a minimal example method (e.g., add a simple fetch/testNetwork() method that
calls networkService.request or similar) or document intent with a TODO/comment
in the struct and mark the property as used in tests/examples; locate
DefaultTestRepository and the private let networkService and update by adding a
short example method or a clarifying comment/TODO referencing networkService and
the init so the injection is intentional and not unused.
wotjs020708
approved these changes
Jan 11, 2026
Contributor
wotjs020708
left a comment
wotjs020708
left a comment
There was a problem hiding this comment.
사용해봐야 이해가 갈 거 같네욤 ㅠㅠ 고생하셨습니담
wotjs020708
approved these changes
Jan 11, 2026
Comment on lines
+11
to
+15
| private let networkService: NetworkService | ||
|
|
||
| init() { | ||
| self.networkService = DefaultNetworkService() | ||
| } |
Contributor
There was a problem hiding this comment.
저도 이 부분에 동의합니다.내용을 완벽히 이해 하지는 않았지만 내용 확인하면 좋을 거 같아요
sum130
approved these changes
Jan 12, 2026
Contributor
sum130
left a comment
sum130
left a comment
There was a problem hiding this comment.
수고하셨숨니다!!! 중간에 오타수정만 하면 될 것 가타유 좋습니댜
Comment on lines
+11
to
+15
| private let networkService: NetworkService | ||
|
|
||
| init() { | ||
| self.networkService = DefaultNetworkService() | ||
| } |
Contributor
There was a problem hiding this comment.
찾아보니까 관리하기는 편하다는데 지금 이해하기도 쉽지않았는데 저희가 잘 활용할 수 있을지..! 좀 걱정되긴합니당 한 번 생각해주세요
Comment on lines
45
to
61
| case .unknownError: | ||
| return "알 수 없는 오류" | ||
| case .badRequest: | ||
| return "잘못된 요청" | ||
| case .unauthorized: | ||
| return "인증 실패" | ||
| case .forbidden: | ||
| return "권한 없음" | ||
| case .conflict: | ||
| return "리소스 충돌" | ||
| case .notFound: | ||
| return "리소스를 찾을 수 없음" | ||
| case .tooManyRequests: | ||
| return "요청 한도 초과" | ||
| } | ||
| } | ||
| } |
| var path: String { get } | ||
| var method: HTTPMethod { get } | ||
| var headers: HeaderType { get } | ||
| var parameterEncoding: ParameterEncoding { get } |
Comment on lines
+8
to
+16
| enum TestAPI { | ||
| case login | ||
| case logout | ||
| } | ||
|
|
||
| /// extension에서 endpoint를 채택하여 구현 | ||
| // extension TestAPI: EndPoint { | ||
| // | ||
| // } |
|
|
||
| private func handleError(_ statusCode: Int, _ errorResponse: String) -> CherrishError { | ||
| let error: CherrishError | ||
|
|
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In @Cherrish-iOS/Cherrish-iOS/Data/Network/EndPoint/EndPoint.swift:
- Around line 21-22: Fix the typos in the EndPoint documentation comment: change
the first comment token from "[mission: mission[" to "[mission: mission]" and
correct the misspelled "bodyParameteres" to "bodyParameters" in the
EndPoint.swift comment block (look for the EndPoint or EndPoint-related doc
comment near those lines to update).
- Line 30: The EndPoint protocol has a typo in the property name
`queryParmeters` — rename it to `queryParameters` everywhere: update the
protocol declaration (replace `var queryParmeters: [String: String]?` with `var
queryParameters: [String: String]?`), update the usage at the other occurrence
flagged (previously line 46) and any conforming types, implementations, test
code, and docs to use `queryParameters` so compilation and API consistency are
preserved.
- Around line 41-54: The current URL construction returns a forced-empty URL via
URL(string: "")! which is unsafe and confusing; change the computed
property/method that builds the URL (e.g., requestURL) to return an optional URL
(URL?) or make it throwing: replace the forced empty-URL returns with nil (if
using URL?) or throw a descriptive error (if throwing), update the guard
branches that log CherrishError.URLError to return nil or rethrow, and then
update all callers of requestURL to handle the optional (guard/if-let) or catch
the thrown error accordingly so no forced empty URL is produced.
In @Cherrish-iOS/Cherrish-iOS/Data/Network/NetworkService.swift:
- Around line 41-53: The if-block handling response.error is indented with 3
spaces instead of the project-standard 4; fix the indentation for the entire
block starting at the `if let error = response.error {` line so nested lines
(statusCode, data check, decoding to EmptyResponseDTO, calls to `handleError`
and `CherrishLogger.error`, and the thrown errors) use 4-space indentation to
match the rest of the file and keep consistent styling.
- Around line 101-107: The requestLogger(_ endPoint: EndPoint) currently logs
raw headers and bodyParameters (via endPoint.headers.value and
endPoint.bodyParameters) which may expose sensitive tokens; update requestLogger
to avoid logging secrets in production by either wrapping header/parameter logs
in a compile-time DEBUG guard (#if DEBUG ... #else log "[REDACTED]" ...) or by
filtering/masking known sensitive header keys (e.g., "Authorization", "Cookie",
"Set-Cookie", "X-Auth-Token") and replacing their values with masked text before
calling CherrishLogger.network, and apply the same masking for bodyParameters.
- Around line 22-68: Duplicate request/error handling logic should be extracted
into a single private helper (e.g., performRequest) that takes EndPoint, runs
requestLogger, AF.request(...).validate().serializingData().response,
responseLogger, handles response.error by decoding EmptyResponseDTO and mapping
to handleError(statusCode:message:) or rethrowing the AF error, and returns
Data?; then update both request<T: Decodable>(_ endPoint:decodingType:) and
request(_ endPoint:) to call performRequest, check for nil data and throw
CherrishError.noData, and in the generic variant decode BaseResponseDTO<T> and
return decoded.data (or throw noData) so all duplicated logging/validation/error
branches are consolidated in performRequest.
- Around line 116-140: Update error handling to explicitly surface 5xx
responses: add a serverError case to the CherrishError enum (e.g.,
serverError(code: Int, message: String)) and modify the
NetworkService.handleError(_ statusCode: Int, _ errorResponse: String) function
to map status codes 500...599 to .serverError(code: statusCode, message:
errorResponse) instead of falling through to the default networkError; keep
other mappings intact and return the new .serverError for server-side failures.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
Cherrish-iOS/Cherrish-iOS/Data/Network/EndPoint/EndPoint.swiftCherrish-iOS/Cherrish-iOS/Data/Network/NetworkService.swiftCherrish-iOS/Cherrish-iOS/Presentation/Global/Component.swift
💤 Files with no reviewable changes (1)
- Cherrish-iOS/Cherrish-iOS/Presentation/Global/Component.swift
🧰 Additional context used
🧬 Code graph analysis (1)
Cherrish-iOS/Cherrish-iOS/Data/Network/NetworkService.swift (1)
Cherrish-iOS/Cherrish-iOS/Core/CherrishLogger.swift (3)
error(147-153)data(139-145)network(123-129)
🪛 SwiftLint (0.57.0)
Cherrish-iOS/Cherrish-iOS/Data/Network/EndPoint/EndPoint.swift
[Warning] 38-38: TODOs should be resolved (서버 배포 후 수정)
(todo)
[Warning] 72-72: TODOs should be resolved (서버 구조에 맞춰 수정)
(todo)
🔇 Additional comments (2)
Cherrish-iOS/Cherrish-iOS/Data/Network/EndPoint/EndPoint.swift (1)
61-76: HeaderType 구현 적절함기본 헤더와 인증 헤더를 enum으로 분리한 구현이 깔끔합니다. TODO 주석도 향후 서버 구조에 맞춰 수정이 필요함을 잘 표시해두었습니다.
Cherrish-iOS/Cherrish-iOS/Data/Network/NetworkService.swift (1)
12-18: NetworkService 프로토콜 설계 적절함제네릭을 활용한 디코딩 메서드와 응답 무시 메서드를 분리한 설계가 깔끔합니다. 의존성 주입을 위한 프로토콜 추상화도 잘 되어 있습니다.
Comment on lines
41
to
54
| guard var urlComponents = URLComponents(string: urlString) else { | ||
| CherrishLogger.error(CherrishError.URLError) | ||
| return URL(string: "")! | ||
| } | ||
|
|
||
| if let queryParmeters { | ||
| urlComponents.queryItems = queryParmeters.map { | ||
| URLQueryItem(name: $0, value: $1) | ||
| } | ||
| } | ||
|
|
||
| guard let url = urlComponents.url else { | ||
| CherrishLogger.error(CherrishError.URLError) | ||
| return URL(string: "")! |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
URL 생성 실패 시 빈 URL 반환 패턴 개선 필요
URL(string: "")! 반환은 앱 크래시는 발생하지 않지만, 네트워크 요청 시 혼란스러운 에러를 유발할 수 있습니다. requestURL을 옵셔널(URL?)로 변경하거나 throwing computed property로 리팩터링을 고려해주세요.
♻️ 옵셔널 URL 반환 패턴 제안
protocol EndPoint {
// ... other properties
- var requestURL: URL { get }
+ var requestURL: URL? { get }
}
extension EndPoint {
- var requestURL: URL {
+ var requestURL: URL? {
let baseURL = "" // TODO: 서버 배포 후 수정
let urlString = baseURL + basePath + path
guard var urlComponents = URLComponents(string: urlString) else {
CherrishLogger.error(CherrishError.URLError)
- return URL(string: "")!
+ return nil
}
- if let queryParmeters {
- urlComponents.queryItems = queryParmeters.map {
+ if let queryParameters {
+ urlComponents.queryItems = queryParameters.map {
URLQueryItem(name: $0, value: $1)
}
}
- guard let url = urlComponents.url else {
- CherrishLogger.error(CherrishError.URLError)
- return URL(string: "")!
- }
-
- return url
+ return urlComponents.url
}
}🤖 Prompt for AI Agents
In @Cherrish-iOS/Cherrish-iOS/Data/Network/EndPoint/EndPoint.swift around lines
41 - 54, The current URL construction returns a forced-empty URL via URL(string:
"")! which is unsafe and confusing; change the computed property/method that
builds the URL (e.g., requestURL) to return an optional URL (URL?) or make it
throwing: replace the forced empty-URL returns with nil (if using URL?) or throw
a descriptive error (if throwing), update the guard branches that log
CherrishError.URLError to return nil or rethrow, and then update all callers of
requestURL to handle the optional (guard/if-let) or catch the thrown error
accordingly so no forced empty URL is produced.
Comment on lines
+22
to
+68
| func request<T: Decodable>( | ||
| _ endPoint: EndPoint, | ||
| decodingType: T.Type) | ||
| async throws -> T { | ||
| requestLogger(endPoint) | ||
|
|
||
| let response = await AF.request( | ||
| endPoint.requestURL, | ||
| method: endPoint.method, | ||
| parameters: endPoint.bodyParameters, | ||
| encoding: endPoint.parameterEncoding, | ||
| headers: endPoint.headers.value | ||
| ) | ||
| .validate() | ||
| .serializingData() | ||
| .response | ||
|
|
||
| responseLogger(response) | ||
|
|
||
| if let error = response.error { | ||
| let statusCode = response.response?.statusCode ?? -1 | ||
|
|
||
| if let data = response.data, | ||
| let errorResponse = try? JSONDecoder().decode(EmptyResponseDTO.self, from: data) { | ||
| let cherrishError = handleError(statusCode, errorResponse.message) | ||
| CherrishLogger.error(cherrishError) | ||
| throw cherrishError | ||
| } else { | ||
| CherrishLogger.error(error) | ||
| throw error | ||
| } | ||
| } | ||
|
|
||
| guard let data = response.data else { | ||
| CherrishLogger.error(CherrishError.noData) | ||
| throw CherrishError.noData | ||
| } | ||
|
|
||
| let decoded = try JSONDecoder().decode(BaseResponseDTO<T>.self, from: data) | ||
|
|
||
| guard let decodedData = decoded.data else { | ||
| CherrishLogger.error(CherrishError.noData) | ||
| throw CherrishError.noData | ||
| } | ||
|
|
||
| return decodedData | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
코드 중복: 요청 및 에러 처리 로직 추출 권장
두 request 메서드가 거의 동일한 요청/에러 처리 로직을 공유합니다. 공통 로직을 private 메서드로 추출하면 유지보수성이 향상됩니다.
♻️ 공통 로직 추출 제안
private func performRequest(_ endPoint: EndPoint) async throws -> Data? {
requestLogger(endPoint)
let response = await AF.request(
endPoint.requestURL,
method: endPoint.method,
parameters: endPoint.bodyParameters,
encoding: endPoint.parameterEncoding,
headers: endPoint.headers.value
)
.validate()
.serializingData()
.response
responseLogger(response)
if let error = response.error {
let statusCode = response.response?.statusCode ?? -1
if let data = response.data,
let errorResponse = try? JSONDecoder().decode(EmptyResponseDTO.self, from: data) {
let cherrishError = handleError(statusCode, errorResponse.message)
CherrishLogger.error(cherrishError)
throw cherrishError
} else {
CherrishLogger.error(error)
throw error
}
}
return response.data
}
func request<T: Decodable>(_ endPoint: EndPoint, decodingType: T.Type) async throws -> T {
guard let data = try await performRequest(endPoint) else {
CherrishLogger.error(CherrishError.noData)
throw CherrishError.noData
}
let decoded = try JSONDecoder().decode(BaseResponseDTO<T>.self, from: data)
guard let decodedData = decoded.data else {
CherrishLogger.error(CherrishError.noData)
throw CherrishError.noData
}
return decodedData
}
func request(_ endPoint: EndPoint) async throws {
_ = try await performRequest(endPoint)
}Also applies to: 70-99
🤖 Prompt for AI Agents
In @Cherrish-iOS/Cherrish-iOS/Data/Network/NetworkService.swift around lines 22
- 68, Duplicate request/error handling logic should be extracted into a single
private helper (e.g., performRequest) that takes EndPoint, runs requestLogger,
AF.request(...).validate().serializingData().response, responseLogger, handles
response.error by decoding EmptyResponseDTO and mapping to
handleError(statusCode:message:) or rethrowing the AF error, and returns Data?;
then update both request<T: Decodable>(_ endPoint:decodingType:) and request(_
endPoint:) to call performRequest, check for nil data and throw
CherrishError.noData, and in the generic variant decode BaseResponseDTO<T> and
return decoded.data (or throw noData) so all duplicated logging/validation/error
branches are consolidated in performRequest.
Comment on lines
+41
to
+53
| if let error = response.error { | ||
| let statusCode = response.response?.statusCode ?? -1 | ||
|
|
||
| if let data = response.data, | ||
| let errorResponse = try? JSONDecoder().decode(EmptyResponseDTO.self, from: data) { | ||
| let cherrishError = handleError(statusCode, errorResponse.message) | ||
| CherrishLogger.error(cherrishError) | ||
| throw cherrishError | ||
| } else { | ||
| CherrishLogger.error(error) | ||
| throw error | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
들여쓰기 불일치
Lines 41-53의 들여쓰기가 다른 코드와 일치하지 않습니다 (3칸 vs 4칸). 일관된 코드 스타일을 위해 수정을 권장합니다.
🤖 Prompt for AI Agents
In @Cherrish-iOS/Cherrish-iOS/Data/Network/NetworkService.swift around lines 41
- 53, The if-block handling response.error is indented with 3 spaces instead of
the project-standard 4; fix the indentation for the entire block starting at the
`if let error = response.error {` line so nested lines (statusCode, data check,
decoding to EmptyResponseDTO, calls to `handleError` and `CherrishLogger.error`,
and the thrown errors) use 4-space indentation to match the rest of the file and
keep consistent styling.
Comment on lines
+101
to
+107
| private func requestLogger(_ endPoint: EndPoint) { | ||
| CherrishLogger.network("[Request Start]") | ||
| CherrishLogger.network("URL: \(endPoint.requestURL)") | ||
| CherrishLogger.network("Method: \(endPoint.method.rawValue)") | ||
| CherrishLogger.network("Headers: \(endPoint.headers.value)") | ||
| CherrishLogger.network("Parameters: \(String(describing: endPoint.bodyParameters))") | ||
| } |
There was a problem hiding this comment.
민감한 정보 로깅 주의 필요
endPoint.headers.value를 로깅하면 인증 토큰(userID 등)이 로그에 노출될 수 있습니다. 프로덕션 빌드에서는 민감한 헤더를 마스킹하거나 로깅을 제한하는 것을 권장합니다.
🔒 민감 정보 마스킹 제안
private func requestLogger(_ endPoint: EndPoint) {
CherrishLogger.network("[Request Start]")
CherrishLogger.network("URL: \(endPoint.requestURL)")
CherrishLogger.network("Method: \(endPoint.method.rawValue)")
#if DEBUG
CherrishLogger.network("Headers: \(endPoint.headers.value)")
CherrishLogger.network("Parameters: \(String(describing: endPoint.bodyParameters))")
#else
CherrishLogger.network("Headers: [REDACTED]")
CherrishLogger.network("Parameters: [REDACTED]")
#endif
}🤖 Prompt for AI Agents
In @Cherrish-iOS/Cherrish-iOS/Data/Network/NetworkService.swift around lines 101
- 107, The requestLogger(_ endPoint: EndPoint) currently logs raw headers and
bodyParameters (via endPoint.headers.value and endPoint.bodyParameters) which
may expose sensitive tokens; update requestLogger to avoid logging secrets in
production by either wrapping header/parameter logs in a compile-time DEBUG
guard (#if DEBUG ... #else log "[REDACTED]" ...) or by filtering/masking known
sensitive header keys (e.g., "Authorization", "Cookie", "Set-Cookie",
"X-Auth-Token") and replacing their values with masked text before calling
CherrishLogger.network, and apply the same masking for bodyParameters.
Comment on lines
+116
to
+140
| private func handleError(_ statusCode: Int, _ errorResponse: String) -> CherrishError { | ||
| let error: CherrishError | ||
|
|
||
| switch statusCode { | ||
| case 400: | ||
| error = .badRequest | ||
| case 401: | ||
| error = .unauthorized | ||
| case 403: | ||
| error = .forbidden | ||
| case 404: | ||
| error = .notFound | ||
| case 409: | ||
| error = .conflict | ||
| case 429: | ||
| error = .tooManyRequests | ||
| default: | ||
| error = .networkError( | ||
| code: statusCode, | ||
| message: errorResponse | ||
| ) | ||
| } | ||
|
|
||
| return error | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
5xx 서버 에러 처리 추가 권장
현재 500-599 상태 코드는 default 케이스로 처리됩니다. 서버 에러를 명시적으로 처리하면 사용자에게 더 적절한 에러 메시지를 제공할 수 있습니다.
♻️ 서버 에러 케이스 추가 제안
case 429:
error = .tooManyRequests
+ case 500...599:
+ error = .serverError(code: statusCode, message: errorResponse)
default:
error = .networkError(
code: statusCode,
message: errorResponse
)CherrishError에 serverError 케이스 추가가 필요합니다.
🤖 Prompt for AI Agents
In @Cherrish-iOS/Cherrish-iOS/Data/Network/NetworkService.swift around lines 116
- 140, Update error handling to explicitly surface 5xx responses: add a
serverError case to the CherrishError enum (e.g., serverError(code: Int,
message: String)) and modify the NetworkService.handleError(_ statusCode: Int, _
errorResponse: String) function to map status codes 500...599 to
.serverError(code: statusCode, message: errorResponse) instead of falling
through to the default networkError; keep other mappings intact and return the
new .serverError for server-side failures.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @Cherrish-iOS/Cherrish-iOS/Data/Network/EndPoint/EndPoint.swift:
- Line 38: The EndPoint.swift currently hardcodes let baseURL = "" which must be
replaced with a configurable source; change EndPoint to read the base URL from a
configuration provider (e.g., app configuration struct, Info.plist key, or an
environment/xcconfig value) instead of a literal string, add a Configuration or
AppSettings accessor used by EndPoint to return the correct URL (e.g.,
Configuration.shared.baseURL or Bundle/ProcessInfo lookup), provide a sensible
fallback or assert if missing, and update any tests or callers of EndPoint to
use the configuration injection where applicable.
- Around line 21-22: Fix the typos in the EndPoint.swift documentation comments:
change the queryParameters example from `[mission: mission[` to `["mission":
mission]` and correct the misspelled `bodyParameteres` to `bodyParameters` so
the comment for queryParameters/bodyParameters is accurate and consistent with
the actual parameter naming.
- Line 30: The EndPoint protocol has a typo in the property name
`queryParmeters`; rename it to `queryParameters` in the protocol declaration and
update every usage: adjust the corresponding extension(s) and every type that
conforms to EndPoint (replace `queryParmeters` with `queryParameters`), update
any computed properties, initializers or callers, and run the build to fix
remaining references/imports and resolve compiler errors.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
Cherrish-iOS/Cherrish-iOS/Data/Network/EndPoint/EndPoint.swift
🧰 Additional context used
🧬 Code graph analysis (1)
Cherrish-iOS/Cherrish-iOS/Data/Network/EndPoint/EndPoint.swift (1)
Cherrish-iOS/Cherrish-iOS/Core/CherrishLogger.swift (1)
error(147-153)
🪛 SwiftLint (0.57.0)
Cherrish-iOS/Cherrish-iOS/Data/Network/EndPoint/EndPoint.swift
[Warning] 38-38: TODOs should be resolved (서버 배포 후 수정)
(todo)
🔇 Additional comments (1)
Cherrish-iOS/Cherrish-iOS/Data/Network/EndPoint/EndPoint.swift (1)
61-76: HeaderType 구현이 깔끔합니다.
X-User-Id헤더에 사용자 ID를 직접 노출하는 방식이 서버 요구사항에 맞는지 확인해 주세요. 보안이 중요한 API의 경우 토큰 기반 인증을 고려할 수 있습니다.
Comment on lines
+21
to
+22
| /// queryParameters: [mission: mission[ | ||
| /// bodyParameteres: nil |
There was a problem hiding this comment.
문서 주석에 오타가 있습니다.
- Line 21:
[mission: mission[→["mission": mission] - Line 22:
bodyParameteres→bodyParameters
🤖 Prompt for AI Agents
In @Cherrish-iOS/Cherrish-iOS/Data/Network/EndPoint/EndPoint.swift around lines
21 - 22, Fix the typos in the EndPoint.swift documentation comments: change the
queryParameters example from `[mission: mission[` to `["mission": mission]` and
correct the misspelled `bodyParameteres` to `bodyParameters` so the comment for
queryParameters/bodyParameters is accurate and consistent with the actual
parameter naming.
Comment on lines
+41
to
+44
| guard var urlComponents = URLComponents(string: urlString) else { | ||
| CherrishLogger.error(CherrishError.URLError) | ||
| return URL(string: "")! | ||
| } |
There was a problem hiding this comment.
빈 URL 반환 대신 적절한 에러 처리를 고려하세요.
URL 생성 실패 시 빈 URL(URL(string: "")!)을 반환하면 네트워크 요청이 무의미한 상태로 진행되어 디버깅이 어려워집니다. fatalError를 사용하거나, requestURL을 throwing 프로퍼티/메서드로 변경하여 호출부에서 에러를 처리하도록 하는 것이 좋습니다.
♻️ 대안 1: throwing 메서드로 변경
protocol EndPoint {
// ... other properties ...
- var requestURL: URL { get }
+ func makeRequestURL() throws -> URL
}
extension EndPoint {
- var requestURL: URL {
+ func makeRequestURL() throws -> URL {
let baseURL = "" // TODO: 서버 배포 후 수정
let urlString = baseURL + basePath + path
guard var urlComponents = URLComponents(string: urlString) else {
- CherrishLogger.error(CherrishError.URLError)
- return URL(string: "")!
+ throw CherrishError.URLError
}
- if let queryParmeters {
- urlComponents.queryItems = queryParmeters.map {
+ if let queryParameters {
+ urlComponents.queryItems = queryParameters.map {
URLQueryItem(name: $0, value: $1)
}
}
guard let url = urlComponents.url else {
- CherrishLogger.error(CherrishError.URLError)
- return URL(string: "")!
+ throw CherrishError.URLError
}
return url
}
}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In @Cherrish-iOS/Cherrish-iOS/Data/Network/EndPoint/EndPoint.swift:
- Around line 8-11: EndPoint currently exposes Alamofire types (HTTPMethod,
HTTPHeaders, Parameters, ParameterEncoding) which couples your API to Alamofire;
replace those direct references with your own lightweight abstractions (e.g.,
EndpointHTTPMethod enum, Headers/Parameters typealias or wrapper structs, and a
ParameterEncoding protocol) in the EndPoint protocol, then implement an
adapter/mapper inside your networking layer that converts these abstractions to
Alamofire.HTTPMethod, HTTPHeaders, Parameters and ParameterEncoding when
performing requests; update implementations of EndPoint-conforming types to use
the new abstractions so the networking implementation can be swapped without
changing the protocol.
- Around line 12-34: The EndPoint.requestURL should be changed to a
failure-aware API: replace the var requestURL: URL with a throwing factory like
func makeURL() throws -> URL (or at minimum var requestURL: URL?), then update
every conforming type to implement makeURL() using URLComponents/URL(string:)
and throw a descriptive error on failure, and update all callers to handle the
thrown error (try/try? or propagate) so URL construction failures are explicit
instead of causing forced-unwrap crashes; refer to EndPoint, requestURL, and any
concrete endpoint types for edits.
- Line 38: EndPoint.swift currently sets let baseURL = "" which will break
networking if left empty; replace this with a build-config-backed value and a
safe default: read the base URL from Info.plist or an xcconfig-defined key
(e.g., Bundle.main.object(forInfoDictionaryKey: "BASE_URL") as? String) and fall
back to a valid default like "https://api.example.com", or assert/fatalError in
debug builds to fail fast; update the symbol baseURL in EndPoint (or the
EndPoint initializer) to use that lookup so SwiftLint TODOs are resolved and
production builds never ship an empty URL.
- Around line 36-59: The requestURL computed property has two issues: the
queryParameters.map closure incorrectly references $1 (use the dictionary
tuple's .key/.value or destructure to (key, value) when creating URLQueryItem),
and empty-string baseURL plus force-unwrapping URL(string: "")! causes unsafe
crashes; replace the literal baseURL with a configurable source (e.g.,
Environment/baseURL or BuildConfig) and if URLComponents initialization or url
creation fails log the error via CherrishLogger.error(CherrishError.URLError)
and return a safe fallback URL (for example URL(string: "about:blank")!) or
change the API to return an optional URL so callers can handle failures instead
of force-unwrapping.
- Around line 41-55: EndPoint.swift currently references CherrishLogger and
CherrishError without imports which works only while they share the same target;
add explicit imports (e.g., import CherrishLogger and import CherrishError) at
the top of EndPoint.swift or move those types into a shared utilities module and
import that instead so future module separation will compile cleanly; ensure
references to CherrishLogger and CherrishError remain unchanged.
- Around line 61-76: The HeaderType.withAuth(userID:) exposes a
client-controlled X-User-Id header which can be spoofed; update the
implementation so the client does not assert identity solely via X-User-Id: stop
sending user ID as an authentication mechanism and instead include a
server-verified credential (e.g., add an Authorization: Bearer <token> header
via HeaderType.withAuth(token:) or require a secure token provider), remove or
document X-User-Id usage for routing/debug only, and add a TODO/comment in
HeaderType and any call sites to validate the server API contract (ensure server
uses token-based auth and does not trust X-User-Id alone).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
Cherrish-iOS/Cherrish-iOS/Data/Network/EndPoint/EndPoint.swift
🧰 Additional context used
🪛 SwiftLint (0.57.0)
Cherrish-iOS/Cherrish-iOS/Data/Network/EndPoint/EndPoint.swift
[Warning] 38-38: TODOs should be resolved (서버 배포 후 수정)
(todo)
Comment on lines
+8
to
+11
| import Foundation | ||
|
|
||
| import Alamofire | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Alamofire 타입 의존(HTTPMethod/HTTPHeaders/Parameters/ParameterEncoding) 범위가 의도한 수준인지 확인 권장
EndPoint 프로토콜이 Alamofire 타입을 그대로 노출하면(특히 Parameters, HTTPHeaders) 네트워크 구현 교체가 어려워집니다. “추상화 계층” 목표에 맞게 의도된 트레이드오프인지 확인해 주세요.
🤖 Prompt for AI Agents
In @Cherrish-iOS/Cherrish-iOS/Data/Network/EndPoint/EndPoint.swift around lines
8 - 11, EndPoint currently exposes Alamofire types (HTTPMethod, HTTPHeaders,
Parameters, ParameterEncoding) which couples your API to Alamofire; replace
those direct references with your own lightweight abstractions (e.g.,
EndpointHTTPMethod enum, Headers/Parameters typealias or wrapper structs, and a
ParameterEncoding protocol) in the EndPoint protocol, then implement an
adapter/mapper inside your networking layer that converts these abstractions to
Alamofire.HTTPMethod, HTTPHeaders, Parameters and ParameterEncoding when
performing requests; update implementations of EndPoint-conforming types to use
the new abstractions so the networking implementation can be swapped without
changing the protocol.
Comment on lines
+12
to
+34
| protocol EndPoint { | ||
|
|
||
| /// ex) /api/v1/challenge/start?mission={mission} | ||
| /// | ||
| /// basePath: /api/v1/challenge | ||
| /// path: /start | ||
| /// method: get | ||
| /// headers: withAuth(userID: Int) | ||
| /// parameterEncoding: URLEncoding, JSONEncoding | ||
| /// queryParameters: [mission: mission[ | ||
| /// bodyParameteres: nil | ||
| /// | ||
|
|
||
| var basePath: String { get } | ||
| var path: String { get } | ||
| var method: HTTPMethod { get } | ||
| var headers: HeaderType { get } | ||
| var parameterEncoding: ParameterEncoding { get } | ||
| var queryParameters: [String: String]? { get } | ||
| var bodyParameters: Parameters? { get } | ||
|
|
||
| var requestURL: URL { get } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
EndPoint.requestURL가 실패를 표현할 수 없는 설계라, 런타임 크래시/숨은 실패로 이어질 수 있습니다.
var requestURL: URL 고정이면 URL 생성 실패 시 안전하게 처리하기 어렵습니다(현재는 빈 URL 강제 언래핑). URL? 또는 throws(예: func makeURL() throws -> URL)로 바꿔 실패를 명시적으로 전파하는 쪽을 권장합니다.
🤖 Prompt for AI Agents
In @Cherrish-iOS/Cherrish-iOS/Data/Network/EndPoint/EndPoint.swift around lines
12 - 34, The EndPoint.requestURL should be changed to a failure-aware API:
replace the var requestURL: URL with a throwing factory like func makeURL()
throws -> URL (or at minimum var requestURL: URL?), then update every conforming
type to implement makeURL() using URLComponents/URL(string:) and throw a
descriptive error on failure, and update all callers to handle the thrown error
(try/try? or propagate) so URL construction failures are explicit instead of
causing forced-unwrap crashes; refer to EndPoint, requestURL, and any concrete
endpoint types for edits.
Comment on lines
+36
to
+59
| extension EndPoint { | ||
| var requestURL: URL { | ||
| let baseURL = "" // TODO: 서버 배포 후 수정 | ||
| let urlString = baseURL + basePath + path | ||
|
|
||
| guard var urlComponents = URLComponents(string: urlString) else { | ||
| CherrishLogger.error(CherrishError.URLError) | ||
| return URL(string: "")! | ||
| } | ||
|
|
||
| if let queryParameters { | ||
| urlComponents.queryItems = queryParameters.map { | ||
| URLQueryItem(name: $0, value: $1) | ||
| } | ||
| } | ||
|
|
||
| guard let url = urlComponents.url else { | ||
| CherrishLogger.error(CherrishError.URLError) | ||
| return URL(string: "")! | ||
| } | ||
|
|
||
| return url | ||
| } | ||
| } |
There was a problem hiding this comment.
컴파일 에러 + 실패 시 강제 크래시(URL(string: "")!) 처리 필요
- Line 47-49:
Dictionary.map클로저는 단일 튜플 파라미터라$1을 쓸 수 없어 컴파일이 깨집니다. - Line 38-55:
baseURL == ""+ 실패 시URL(string: "")!로 강제 크래시가 납니다(또는 잘못된 URL이 조용히 전파).
Proposed diff (핵심 컴파일/크래시 수정)
extension EndPoint {
var requestURL: URL {
- let baseURL = "" // TODO: 서버 배포 후 수정
- let urlString = baseURL + basePath + path
+ let baseURL = "" // TODO: 서버 배포 후 수정
+ let urlString = baseURL + basePath + path
guard var urlComponents = URLComponents(string: urlString) else {
CherrishLogger.error(CherrishError.URLError)
- return URL(string: "")!
+ preconditionFailure("Invalid URLComponents: \(urlString)")
}
if let queryParameters {
urlComponents.queryItems = queryParameters.map {
- URLQueryItem(name: $0, value: $1)
+ URLQueryItem(name: $0.key, value: $0.value)
}
}
guard let url = urlComponents.url else {
CherrishLogger.error(CherrishError.URLError)
- return URL(string: "")!
+ preconditionFailure("Invalid URL: \(urlComponents)")
}
return url
}
}추가로, baseURL은 문자열 상수 대신 빌드 설정/환경별 구성(Dev/Prod)로 분리하는 게 안전합니다.
🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 38-38: TODOs should be resolved (서버 배포 후 수정)
(todo)
🤖 Prompt for AI Agents
In @Cherrish-iOS/Cherrish-iOS/Data/Network/EndPoint/EndPoint.swift around lines
36 - 59, The requestURL computed property has two issues: the
queryParameters.map closure incorrectly references $1 (use the dictionary
tuple's .key/.value or destructure to (key, value) when creating URLQueryItem),
and empty-string baseURL plus force-unwrapping URL(string: "")! causes unsafe
crashes; replace the literal baseURL with a configurable source (e.g.,
Environment/baseURL or BuildConfig) and if URLComponents initialization or url
creation fails log the error via CherrishLogger.error(CherrishError.URLError)
and return a safe fallback URL (for example URL(string: "about:blank")!) or
change the API to return an optional URL so callers can handle failures instead
of force-unwrapping.
|
|
||
| extension EndPoint { | ||
| var requestURL: URL { | ||
| let baseURL = "" // TODO: 서버 배포 후 수정 |
There was a problem hiding this comment.
TODO 방치 시 배포/테스트 환경에서 전 구간 네트워크 URL이 깨질 수 있음
Line 38의 baseURL = ""는 최소한 빌드 구성 값으로 치환되거나, 디폴트가 유효한 URL이 되도록 막아야 합니다. (SwiftLint todo 경고도 동일 이슈)
🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 38-38: TODOs should be resolved (서버 배포 후 수정)
(todo)
🤖 Prompt for AI Agents
In @Cherrish-iOS/Cherrish-iOS/Data/Network/EndPoint/EndPoint.swift at line 38,
EndPoint.swift currently sets let baseURL = "" which will break networking if
left empty; replace this with a build-config-backed value and a safe default:
read the base URL from Info.plist or an xcconfig-defined key (e.g.,
Bundle.main.object(forInfoDictionaryKey: "BASE_URL") as? String) and fall back
to a valid default like "https://api.example.com", or assert/fatalError in debug
builds to fail fast; update the symbol baseURL in EndPoint (or the EndPoint
initializer) to use that lookup so SwiftLint TODOs are resolved and production
builds never ship an empty URL.
Comment on lines
+41
to
+55
| guard var urlComponents = URLComponents(string: urlString) else { | ||
| CherrishLogger.error(CherrishError.URLError) | ||
| return URL(string: "")! | ||
| } | ||
|
|
||
| if let queryParameters { | ||
| urlComponents.queryItems = queryParameters.map { | ||
| URLQueryItem(name: $0, value: $1) | ||
| } | ||
| } | ||
|
|
||
| guard let url = urlComponents.url else { | ||
| CherrishLogger.error(CherrishError.URLError) | ||
| return URL(string: "")! | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find definitions of CherrishLogger and CherrishError
echo "=== Searching for CherrishLogger and CherrishError definitions ==="
rg -n '(struct|enum|class)\s+(CherrishLogger|CherrishError)\b' -S
echo ""
echo "=== Checking references in EndPoint.swift ==="
rg -n 'CherrishLogger\.|CherrishError\.' Cherrish-iOS/Cherrish-iOS/Data/Network/EndPoint/EndPoint.swift
echo ""
echo "=== Checking imports in EndPoint.swift ==="
head -20 Cherrish-iOS/Cherrish-iOS/Data/Network/EndPoint/EndPoint.swiftRepository: TEAM-Cherrish/Cherrish-iOS
Length of output: 913
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Checking CherrishLogger access level ==="
sed -n '71,75p' Cherrish-iOS/Cherrish-iOS/Core/CherrishLogger.swift
echo ""
echo "=== Checking CherrishError access level ==="
sed -n '10,15p' Cherrish-iOS/Cherrish-iOS/Data/Network/CherrishError.swift
echo ""
echo "=== Checking for Package.swift (SPM indication) ==="
find . -name "Package.swift" -type f 2>/dev/null | head -5
echo ""
echo "=== Checking Xcode project structure ==="
find . -name "project.pbxproj" -type f 2>/dev/null | head -2
echo ""
echo "=== Checking for multiple targets in pbxproj ==="
if [ -f "Cherrish-iOS.xcodeproj/project.pbxproj" ]; then
rg 'targets = \(' Cherrish-iOS.xcodeproj/project.pbxproj -A 5 | head -20
fiRepository: TEAM-Cherrish/Cherrish-iOS
Length of output: 650
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Checking if EndPoint is public or part of a framework ==="
rg -n 'public\s+(protocol|struct|class|enum)\s+EndPoint' Cherrish-iOS/Cherrish-iOS/Data/Network/EndPoint/
echo ""
echo "=== Checking full access level declarations ==="
head -15 Cherrish-iOS/Cherrish-iOS/Core/CherrishLogger.swift | tail -5
head -15 Cherrish-iOS/Cherrish-iOS/Data/Network/CherrishError.swift | tail -5
echo ""
echo "=== Checking EndPoint protocol declaration ==="
grep -n "protocol EndPoint" Cherrish-iOS/Cherrish-iOS/Data/Network/EndPoint/EndPoint.swift -A 20 | head -25
echo ""
echo "=== Verifying module structure - checking for multiple targets ==="
rg -U 'targets\s*=' Cherrish-iOS.xcodeproj/project.pbxproj | head -3Repository: TEAM-Cherrish/Cherrish-iOS
Length of output: 131
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Reading full CherrishLogger declaration (first 30 lines) ==="
head -30 Cherrish-iOS/Cherrish-iOS/Core/CherrishLogger.swift | cat -n
echo ""
echo "=== Reading full CherrishError declaration (first 20 lines) ==="
head -20 Cherrish-iOS/Cherrish-iOS/Data/Network/CherrishError.swift | cat -n
echo ""
echo "=== Checking EndPoint.swift complete imports section ==="
sed -n '1,15p' Cherrish-iOS/Cherrish-iOS/Data/Network/EndPoint/EndPoint.swift | cat -n
echo ""
echo "=== Verifying if files are in same target/module ==="
find Cherrish-iOS/Cherrish-iOS -name "*.swift" -type f | wc -l
echo "Total Swift files in main target"Repository: TEAM-Cherrish/Cherrish-iOS
Length of output: 2129
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Checking for bridging header ==="
find Cherrish-iOS -name "*Bridging*" -o -name "*bridging*" 2>/dev/null
echo ""
echo "=== Checking if there's a common imports file ==="
rg -l "CherrishLogger|CherrishError" Cherrish-iOS/Cherrish-iOS --type swift | head -10
echo ""
echo "=== Checking what files import CherrishLogger/CherrishError ==="
rg "import.*CherrishLogger|import.*CherrishError" Cherrish-iOS/Cherrish-iOS --type swift
echo ""
echo "=== Checking if these types are used elsewhere without imports ==="
rg "CherrishLogger\.|CherrishError\." Cherrish-iOS/Cherrish-iOS --type swift -c | sort | uniq -c | sort -rnRepository: TEAM-Cherrish/Cherrish-iOS
Length of output: 523
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Checking all usages of CherrishLogger and CherrishError without imports ==="
rg "CherrishLogger\.|CherrishError\." Cherrish-iOS/Cherrish-iOS --type swift | head -20
echo ""
echo "=== Checking if module separation/frameworks are planned (looking for Package.swift, dynamic frameworks, etc.) ==="
find Cherrish-iOS -maxdepth 3 -name "*.xcconfig" -o -name "Podfile" -o -name "*.podspec" 2>/dev/null
echo ""
echo "=== Checking file structure for modular organization ==="
ls -la Cherrish-iOS/Cherrish-iOS/Core/
echo "---"
ls -la Cherrish-iOS/Cherrish-iOS/Data/Network/
echo ""
echo "=== Verifying current access levels are all internal/default ==="
rg "^(public|open|private|fileprivate)\s+(struct|enum|class|protocol)" Cherrish-iOS/Cherrish-iOS/Core/CherrishLogger.swift
rg "^(public|open|private|fileprivate)\s+(struct|enum|class|protocol)" Cherrish-iOS/Cherrish-iOS/Data/Network/CherrishError.swiftRepository: TEAM-Cherrish/Cherrish-iOS
Length of output: 3224
현재 모듈 내에서는 작동하지만, 모듈 분리 시 명시적 import 추가 필요
CherrishLogger(Core)와 CherrishError(Data/Network)는 현재 같은 Xcode 타겟의 내부(internal) 접근 수준이므로, EndPoint.swift에서 import 없이 참조해도 빌드가 성공합니다. 다만 향후 모듈 분리나 프레임워크 분리가 발생할 경우 명시적 import를 추가해야 합니다. 현재 단계에서 import CherrishLogger, import CherrishError 같은 import 문을 추가하거나, 이들을 별도 유틸리티 모듈로 분리하는 것을 고려하면 향후 리팩토링 시 번거로움을 줄일 수 있습니다.
🤖 Prompt for AI Agents
In @Cherrish-iOS/Cherrish-iOS/Data/Network/EndPoint/EndPoint.swift around lines
41 - 55, EndPoint.swift currently references CherrishLogger and CherrishError
without imports which works only while they share the same target; add explicit
imports (e.g., import CherrishLogger and import CherrishError) at the top of
EndPoint.swift or move those types into a shared utilities module and import
that instead so future module separation will compile cleanly; ensure references
to CherrishLogger and CherrishError remain unchanged.
Comment on lines
+61
to
+76
| enum HeaderType { | ||
| case basic | ||
| case withAuth(userID: Int) | ||
|
|
||
| var value: HTTPHeaders { | ||
| switch self { | ||
| case .basic: | ||
| return ["Content-Type" : "application/json"] | ||
| case .withAuth(let userID): | ||
| return [ | ||
| "Content-Type": "application/json", | ||
| "X-User-Id": "\(userID)" | ||
| ] | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find all files in the project structure
git ls-files | head -20Repository: TEAM-Cherrish/Cherrish-iOS
Length of output: 1291
🏁 Script executed:
# Search for HeaderType usage
rg "HeaderType" --type swiftRepository: TEAM-Cherrish/Cherrish-iOS
Length of output: 249
🏁 Script executed:
# Search for X-User-Id header usage
rg "X-User-Id" --type swiftRepository: TEAM-Cherrish/Cherrish-iOS
Length of output: 173
🏁 Script executed:
# Search for Bearer token or other auth mechanisms
rg -i "bearer|authentication|authorization" --type swift | head -30Repository: TEAM-Cherrish/Cherrish-iOS
Length of output: 52
🏁 Script executed:
# Look for where withAuth is being called
rg "withAuth" --type swiftRepository: TEAM-Cherrish/Cherrish-iOS
Length of output: 364
🏁 Script executed:
# Read the EndPoint.swift file to understand context
cat -n Cherrish-iOS/Cherrish-iOS/Data/Network/EndPoint/EndPoint.swiftRepository: TEAM-Cherrish/Cherrish-iOS
Length of output: 2457
🏁 Script executed:
# Search for where headers property is used in network calls
rg "\.headers" --type swift -B 2 -A 2Repository: TEAM-Cherrish/Cherrish-iOS
Length of output: 2273
🏁 Script executed:
# Look for network client/request implementation
fd ".*Network.*" --type f --extension swift | head -20Repository: TEAM-Cherrish/Cherrish-iOS
Length of output: 129
🏁 Script executed:
# Search for any token or credential storage
rg -i "token|credential|bearer" --type swiftRepository: TEAM-Cherrish/Cherrish-iOS
Length of output: 52
🏁 Script executed:
# Search for where withAuth is being instantiated/called
rg "withAuth\(" --type swift -B 3 -A 3Repository: TEAM-Cherrish/Cherrish-iOS
Length of output: 1923
🏁 Script executed:
# Look for API endpoint implementations to see which ones use withAuth
fd ".*EndPoint.*" --type f --extension swift | grep -v "EndPoint.swift"Repository: TEAM-Cherrish/Cherrish-iOS
Length of output: 52
🏁 Script executed:
# Check NetworkService.swift to understand the request flow
cat -n Cherrish-iOS/Cherrish-iOS/Data/Network/NetworkService.swiftRepository: TEAM-Cherrish/Cherrish-iOS
Length of output: 5428
🏁 Script executed:
# Search for where userID might come from (stored credentials, user model, etc.)
rg "userID|user.*[Ii]d" --type swift | grep -i "let\|var\|get\|stored\|user" | head -30Repository: TEAM-Cherrish/Cherrish-iOS
Length of output: 468
🏁 Script executed:
# Find all concrete EndPoint implementations
fd ".*EndPoint.*" --type f --extension swiftRepository: TEAM-Cherrish/Cherrish-iOS
Length of output: 132
🏁 Script executed:
# Search for actual endpoint case implementations
rg "case.*:" --type swift -A 5 | grep -A 5 "EndPoint"Repository: TEAM-Cherrish/Cherrish-iOS
Length of output: 1514
🏁 Script executed:
# Search for all EndPoint protocol implementations
rg "protocol EndPoint\|extension.*EndPoint\|struct.*EndPoint\|enum.*EndPoint" --type swift -A 10Repository: TEAM-Cherrish/Cherrish-iOS
Length of output: 52
🏁 Script executed:
# Look for Challenge or any specific endpoint enums
rg "enum.*:" --type swift -B 2 -A 20 | grep -A 20 "Challenge\|API\|Request"Repository: TEAM-Cherrish/Cherrish-iOS
Length of output: 1478
🏁 Script executed:
# Check if there are any repository/data layer files that might show endpoint usage
find Cherrish-iOS -type f -name "*.swift" -path "*/Repository/*" -o -path "*/DataSource/*" | head -10Repository: TEAM-Cherrish/Cherrish-iOS
Length of output: 132
🏁 Script executed:
# List all files in the EndPoint directory
ls -la Cherrish-iOS/Cherrish-iOS/Data/Network/EndPoint/Repository: TEAM-Cherrish/Cherrish-iOS
Length of output: 307
🏁 Script executed:
# Search for files that might define concrete endpoints
find Cherrish-iOS -type f -name "*.swift" | xargs grep -l "headers.*HeaderType\|headers.*=.*withAuth\|headers.*=.*basic"Repository: TEAM-Cherrish/Cherrish-iOS
Length of output: 132
🏁 Script executed:
# Look for enum definitions that conform to EndPoint
rg "enum.*:\s*EndPoint\|struct.*:\s*EndPoint" --type swift -A 30Repository: TEAM-Cherrish/Cherrish-iOS
Length of output: 52
🏁 Script executed:
# Read TestAPI.swift to see how endpoints are actually implemented
cat -n Cherrish-iOS/Cherrish-iOS/Data/Network/EndPoint/TestAPI.swiftRepository: TEAM-Cherrish/Cherrish-iOS
Length of output: 381
🏁 Script executed:
# Search for any other files that use HeaderType
rg "headers\s*=" --type swift -B 2 -A 2Repository: TEAM-Cherrish/Cherrish-iOS
Length of output: 52
🏁 Script executed:
# Check if there's any authentication or session management in the app
find Cherrish-iOS -type f -name "*.swift" | xargs grep -l "Session\|Auth\|Credential\|Token" | head -15Repository: TEAM-Cherrish/Cherrish-iOS
Length of output: 132
🏁 Script executed:
# Check repository structure to understand project stage
ls -la Cherrish-iOS/Cherrish-iOS/Data/Repository/Repository: TEAM-Cherrish/Cherrish-iOS
Length of output: 247
🏁 Script executed:
# See if there's any documentation or API spec
find Cherrish-iOS -type f \( -name "README*" -o -name "*.md" -o -name "*API*" \) | head -10Repository: TEAM-Cherrish/Cherrish-iOS
Length of output: 131
🏁 Script executed:
# Check git history to see if this is a new project
git log --oneline -10Repository: TEAM-Cherrish/Cherrish-iOS
Length of output: 93
withAuth(userID:) 인증 헤더의 스푸핑/권한오남용 리스크 확인 필요
클라이언트가 임의의 X-User-Id 값을 설정할 수 있으며, 현재 코드에서는 이를 검증하는 로직이 없습니다. 만약 서버에서 X-User-Id 헤더만으로 인증/인가를 처리한다면 취약점이 될 수 있습니다.
현재 프로젝트는 초기 단계이므로, 서버 API 계약을 명확히 정의할 때 다음을 확인하세요:
- 서버가 토큰 기반 인증(Bearer token 등)을 함께 사용하는지 여부
X-User-Id가 실제 인증 판단에 사용되는지, 아니면 라우팅/디버그 목적인지
🤖 Prompt for AI Agents
In @Cherrish-iOS/Cherrish-iOS/Data/Network/EndPoint/EndPoint.swift around lines
61 - 76, The HeaderType.withAuth(userID:) exposes a client-controlled X-User-Id
header which can be spoofed; update the implementation so the client does not
assert identity solely via X-User-Id: stop sending user ID as an authentication
mechanism and instead include a server-verified credential (e.g., add an
Authorization: Bearer <token> header via HeaderType.withAuth(token:) or require
a secure token provider), remove or document X-User-Id usage for routing/debug
only, and add a TODO/comment in HeaderType and any call sites to validate the
server API contract (ensure server uses token-based auth and does not trust
X-User-Id alone).
This was referenced Jan 15, 2026
Merged
Merged
Merged
This was referenced Jan 20, 2026
Merged
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
4 participants
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.
🔗 연결된 이슈
📄 작업 내용