Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
"start:local": "APP_ENV=local nest start --watch --exec bun",
"start:prod": "./dist/server",
"test": "bun run test:unit && bun run test:e2e",
"test:e2e": "DATABASE_URL=${TEST_DATABASE_URL:-postgres://postgres:postgres@localhost:5432/team_mino} GOOGLE_CLOUD_PROJECT=team-mino-test INSTAGRAM_GRAPHQL_ENDPOINT=https://www.instagram.com/api/graphql INSTAGRAM_DOC_ID=test INSTAGRAM_LSD=test INSTAGRAM_APP_ID=test INSTAGRAM_USER_AGENT=test bun test e2e/",
"test:e2e": "DATABASE_URL=${TEST_DATABASE_URL:-postgres://postgres:postgres@localhost:5432/team_mino} KAKAO_REST_API_KEY=test GOOGLE_CLOUD_PROJECT=team-mino-test INSTAGRAM_GRAPHQL_ENDPOINT=https://www.instagram.com/api/graphql INSTAGRAM_DOC_ID=test INSTAGRAM_LSD=test INSTAGRAM_APP_ID=test INSTAGRAM_USER_AGENT=test bun test e2e/",
"test:unit": "bun test src/",
"test:watch": "bun test --watch",
"typecheck": "tsgo --noEmit",
Expand Down
1 change: 1 addition & 0 deletions src/config/env.schema.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const requiredEnvironment = {
INSTAGRAM_GRAPHQL_ENDPOINT: "https://www.instagram.com/api/graphql",
INSTAGRAM_LSD: "test",
INSTAGRAM_USER_AGENT: "test",
KAKAO_REST_API_KEY: "test",
};

describe("Sentry environment", () => {
Expand Down
2 changes: 1 addition & 1 deletion src/config/env.schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ const envSchema = v.object({
GOOGLE_CLOUD_PROJECT: v.pipe(v.string(), v.minLength(1)),
// Gemini 3.x는 global 전용이므로 기본 "global" 사용
GOOGLE_VERTEX_LOCATION: v.optional(v.string(), "global"),
KAKAO_REST_API_KEY: v.optional(v.string()),
KAKAO_REST_API_KEY: v.pipe(v.string(), v.minLength(1)),
SENTRY_DSN: v.optional(v.pipe(v.string(), v.url(), v.regex(/^https:\/\//))),
SENTRY_RELEASE: v.optional(v.pipe(v.string(), v.minLength(1))),
// Instagram 비공개 GraphQL 호출용 값들. 인스타가 토큰/구조를 바꾸면 env만 갱신하면 됨.
Expand Down
11 changes: 10 additions & 1 deletion src/infrastructures/geocoder/geocoder.service.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import { HttpStatus, Inject, Injectable } from "@nestjs/common";
import { HttpStatus, Inject, Injectable, Logger } from "@nestjs/common";
import { AppException } from "../../common/exceptions/app.exception";
import type { GeoCandidate, GeocoderProvider, GeoQuery } from "./geocoder.type";

export const GEOCODER_PROVIDERS = Symbol("GEOCODER_PROVIDERS");

@Injectable()
export class GeocoderService {
private readonly logger = new Logger(GeocoderService.name);

constructor(
@Inject(GEOCODER_PROVIDERS)
private readonly providers: GeocoderProvider[],
Expand All @@ -15,6 +17,13 @@ export class GeocoderService {
const settled = await Promise.allSettled(
this.providers.map((provider) => provider.search(query)),
);
settled.forEach((result, index) => {
if (result.status === "rejected") {
this.logger.warn(
`Geocoder provider "${this.providers[index].name}" failed: ${result.reason}`,
);
}
});
Comment on lines +20 to +26

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 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.

Suggested change
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.

const succeeded = settled.filter(
(result): result is PromiseFulfilledResult<GeoCandidate[]> =>
result.status === "fulfilled",
Expand Down
3 changes: 2 additions & 1 deletion src/infrastructures/geocoder/geocoder.type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ export interface Coordinate {
lng: number;
}

export type AreaType = "landmark" | "address" | "region";
export const AREA_TYPES = ["landmark", "address", "region"] as const;
export type AreaType = (typeof AREA_TYPES)[number];

export interface GeoQuery {
areaName: string;
Expand Down
169 changes: 169 additions & 0 deletions src/infrastructures/geocoder/providers/kakao.provider.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
import { afterEach, describe, expect, it, jest } from "bun:test";
import type { ConfigService } from "@nestjs/config";
import { AppException } from "../../../common/exceptions/app.exception";
import type { Env } from "../../../config/env.schema";
import type { GeoQuery } from "../geocoder.type";
import { KakaoProvider } from "./kakao.provider";

const originalFetch = globalThis.fetch;
const query: GeoQuery = {
areaName: "서울 강남구",
areaType: "landmark",
placeName: "카카오프렌즈",
};

function createProvider() {
const configService = {
getOrThrow: jest.fn(() => "test-api-key"),
} as unknown as ConfigService<Env>;

return new KakaoProvider(configService);
}

function createKakaoResponse(overrides: Record<string, unknown> = {}) {
return {
meta: {
same_name: {
region: [],
keyword: "카카오프렌즈",
selected_region: "",
},
pageable_count: 1,
total_count: 1,
is_end: true,
},
documents: [
{
id: "26338954",
place_name: "카카오프렌즈 코엑스점",
category_name: "가정,생활 > 문구,사무용품 > 디자인문구 > 카카오프렌즈",
category_group_code: "",
category_group_name: "",
phone: "02-6002-1880",
address_name: "서울 강남구 삼성동 159",
road_address_name: "서울 강남구 영동대로 513",
x: "127.05902969025047",
y: "37.51207412593136",
place_url: "http://place.map.kakao.com/26338954",
distance: "418",
...overrides,
},
],
};
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

function mockFetchJson(body: unknown, init: ResponseInit = {}) {
const fetchMock = jest.fn().mockResolvedValue(
new Response(JSON.stringify(body), {
status: 200,
headers: { "content-type": "application/json" },
...init,
}),
);
globalThis.fetch = fetchMock as unknown as typeof fetch;
return fetchMock;
}

async function expectAppException(
promise: Promise<unknown>,
errorCode: string,
) {
const error = await promise.then(
() => undefined,
(error: unknown) => error,
);

expect(error).toBeInstanceOf(AppException);
expect(error).toMatchObject({ errorCode });
}

describe("KakaoProvider", () => {
afterEach(() => {
globalThis.fetch = originalFetch;
});

it("keyword search API를 올바른 query와 인증 헤더로 호출한다", async () => {
const fetchMock = mockFetchJson(createKakaoResponse());
const provider = createProvider();

await provider.search(query);

const [url, init] = fetchMock.mock.calls[0] as [
string,
RequestInit | undefined,
];
const requestUrl = new URL(url);
expect(requestUrl.origin).toBe("https://dapi.kakao.com");
expect(requestUrl.pathname).toBe("/v2/local/search/keyword.json");
expect(requestUrl.searchParams.get("query")).toBe(
"서울 강남구 카카오프렌즈",
);
expect(requestUrl.searchParams.get("size")).toBe("15");
expect(init?.headers).toEqual({
Authorization: "KakaoAK test-api-key",
});
});

it("Kakao 응답을 GeoCandidate로 정규화한다", async () => {
mockFetchJson(createKakaoResponse());
const provider = createProvider();

const result = await provider.search(query);

expect(result).toEqual([
{
provider: "kakao",
providerPlaceId: "26338954",
placeName: "카카오프렌즈 코엑스점",
address: "서울 강남구 삼성동 159",
coordinate: {
lat: 37.51207412593136,
lng: 127.05902969025047,
},
distance: 418,
mapUrl: "http://place.map.kakao.com/26338954",
phone: "02-6002-1880",
category: "가정,생활 > 문구,사무용품 > 디자인문구 > 카카오프렌즈",
},
]);
});

it("distance가 없거나 빈 문자열이면 distance를 생략한다", async () => {
mockFetchJson(createKakaoResponse({ distance: "" }));
const provider = createProvider();

const [candidate] = await provider.search(query);

expect(candidate.distance).toBeUndefined();
});

it("fetch가 실패하면 KAKAO_REQUEST_FAILED를 던진다", async () => {
globalThis.fetch = jest
.fn()
.mockRejectedValue(new Error("network error")) as unknown as typeof fetch;
const provider = createProvider();

await expectAppException(provider.search(query), "KAKAO_REQUEST_FAILED");
});

it("429 응답이면 KAKAO_RATE_LIMITED를 던진다", async () => {
mockFetchJson({ error: "rate limited" }, { status: 429 });
const provider = createProvider();

await expectAppException(provider.search(query), "KAKAO_RATE_LIMITED");
});

it("2xx 응답이 아니면 KAKAO_REQUEST_FAILED를 던진다", async () => {
mockFetchJson({ error: "bad gateway" }, { status: 502 });
const provider = createProvider();

await expectAppException(provider.search(query), "KAKAO_REQUEST_FAILED");
});

it("응답 형식이 다르면 KAKAO_RESPONSE_INVALID를 던진다", async () => {
mockFetchJson(createKakaoResponse({ x: "not-a-number" }));
const provider = createProvider();

await expectAppException(provider.search(query), "KAKAO_RESPONSE_INVALID");
});
});
131 changes: 128 additions & 3 deletions src/infrastructures/geocoder/providers/kakao.provider.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,144 @@
import { Injectable } from "@nestjs/common";
import { HttpStatus, Injectable } from "@nestjs/common";
import { ConfigService } from "@nestjs/config";
import * as v from "valibot";
import { AppException } from "../../../common/exceptions/app.exception";
import type { Env } from "../../../config/env.schema";
import type {
GeoCandidate,
GeocoderProvider,
GeoQuery,
} from "../geocoder.type";
import {
type KakaoKeywordDocument,
kakaoKeywordSearchResponseSchema,
} from "./kakao.type";

const KAKAO_KEYWORD_SEARCH_URL =
"https://dapi.kakao.com/v2/local/search/keyword.json";
const KAKAO_SEARCH_SIZE = "15";
const KAKAO_REQUEST_TIMEOUT_MS = 5000;

@Injectable()
export class KakaoProvider implements GeocoderProvider {
readonly name = "kakao" as const;

constructor(private readonly configService: ConfigService<Env>) {}

async search(_query: GeoQuery): Promise<GeoCandidate[]> {
throw new Error("Not implemented");
async search(query: GeoQuery): Promise<GeoCandidate[]> {
const apiKey = this.configService.getOrThrow("KAKAO_REST_API_KEY", {
infer: true,
});
const url = this.createKeywordSearchUrl(query);
let response: Response;
Comment on lines +31 to +32

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💬
사소한 질문이 있는데요. 저는 뭔가... response, url 과 같은 변수명이 명확하지않아서 좀 지양하려 하거든요..!
물론, 지금 이 service logic에서는 이해가 안될 부분이 전혀 없지만.. 어떻게 생각하시는지 궁금해요!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

response, url 과 같은 변수명이 명확하지않아서 좀 지양하려 하거든요..!

제 생각은..! 예전에는 저도 싫어서 많이 지양하려고 했던 것 같은데요
어느순간 마음의 불편함이 사라지기 시작한 것 같아요!

  1. ai를 사용하면서 더 굳이굳이인 부분이 되기도 했고, ai가 해당 맥락을 명시하지 않는 경우가 꽤나 많은데 크게 중요하지 않은 부분에 일일이 수정하라고 하는 비용이 더 든다고 느꼈어요!
  2. search(searchUrl: string) vs search(url: string) 을 보면, 저의 경우는 전자는 해당 함수에 다른 url이 또 있나? 라는 생각이 들고, 후자는 우선 저 url은 search url이겠구나, 그리고 다른 url이 없나? 라는 생각이 들어요
    결국, 가독성이 나아진다고 느껴지지 않더라구요
    (한 문맥에 여러 컨텍스트가 있는 경우(e.g. response 여러개, url 여러개) 가 아닌 하나의 컨텍스트만 있거나, 해당 문맥이 짧은 경우)


try {
response = await fetch(url, {
headers: { Authorization: `KakaoAK ${apiKey}` },
signal: AbortSignal.timeout(KAKAO_REQUEST_TIMEOUT_MS),
});
} catch {
throw new AppException(
"KAKAO_REQUEST_FAILED",
"카카오 장소 검색 요청에 실패했습니다.",
HttpStatus.BAD_GATEWAY,
);
}

if (response.status === HttpStatus.TOO_MANY_REQUESTS) {
throw new AppException(
"KAKAO_RATE_LIMITED",
"카카오 장소 검색 요청 한도를 초과했습니다.",
HttpStatus.TOO_MANY_REQUESTS,
);
}
Comment on lines +47 to +53

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💬
요청 한도가 초과되기 전에 파악하는 방법이 있을까요?
만약 요청 한도가 초과되어 이게 에러가 나기 시작한다면, 감당 안되질 거 같아서 대안이 있으면 어떨까 했습니다.

  • 요청 한도를 조회하다가 한도 시점이 오면 discord에 알림
  • 카카오에 카드를 등록하여 한도를 초과하더라도 자동 결제
  • fallback 대응

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

요청 한도가 초과되기 전에 파악하는 방법이 있을까요?

찾아보니 응답에도 없고 사용량 조회 API도 없네요!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

제안주신것들 좋습니다! 요거는 회의때 한번 논의해보면 좋을것 같은데요 그 후에 작업 분리해서 가시죠ㅎㅎ


if (!response.ok) {
throw new AppException(
"KAKAO_REQUEST_FAILED",
"카카오 장소 검색 요청에 실패했습니다.",
HttpStatus.BAD_GATEWAY,
);
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

const body = await this.parseJson(response);
const parsed = v.safeParse(kakaoKeywordSearchResponseSchema, body);

if (!parsed.success) {
throw this.createInvalidResponseException();
}

return parsed.output.documents.map((document) =>
this.toGeoCandidate(document),
);
}

private createKeywordSearchUrl(query: GeoQuery): string {
const url = new URL(KAKAO_KEYWORD_SEARCH_URL);
url.searchParams.set("query", this.createKeyword(query));
url.searchParams.set("size", KAKAO_SEARCH_SIZE);

return url.toString();
}

private createKeyword(query: GeoQuery): string {
return [query.areaName, query.placeName]
.map((value) => value.trim())
.filter(Boolean)
.join(" ");
}

private async parseJson(response: Response): Promise<unknown> {
try {
return await response.json();
} catch {
throw this.createInvalidResponseException();
}
}

private toGeoCandidate(document: KakaoKeywordDocument): GeoCandidate {
return {
provider: this.name,
providerPlaceId: document.id,
placeName: document.place_name,
address: document.address_name,
coordinate: {
lat: this.parseCoordinate(document.y),
lng: this.parseCoordinate(document.x),
},
distance: this.parseDistance(document.distance),
mapUrl: document.place_url,
phone: document.phone || undefined,
category: document.category_name || undefined,
};
}

private parseCoordinate(value: string): number {
const coordinate = Number(value);

if (!Number.isFinite(coordinate)) {
throw this.createInvalidResponseException();
}

return coordinate;
}

private parseDistance(value: string | undefined): number | undefined {
if (!value) return undefined;

const distance = Number(value);

if (!Number.isFinite(distance)) {
throw this.createInvalidResponseException();
}

return distance;
}

private createInvalidResponseException(): AppException {
return new AppException(
"KAKAO_RESPONSE_INVALID",
"카카오 장소 검색 응답 형식이 올바르지 않습니다.",
HttpStatus.BAD_GATEWAY,
);
}
}
Loading
Loading