[FEAT] api client axios -> fetch 수정 - #13
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAPI 응답·오류 계약과 공통 오류 코드를 정의하고, 타임아웃·쿠키·FormData·토큰 갱신을 지원하는 fetch 기반 API 클라이언트를 추가합니다. 401 응답은 토큰 갱신 후 한 번 재시도합니다. ChangesFetch API 클라이언트
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant fetchInstance
participant API서버
participant refreshAccessToken
Client->>fetchInstance: HTTP 메서드와 endpoint 호출
fetchInstance->>API서버: timeout·쿠키·body가 포함된 요청
API서버-->>fetchInstance: 성공 또는 401 응답
fetchInstance->>refreshAccessToken: 401이면 토큰 갱신
refreshAccessToken->>API서버: refresh endpoint POST
API서버-->>refreshAccessToken: 갱신 결과
fetchInstance->>API서버: 원래 요청 1회 재시도
fetchInstance-->>Client: data 반환 또는 ApiError 발생
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 3
🤖 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/lib/api/fetchInstance.ts`:
- Around line 48-53: Replace the direct fetch calls in the refresh API flow with
the existing axiosInstance from src/lib/api/axiosInstance.ts, preserving the
POST endpoint, credentials, headers, timeout signal, and response handling.
Update the surrounding logic in the refresh-related function without introducing
a separate Fetch-based client.
- Around line 45-57: Update refreshAccessToken to route non-OK refresh responses
through the existing common API error parsing instead of always throwing
ApiError with status 401. Preserve the response’s actual status and code, and
continue propagating non-AbortError network or timeout exceptions unchanged.
- Around line 99-101: Update the SSR retry flow around refreshAccessToken and
request<T> so the retried request uses the refreshed authentication cookies
instead of rereading the original server cookies. Route the refresh/retry
through a Route Handler or BFF, or explicitly pass the updated cookies into
request<T>, while preserving the non-refresh request path.
🪄 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: CHILL
Plan: Pro Plus
Run ID: e72206b9-33a8-4eeb-b669-c057ee95c3e9
📒 Files selected for processing (3)
src/lib/api/fetchInstance.tssrc/lib/constants/errorCodes.tssrc/types/api.ts
juengseulki
left a comment
juengseulki
left a comment
There was a problem hiding this comment.
📋 PR 리뷰
Axios 기반 API Client를 Fetch 기반으로 전환하고, 요청 타임아웃, 서버 환경 쿠키 전달, 공통 응답 및 오류 타입, 401 발생 시 토큰 갱신 후 재시도 로직을 추가한 변경 사항을 확인했습니다.
👍 좋았던 점
- 브라우저와 서버 환경을 구분하여 서버 컴포넌트에서는 요청 쿠키를 직접 전달하도록 구성했습니다.
- FormData 요청에는 Content-Type을 직접 지정하지 않아 브라우저가 boundary를 설정할 수 있도록 처리했습니다.
- 기본 타임아웃과 호출부의 취소 신호를 함께 사용할 수 있도록 구현했습니다.
- Refresh, 로그인, 회원가입 API는 재갱신 대상에서 제외하여 무한 재시도를 방지했습니다.
- 재시도 여부를 한 번으로 제한한 점도 적절했습니다.
204 No Content와 JSON 파싱 실패 상황을 별도로 고려했습니다.- 성공 응답, 오류 응답,
ApiError를 공통 타입으로 정의해 API 사용부에서 일관되게 처리할 수 있도록 구성했습니다.
🚨 수정이 필요한 부분
인라인 코멘트로 아래 내용을 남겼습니다.
- Refresh API 실패를 모두
401로 변환하지 않고 실제 응답의 상태, 오류 코드 및 메시지를 보존해야 합니다. - 서버 컴포넌트에서는 Refresh 응답으로 갱신된 쿠키가 현재
cookies()에 반영되지 않으므로, 현재 방식의 즉시 재시도가 기존 쿠키를 다시 사용할 수 있습니다.
🔍 확인 및 제안
여러 요청이 동시에 401을 반환할 때 Refresh 요청이 중복 실행되지 않도록, 진행 중인 Refresh Promise를 공유하는 방식도 함께 고려하면 좋겠습니다.
질문한 다른 인증·인가 처리는 이번 공통 Client에 미리 모두 넣기보다, 인증 기능을 구현하면서 로그인 상태 초기화나 로그아웃 처리처럼 애플리케이션 상태와 연결되는 부분을 추가하는 편이 자연스럽습니다. 다만 Refresh 동시 요청 제어와 Refresh 실패 처리 기준은 공통 Client의 책임이므로 이번 단계에서 정리하는 것이 좋습니다.
전체적인 Fetch Client 방향과 공통 타입 구조는 잘 잡혀 있지만, 서버 환경의 Refresh 재시도는 실제 인증 동작에 직접 영향을 줄 수 있어 수정 후 확인이 필요해 보입니다. 수고하셨습니다! 😊
There was a problem hiding this comment.
확인했습니다! 이미 리뷰가 나온 부분은 제외하고 아주 사소한 리뷰 하나 추가했습니다! refresh 실패 시 모두 401로 변환하는 로직, 동시에 여러 요청이 401을 받을 경우 처리 로직(가령 첫 요청 시 refreshPromise 같은 것을 생성하고 나머지 요청은 refreshPromise을 기다리게 하는 등)은 수정해서 드렸어야 했는데 죄송합니다
Server Component에서 refresh 문제는 저 같은 경우에는 refresh(+auth 관련 api)를 Route Handler에서 처리하는 방식을 사용했습니다. 그러면 클라이언트에서 바로 백엔드에 요청을 보내는 게 아니라, Route Handler가 백엔드에 요청을 보내고 401이면 refresh 요청을 보낸 뒤, refresh 응답의 Set-Cookie를 받아서 응답에 다시 실어주게 되어 문제를 해결할 수 있습니다!
(아마 아시는 내용이겠지만 그냥 적어보았습니다)
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/app/test/api/page.tsx (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win페이지 전체를 Client Component로 만들지 마세요.
상태와 이벤트 처리만 별도 말단 Client Component로 분리하고,
page.tsx는 Server Component로 유지해 메타데이터도 함께 정의하세요. As per path instructions,"use client" 는 가능한 말단 컴포넌트에만 두고, 페이지 전체가 클라이언트 컴포넌트가 되지 않도록 해야 합니다.🤖 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/app/test/api/page.tsx` at line 1, Remove the "use client" directive from the page component so page.tsx remains a Server Component and can define metadata. Extract only the stateful and event-handling UI into a separate leaf Client Component, then render that component from the page while keeping the page’s server-rendered content intact.Source: Path instructions
🤖 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/app/test/api/page.tsx`:
- Line 12: Replace the hardcoded "/auth/me" initial value in the endpoint state
with the corresponding constant from API_ROUTES imported from
src/lib/constants/apiRoutes.ts, keeping the existing useState behavior
unchanged.
- Around line 53-79: Connect accessible labels to each form control in the API
test UI: add distinct label text and matching id/htmlFor pairs for the method
select, endpoint input, and conditionally rendered request-body textarea. Keep
the existing values, handlers, and conditional rendering unchanged.
---
Nitpick comments:
In `@src/app/test/api/page.tsx`:
- Line 1: Remove the "use client" directive from the page component so page.tsx
remains a Server Component and can define metadata. Extract only the stateful
and event-handling UI into a separate leaf Client Component, then render that
component from the page while keeping the page’s server-rendered content intact.
🪄 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: CHILL
Plan: Pro Plus
Run ID: 451a8a57-bcc8-419e-b5b3-fe90906d4341
📒 Files selected for processing (2)
src/app/test/api/page.tsxsrc/lib/api/fetchInstance.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/api/fetchInstance.ts
| import { API_ROUTES } from "@/lib/constants/apiRoutes"; | ||
| import { ApiError, type ApiSuccessResponse, type ApiErrorResponse } from "@/types/api"; | ||
|
|
||
| const BASE_URL = process.env.NEXT_PUBLIC_API_BASE_URL; |
There was a problem hiding this comment.
NEXT_PUBLIC_API_BASE_URL이 설정되지 않으면 요청 URL이 undefined/auth/me처럼 만들어져서 원인 찾기 어려운 에러가 날 것 같습니다. 미설정 시 바로 에러를 던지도록 가드를 추가하는 것도 좋을 것 같습니다.
|
구현하느라 고생 많으셨습니다 👍 다른 auth 처리를 미리 추가할지 고민하신 부분은, 지금의 범위대로 구현해두어도 괜찮을 것 같습니다. 앞선 리뷰에서 나온 서버 컴포넌트 refresh 재시도 건만 인증 작업 때 잊지 않고 같이 정리해주시면 될 것 같습니다. :) |
4 participants
📋 작업 내용
🔥 변경 사항
AbortSignal->axios timeout대체), 서버 컴포넌트용 쿠키 포워딩, 401 응답 시 리프레시 토큰 재시도 로직을 포함하였습니다.errorCodes.ts로 분리하여 상수로 관리하도록 하였습니다.ApiSuccessResponse,ApiErrorResponse,ApiError타입을types/api.ts에 정의하였습니다.✅ 체크리스트
💬 To Reviewer
Summary by CodeRabbit
새로운 기능
버그 수정