-
Notifications
You must be signed in to change notification settings - Fork 0
[init/#110] api 기본 세팅 #111
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
Walkthrough환경변수 기반 Axios 인스턴스, 공통 요청 유틸리티, 표준 응답 타입, React Query 클라이언트 및 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as UI/Caller
participant R as request<T>()
participant A as axiosInstance
participant S as API Server
rect #f8f9fa
note right of A: 공통 설정·토큰 주입·응답 처리
end
U->>R: RequestConfig(method,url,query,body,headers)
R->>A: AxiosRequestConfig 구성 후 요청
A->>A: Request Interceptor\n(Authorization: Bearer accessToken)
A->>S: HTTP 요청
S-->>A: 2xx 응답 (BaseResponse)
A-->>R: response.data
R-->>U: data (T)
alt 401 Unauthorized
S-->>A: 401 에러
A->>A: localStorage.accessToken 제거
A->>U: 리다이렉트(ROUTES.HOME)
end
alt 네트워크/기타 에러
S-->>A: 오류/타임아웃
A-->>R: AxiosError
R->>R: DEV 모드 로깅 후 재던지기
R-->>U: 에러 throw
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks (4 passed, 1 inconclusive)❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ 빌드에 성공했습니다! 🎉 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (5)
src/api/axiosInstance.ts (3)
31-35: 401 처리 시 히스토리 오염 방지 및 리다이렉트 루프 가드현재는 HOME으로 href 이동이라 뒤로가기로 보호 페이지로 쉽게 회귀할 수 있고, 이미 HOME인 경우에도 불필요한 네비게이션이 발생합니다. replace와 경로 가드를 권장합니다.
- window.location.href = ROUTES.HOME; + if (window.location.pathname !== ROUTES.HOME) { + window.location.replace(ROUTES.HOME); + }
4-10: baseURL 누락 시 탐지하기환경 변수 미설정으로 인해 빈 baseURL로 나가는 사고를 피하려면 개발 모드에서 경고/에러를 내는 가드를 추가하세요.
가능한 접근:
- DEV에서 VITE_API_BASE_URL falsy 시 console.warn 또는 throw
- e2e/로컬 프로필별 .env 검증 스크립트 추가
12-18: 로컬스토리지 토큰 주입 방식에 대한 보안 고려XSS에 노출되면 accessToken 탈취 위험이 있습니다. 장기적으로는 httpOnly 쿠키 + CSRF 대책(또는 Double Submit/SameSite)으로의 전환을 검토하세요. 단기적으로는 CSP 강화, DOM XSS 린트, 신뢰되지 않은 문자열의 안전한 렌더링을 권장합니다.
src/api/request.ts (2)
26-37: GET 요청에 body가 실리는 혼동 방지axios는 GET의 data를 전송하지 않지만, config에 data를 넣으면 디버깅이 어렵습니다. 바디 허용 메서드에서만 data를 설정하세요. 또한 Content-Type 기본 설정 병합을 단순화할 수 있습니다.
- const requestConfig: AxiosRequestConfig = { - method, - url, - params: query, - data: body, - }; + const requestConfig: AxiosRequestConfig = { + method, + url, + params: query, + }; + + if (body !== undefined && method !== HTTPMethod.GET) { + requestConfig.data = body; + } - if (headers) { - requestConfig.headers = headers; - } else if (body && !(body instanceof FormData)) { - requestConfig.headers = { 'Content-Type': 'application/json' }; - } + // 헤더 병합: 제공된 헤더가 있으면 우선하되, JSON 바디면 기본 Content-Type 보장 + requestConfig.headers = { + ...(body && !(body instanceof FormData) ? { 'Content-Type': 'application/json' } : {}), + ...(headers ?? {}), + };
39-42: 서버 표준 응답(success/code) 확인 로직 검토백엔드가 success=false로 에러를 표현하는 경우(2xx + 실패 메시지)에도 현재는 data를 그대로 반환합니다. 필요 시 success=false 또는 특정 code 범위에서 throw하도록 정책을 정해 주세요.
원하시면 success/code 규칙을 반영한 공통 에러 처리 분기(예: 성공 시에만 data 반환)를 제안해 드리겠습니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
package.json(1 hunks)src/api/axiosInstance.ts(1 hunks)src/api/queryClient.ts(1 hunks)src/api/request.ts(1 hunks)src/api/types.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/api/types.ts (1)
api/data-contracts.ts (10)
BaseResponsePromisePlaceResponseDTO(717-727)BaseResponsePlaceSummaryInfoResponseDTO(987-997)BaseResponsePromiseStatusResponseDTO(692-702)BaseResponseVoid(40-50)BaseErrorResponse(13-20)BaseResponsePinResponseDTO(1070-1080)BaseResponsePendingPromiseResponseDTO(763-773)BaseResponsePromiseManagementResponseDTO(851-861)BaseResponsePromiseStatusConfirmedPromiseResponseDTO(809-819)BaseResponsePinPageResponseDTO(1115-1125)
src/api/axiosInstance.ts (2)
src/router/constant/Routes.ts (1)
ROUTES(1-8)api/http-client.ts (3)
constructor(69-82)ApiConfig(45-52)T(139-185)
src/api/request.ts (1)
src/api/types.ts (1)
BaseResponse(1-5)
🔇 Additional comments (3)
package.json (2)
20-20: axios 추가 적절합니다Swagger로 생성된 클라이언트와 신규 axios 유틸들에 필요한 코어 의존성 추가로 보이며, 버전 범위도 합리적입니다.
15-30: axios 버전/보안 승인
현재 ^1.11.0이 2025-09-08 기준 최신 안정화 릴리스이며, 1.11.x에 대한 알려진 CVE가 없습니다.src/api/request.ts (1)
1-11: axios 클라이언트 중복 사용 여부 및 일원화 검증 필요
swagger-typescript-api로 생성된 모든 API 클라이언트가 자체 axios 인스턴스를 생성하지 않고, 공통axiosInstance를 주입받도록 구성됐는지 확인해 주세요.
또한,src디렉터리 전역에서import 'axios'또는require('axios')로 직접 사용한 부분이 없는지 재검토가 필요합니다.
|
✅ 빌드에 성공했습니다! 🎉 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/api/request.ts (1)
52-56: OR 우선순위/폴백 버그로 메시지가 비어도 폴백이 적용되지 않음
status + ' ' + message는 항상 truthy여서 “알 수 없는 오류...”가 표시되지 않습니다. 안전한 결합과 폴백을 적용하세요.- const displayMessage = status + ' ' + message; + const combined = [status, message ?? ''].filter(Boolean).join(' ').trim(); + const displayMessage = combined || '알 수 없는 오류가 발생했습니다.'; if (import.meta.env.DEV) { console.error(`[실패] ${url} : ${displayMessage}`); }
🧹 Nitpick comments (3)
src/api/request.ts (3)
33-37: 헤더 병합 로직 개선 — JSON 본문 시 Content-Type 보장현재는
headers가 전달되면 JSON 본문이어도Content-Type을 설정하지 않습니다. 서버에 따라 문제될 수 있으니 병합 방식으로 보장하세요. FormData일 땐 설정하지 말아야 합니다.- if (headers) { - requestConfig.headers = headers; - } else if (body && !(body instanceof FormData)) { - requestConfig.headers = { 'Content-Type': 'application/json' }; - } + if (body && !(body instanceof FormData)) { + requestConfig.headers = { 'Content-Type': 'application/json', ...(headers ?? {}) }; + } else if (headers) { + requestConfig.headers = headers; + }
15-21: 요청 설정 타입 범위 확장 제안(배열/nullable 지원)쿼리/바디 입력이 실제 사용에서 배열·nullable을 포함하는 경우가 많습니다. 초기에 넓혀두면 캐스팅을 줄일 수 있습니다.
export interface RequestConfig { method: HTTPMethodType; url: string; - query?: Record<string, string | number | boolean>; - body?: Record<string, unknown> | FormData; + query?: Record<string, string | number | boolean | null | undefined>; + body?: Record<string, unknown> | string | Blob | FormData; headers?: Record<string, string>; }
48-61: 에러 표준화 가벼운 제안
error.response가 있는 경우에도data?.message타입이 string이 아닐 수 있습니다. 메시지 생성 시 문자열 강제/트리밍을 보장해 로그 품질을 높이세요.- const { status, data } = error.response; - const message = data?.message; + const { status, data } = error.response; + const message = typeof data?.message === 'string' ? data.message : '';
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/App.tsx(1 hunks)src/api/request.ts(1 hunks)src/api/types.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/api/types.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/api/request.ts (2)
src/api/types.ts (1)
BaseResponse(1-6)api/http-client.ts (6)
HttpClient(62-186)T(139-185)FullRequestParams(24-38)ApiConfig(45-52)mergeRequestParams(88-108)constructor(69-82)
🪛 GitHub Check: build
src/api/request.ts
[failure] 41-41:
Type 'T | undefined' is not assignable to type 'T'.
🪛 GitHub Actions: CI/CD Pipeline
src/api/request.ts
[error] 41-41: TypeScript error TS2322: Type 'T | undefined' is not assignable to type 'T' (src/api/request.ts:41).
🔇 Additional comments (1)
src/App.tsx (1)
4-6: 중앙 집중 QueryClient 싱글턴 도입 좋습니다App에서 인스턴스를 생성하지 않고 공용 queryClient를 주입해 HMR/리렌더 시 재생성 문제를 피할 수 있습니다. 👍
|
✅ 빌드에 성공했습니다! 🎉 |
|
✅ 빌드에 성공했습니다! 🎉 |
|
✅ 빌드에 성공했습니다! 🎉 |
📌 Related Issues
✅ 체크 리스트
📄 Tasks
저번 해커톤 때 사용했던 설정을 보고 작성하였습니다.
⭐ PR Point (To Reviewer)
하지만 constansts 파일 부분을 다 뺐는데
이유는 401 error는 같지만 나머지 에러가 조금이나마 다 달라서 하나로 묶는 것이 이상했기 때문입니다.
그리고 requestConfig: any => requestConfig: AxiosRequestConfig 로 바꿔 좀 더 타입을 안정성 있게 구축했습니다.
Record<string, unknown> 은 “모든 string 키에 값이 있다”는 인덱스 시그니처를 요구합니다.
반면 LoginRequestDTO 같은 보통의 인터페이스는 특정 키만 갖고 있고, “모든 string 키”를 보장하지 않죠.
그래서 LoginRequestDTO → Record<string, unknown> 로는 할당 불가이기 때문에 unknown으로 수정 완
📷 Screenshot
🔔 ETC
Summary by CodeRabbit