Repository navigation
feat: Kakao 키워드 장소 검색 구현 - #33
Conversation
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthrough
ChangesKakao 지오코더 통합
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant PlaceController
participant GeocoderService
participant KakaoProvider
participant KakaoAPI
PlaceController->>GeocoderService: searchAll(placeName, areaName, areaType)
GeocoderService->>KakaoProvider: search(query)
KakaoProvider->>KakaoAPI: fetch(keyword search URL, Authorization)
KakaoAPI-->>KakaoProvider: response(status, json)
alt 성공
KakaoProvider-->>GeocoderService: GeoCandidate[]
else 실패/에러
KakaoProvider-->>GeocoderService: AppException
GeocoderService->>GeocoderService: 경고 로그 기록
end
GeocoderService-->>PlaceController: GeoCandidate[]
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
labyrinth30
left a comment
labyrinth30
left a comment
There was a problem hiding this comment.
Kakao 키워드 검색 연동의 요청 타임아웃, 응답 스키마 검증, 예외 매핑과 테스트를 확인했습니다.
다만 GET /api/v1/place/_test/geocode가 인증이나 환경 제한 없이 공개되어 있어, 외부에서 Kakao API 호출 비용과 rate limit을 소모할 수 있습니다. 임시 엔드포인트를 제거하거나 비운영 환경에서만 등록되도록 제한 부탁드립니다.
| private getApiKey(): string { | ||
| const apiKey = this.configService.get("KAKAO_REST_API_KEY", { | ||
| infer: true, | ||
| }); | ||
|
|
||
| if (!apiKey?.trim()) { | ||
| throw new AppException( | ||
| "KAKAO_REST_API_KEY_MISSING", | ||
| "카카오 REST API 키가 설정되지 않았습니다.", | ||
| HttpStatus.INTERNAL_SERVER_ERROR, | ||
| ); | ||
| } | ||
|
|
||
| return apiKey; | ||
| } |
There was a problem hiding this comment.
이 부분은 env var validation 쪽에서 required + min length 1 설정해서 방지해보는 건 어떨까요 👀
| const url = this.createKeywordSearchUrl(query); | ||
| let response: Response; |
There was a problem hiding this comment.
💬
사소한 질문이 있는데요. 저는 뭔가... response, url 과 같은 변수명이 명확하지않아서 좀 지양하려 하거든요..!
물론, 지금 이 service logic에서는 이해가 안될 부분이 전혀 없지만.. 어떻게 생각하시는지 궁금해요!
There was a problem hiding this comment.
response, url 과 같은 변수명이 명확하지않아서 좀 지양하려 하거든요..!
제 생각은..! 예전에는 저도 싫어서 많이 지양하려고 했던 것 같은데요
어느순간 마음의 불편함이 사라지기 시작한 것 같아요!
- ai를 사용하면서 더 굳이굳이인 부분이 되기도 했고, ai가 해당 맥락을 명시하지 않는 경우가 꽤나 많은데 크게 중요하지 않은 부분에 일일이 수정하라고 하는 비용이 더 든다고 느꼈어요!
search(searchUrl: string)vssearch(url: string)을 보면, 저의 경우는 전자는해당 함수에 다른 url이 또 있나?라는 생각이 들고, 후자는우선 저 url은 search url이겠구나, 그리고 다른 url이 없나?라는 생각이 들어요
결국, 가독성이 나아진다고 느껴지지 않더라구요
(한 문맥에 여러 컨텍스트가 있는 경우(e.g. response 여러개, url 여러개) 가 아닌 하나의 컨텍스트만 있거나, 해당 문맥이 짧은 경우)
| if (response.status === HttpStatus.TOO_MANY_REQUESTS) { | ||
| throw new AppException( | ||
| "KAKAO_RATE_LIMITED", | ||
| "카카오 장소 검색 요청 한도를 초과했습니다.", | ||
| HttpStatus.TOO_MANY_REQUESTS, | ||
| ); | ||
| } |
There was a problem hiding this comment.
💬
요청 한도가 초과되기 전에 파악하는 방법이 있을까요?
만약 요청 한도가 초과되어 이게 에러가 나기 시작한다면, 감당 안되질 거 같아서 대안이 있으면 어떨까 했습니다.
- 요청 한도를 조회하다가 한도 시점이 오면 discord에 알림
- 카카오에 카드를 등록하여 한도를 초과하더라도 자동 결제
- fallback 대응
There was a problem hiding this comment.
요청 한도가 초과되기 전에 파악하는 방법이 있을까요?
찾아보니 응답에도 없고 사용량 조회 API도 없네요!
There was a problem hiding this comment.
제안주신것들 좋습니다! 요거는 회의때 한번 논의해보면 좋을것 같은데요 그 후에 작업 분리해서 가시죠ㅎㅎ
AI야. Summary 잘 읽어라. @labyrinth30 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
src/infrastructures/geocoder/providers/kakao.provider.ts (1)
47-61: 🩺 Stability & Availability | 🔵 TrivialRate limit 대응 전략 미해결(과거 리뷰 재확인).
이전 리뷰에서 요청 한도 초과 전 대응 방안(한도 모니터링/알림, 결제 등록, fallback)에 대한 논의가 있었으나, 현재 코드는 429 응답을 예외로 던지는 것 외에 사전 대응 로직이 없습니다. 운영 관점에서 Kakao API 쿼터 소진 시 전체 검색 기능이 즉시 저하될 수 있으므로, 최소한 요청 실패율/429 발생을 관찰할 수 있는 모니터링(알림)을 고려해 주세요.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/infrastructures/geocoder/providers/kakao.provider.ts` around lines 47 - 61, The Kakao geocoder currently only throws on 429 or other failures in kakao.provider.ts, but it does not surface rate-limit exhaustion for operations monitoring. Update the Kakao API request handling in the provider to emit observable telemetry when HttpStatus.TOO_MANY_REQUESTS or other request failures occur, using the existing request/exception path in Kakao provider methods to increment failure-rate or 429 metrics and trigger an alert-friendly log. Keep the AppException behavior, but add monitoring hooks around the response handling so operators can detect quota exhaustion before search failures spread.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/infrastructures/geocoder/geocoder.service.ts`:
- Around line 14-31: `GeocoderService.searchAll`는 `Promise.allSettled`에서
rejected된 provider의 실패 원인을 버리고 있으므로, `settled`를 처리할 때 `rejected` 항목의 `reason`을
`NestJS Logger`로 warn 이상에 기록하도록 추가하세요. `searchAll` 내부에서 `fulfilled`만 모으기 전에 각 실패
provider와 에러 사유를 남기고, 전체 실패 시에도 `AppException`을 던지기 전에 어떤 provider들이 왜 실패했는지 추적
가능하게 유지하세요.
In `@src/infrastructures/geocoder/providers/kakao.provider.spec.ts`:
- Around line 34-53: The Kakao provider spec is missing coverage for the `catch`
path in `kakao.provider.ts`, so add a test that replaces `globalThis.fetch` with
a rejected mock to simulate network failure or timeout/abort and asserts
`KAKAO_REQUEST_FAILED` is thrown. Reuse the existing `createKakaoResponse` setup
and place the new case alongside the current response-error tests so the
`fetchAddress`/provider error handling is verified for both HTTP failures and
fetch rejection paths.
In `@src/infrastructures/geocoder/providers/kakao.provider.ts`:
- Around line 55-61: The Kakao provider’s non-ok handling in the geocoder
request path currently maps every failure to the same
KAKAO_REQUEST_FAILED/BAD_GATEWAY exception. Update the response handling in
kakao.provider.ts to branch on response status in the provider logic (the code
around the response.ok check) and use distinct AppException codes/statuses for
authentication failures like 401/403, bad requests like 400, and
upstream/service failures like 5xx. Keep the existing KAKAO_REQUEST_FAILED path
only for true external outage cases, and make the new error codes/statuses easy
to locate through the Kakao provider’s request method.
In `@src/modules/place/place.dto.ts`:
- Around line 11-20: `testGeocodeRequestSchema`의 `areaType` 값이 `AreaType`과 중복
하드코딩되어 있습니다. `place.dto.ts`의 `testGeocodeRequestSchema`에서
`v.picklist(["landmark", "address", "region"])`를 직접 쓰지 말고, `geocoder.type.ts`의
`AreaType`을 단일 소스로 두도록 공용 상수(예: `AREA_TYPES`)를 export한 뒤
`v.picklist(AREA_TYPES)`로 연결하세요. 필요하면 `AreaType`도 해당 상수에서 파생되게 바꿔 스키마와 타입이 항상 함께
유지되도록 정리하세요.
---
Duplicate comments:
In `@src/infrastructures/geocoder/providers/kakao.provider.ts`:
- Around line 47-61: The Kakao geocoder currently only throws on 429 or other
failures in kakao.provider.ts, but it does not surface rate-limit exhaustion for
operations monitoring. Update the Kakao API request handling in the provider to
emit observable telemetry when HttpStatus.TOO_MANY_REQUESTS or other request
failures occur, using the existing request/exception path in Kakao provider
methods to increment failure-rate or 429 metrics and trigger an alert-friendly
log. Keep the AppException behavior, but add monitoring hooks around the
response handling so operators can detect quota exhaustion before search
failures spread.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 272a5aa1-a950-4e7e-abdc-02b155ca8308
📒 Files selected for processing (11)
src/config/env.schema.tssrc/infrastructures/geocoder/geocoder.service.spec.tssrc/infrastructures/geocoder/geocoder.service.tssrc/infrastructures/geocoder/geocoder.type.tssrc/infrastructures/geocoder/providers/kakao.provider.spec.tssrc/infrastructures/geocoder/providers/kakao.provider.tssrc/infrastructures/geocoder/providers/kakao.type.tssrc/modules/place/place.controller.tssrc/modules/place/place.dto.tssrc/modules/place/place.service.spec.tssrc/modules/place/place.service.ts
충돌 해결: - env.schema.ts: KAKAO_REST_API_KEY(required) + main의 INSTAGRAM_* 유니온 - geocoder.type.ts: AREA_TYPES 단일 소스 유지 (main 필드 포함 상위집합) - geocoder.service.ts: provider 실패 로깅 유지 (main 구현 상위집합) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
KAKAO_REST_API_KEY가 env 검증에서 required가 되면서 e2e 부팅 시 validateEnv가 실패하던 문제 해결. INSTAGRAM_* 주입과 동일 패턴. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/infrastructures/geocoder/geocoder.service.ts`:
- Around line 20-26: The provider failure warn log in geocoder.service should
preserve full error details when the rejection reason is an Error instance.
Update the logging in the settled.forEach block to detect result.reason in
geocoder.service and, for Error objects, log the stack (or the Error object
itself) instead of interpolating only the stringified message; keep the existing
provider name context in the warn message.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4a7f6634-ab47-490d-a166-767b530b00a6
📒 Files selected for processing (6)
package.jsonsrc/config/env.schema.tssrc/infrastructures/geocoder/geocoder.service.tssrc/infrastructures/geocoder/geocoder.type.tssrc/infrastructures/geocoder/providers/kakao.provider.spec.tssrc/modules/place/place.dto.ts
| settled.forEach((result, index) => { | ||
| if (result.status === "rejected") { | ||
| this.logger.warn( | ||
| `Geocoder provider "${this.providers[index].name}" failed: ${result.reason}`, | ||
| ); | ||
| } | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
과거 리뷰 요청사항 정확히 반영됨.
Provider별 실패 사유를 warn 레벨로 로깅하는 로직이 이전 리뷰 코멘트에서 요청한 대로 정확히 구현되었습니다.
다만 result.reason이 Error 인스턴스인 경우 템플릿 문자열 삽입 시 toString() 결과(예: Error: message)만 남고 stack trace는 사라집니다. 운영 중 디버깅 편의를 위해 Error 인스턴스 여부를 확인해 stack을 함께 로깅하는 것을 고려해보세요.
♻️ 제안
settled.forEach((result, index) => {
if (result.status === "rejected") {
+ const reason =
+ result.reason instanceof Error
+ ? result.reason.stack
+ : result.reason;
this.logger.warn(
- `Geocoder provider "${this.providers[index].name}" failed: ${result.reason}`,
+ `Geocoder provider "${this.providers[index].name}" failed: ${reason}`,
);
}
});📝 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.
| settled.forEach((result, index) => { | |
| if (result.status === "rejected") { | |
| this.logger.warn( | |
| `Geocoder provider "${this.providers[index].name}" failed: ${result.reason}`, | |
| ); | |
| } | |
| }); | |
| settled.forEach((result, index) => { | |
| if (result.status === "rejected") { | |
| const reason = | |
| result.reason instanceof Error | |
| ? result.reason.stack | |
| : result.reason; | |
| this.logger.warn( | |
| `Geocoder provider "${this.providers[index].name}" failed: ${reason}`, | |
| ); | |
| } | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/infrastructures/geocoder/geocoder.service.ts` around lines 20 - 26, The
provider failure warn log in geocoder.service should preserve full error details
when the rejection reason is an Error instance. Update the logging in the
settled.forEach block to detect result.reason in geocoder.service and, for Error
objects, log the stack (or the Error object itself) instead of interpolating
only the stringified message; keep the existing provider name context in the
warn message.
…eyword-search # Conflicts: # package.json # src/config/env.schema.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/config/env.schema.spec.ts`:
- Around line 4-13: Add validation coverage for the new KAKAO_REST_API_KEY
requirement in env.schema.spec.ts by extending the validateEnv tests to assert
that a missing or empty KAKAO_REST_API_KEY causes an exception. Use the existing
requiredEnvironment fixture and the validateEnv helper in
src/config/env.schema.spec.ts to cover both the success path and the failure
path for the minLength(1) rule.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b82ecd04-9b11-41a1-8ca0-a6fd9a081a01
📒 Files selected for processing (3)
package.jsonsrc/config/env.schema.spec.tssrc/config/env.schema.ts
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/config/env.schema.spec.ts`:
- Around line 4-13: Add validation coverage for the new KAKAO_REST_API_KEY
requirement in env.schema.spec.ts by extending the validateEnv tests to assert
that a missing or empty KAKAO_REST_API_KEY causes an exception. Use the existing
requiredEnvironment fixture and the validateEnv helper in
src/config/env.schema.spec.ts to cover both the success path and the failure
path for the minLength(1) rule.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b82ecd04-9b11-41a1-8ca0-a6fd9a081a01
📒 Files selected for processing (3)
package.jsonsrc/config/env.schema.spec.tssrc/config/env.schema.ts
🛑 Comments failed to post (1)
src/config/env.schema.spec.ts (1)
4-13: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
KAKAO_REST_API_KEY 검증 케이스 추가를 권장합니다.
requiredEnvironment에KAKAO_REST_API_KEY: "test"가 포함되어 있어 기본 환경변수 계약은 잘 반영되었습니다. 하지만 이 PR의 핵심 변경사항인 KAKAO_REST_API_KEY 필수화(minLength(1))에 대한 검증 테스트가 없습니다. 누락 또는 빈 문자열 입력 시validateEnv가 예외를 던지는지 확인하는 케이스를 추가하면 좋겠습니다.🧪 제안하는 테스트 케이스
describe("Sentry environment", () => { // ... 기존 Sentry 테스트 ... }); + +describe("Kakao environment", () => { + it.each([ + ["누락", {}], + ["빈 문자열", { KAKAO_REST_API_KEY: "" }], + ])("KAKAO_REST_API_KEY %s을 거부한다", (_name, override) => { + const { KAKAO_REST_API_KEY: _, ...rest } = requiredEnvironment; + expect(() => + validateEnv({ ...rest, ...override }), + ).toThrow("Invalid environment variables"); + }); +});🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/config/env.schema.spec.ts` around lines 4 - 13, Add validation coverage for the new KAKAO_REST_API_KEY requirement in env.schema.spec.ts by extending the validateEnv tests to assert that a missing or empty KAKAO_REST_API_KEY causes an exception. Use the existing requiredEnvironment fixture and the validateEnv helper in src/config/env.schema.spec.ts to cover both the success path and the failure path for the minLength(1) rule.
4 participants
📌 Related Issue
GM-63
🚀 Description
KakaoProvider에 연결했습니다.GeoQuery를 Kakao keyword search 요청으로 변환하고, Kakao 응답을 서비스 내부 표준 형태인GeoCandidate로 정규화합니다.✅ Done
Authorization: KakaoAK {KAKAO_REST_API_KEY}인증 헤더 적용areaName + placeName기반 query 생성kakao.type.ts로 분리GeoCandidate로 정규화KakaoProvider단위 테스트 추가📢 Notes
_test/geocodeAPI는 KakaoProvider 실제 연동 확인을 위한 임시 엔드포인트이며, 전체 장소 저장/추출 플로우가 연결되면 제거할 예정입니다.KAKAO_REST_API_KEY는 SM에 등록해두었습니다.Summary by CodeRabbit
New Features
Bug Fixes
Tests