-
Notifications
You must be signed in to change notification settings - Fork 0
[Feat] axios 인터셉터 refresh 로직 추가 #217
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
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Walkthrough401 응답 시 인증 재시도 워크플로우를 추가합니다. 401 에러가 발생하고 아직 재시도하지 않았을 경우, API 인증 서비스를 통해 토큰을 갱신한 후 원래 요청을 재시도합니다. 갱신 실패 시 서버는 로그인 페이지로, 클라이언트는 에러와 경로를 포함한 로그인 URL로 리다이렉트합니다. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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 |
🎨 Storybook Report✅ 변경 사항이 없습니다 모든 Story가 이전 빌드와 동일합니다.
|
📊 Coverage Report
📉 #217을 main에 병합하면 coverage가 Coverage 요약@@ Coverage Diff @@
## main #217 +/- ##
===========================================
- Coverage 33.61% 33.57% -0.04%
===========================================
Files 158 158 0
Lines 6712 6726 +14
Branches 250 250 0
===========================================
+ Hits 2256 2258 +2
+ Misses 4456 4468 +12 영향받은 파일
|
🎭 Playwright Report✨ E2E Test가 성공적으로 완료되었습니다. Test 요약 내용을 확인해주세요.
📊 Test Summary
📜 Test Details✅ Passed Tests (9)
|
🚀 PR Preview Report✨ Build가 성공적으로 완료되었습니다. Preview에서 변경사항을 확인하세요.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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
🧹 Nitpick comments (1)
src/api/core/index.ts (1)
54-56: 동시 요청 시 여러 번의 refresh 호출이 발생할 수 있습니다.여러 요청이 동시에 401을 받으면, 각각 독립적으로
refresh()를 호출합니다. 이로 인해 불필요한 네트워크 요청이 발생하거나, refresh token이 이미 사용되어 후속 refresh가 실패할 수 있습니다.Promise 큐 패턴을 사용하여 하나의 refresh만 진행하고, 나머지 요청은 해당 결과를 기다리도록 하는 것이 좋습니다.
🔎 개선 예시
// 파일 상단에 추가 let isRefreshing = false; let refreshSubscribers: Array<() => void> = []; const subscribeTokenRefresh = (callback: () => void) => { refreshSubscribers.push(callback); }; const onRefreshed = () => { refreshSubscribers.forEach((callback) => callback()); refreshSubscribers = []; }; // 401 핸들링 부분 수정 if (status === 401 && !originalRequest._retry) { originalRequest._retry = true; if (isRefreshing) { return new Promise((resolve) => { subscribeTokenRefresh(() => { resolve(baseAPI(originalRequest)); }); }); } isRefreshing = true; try { await API.authService.refresh(); onRefreshed(); return baseAPI(originalRequest); } catch (refreshError) { // ... 기존 에러 핸들링 } finally { isRefreshing = false; } }
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/api/core/index.ts(2 hunks)src/api/service/auth-service/index.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/api/core/index.ts (1)
src/api/index.ts (1)
API(22-22)
src/api/service/auth-service/index.ts (2)
src/api/core/index.ts (1)
api(113-113)src/types/service/auth.ts (1)
RefreshResponse(30-35)
🔇 Additional comments (2)
src/api/service/auth-service/index.ts (1)
30-35: LGTM!
withCredentials: true설정으로 쿠키 기반 refresh token 전송이 가능하고,_retry: true플래그로 refresh 요청 자체가 401 인터셉터에서 재시도되지 않도록 방지합니다.src/api/core/index.ts (1)
5-6: 순환 import는 존재하지만 런타임 에러를 야기하지 않습니다.
src/api/core/index.ts가API를src/api/index.ts에서 import하고, 이는authServiceRemote를 통해src/api/service/auth-service/index.ts로 연결되며, 해당 파일이 다시api를@/api/core에서 import하는 구조입니다. 그러나 이는 문제가 되지 않는데, 그 이유는:
- auth-service에서
api는 함수 본문 내부(async 함수들)에서만 사용되며, 모듈 로드 시점에는 사용되지 않습니다- core/index.ts에서
API는 응답 인터셉터 콜백(async 함수) 내부에서만 사용됩니다- 함수들이 실제로 호출될 시점에는 모든 모듈 로드가 완료되므로 순환 참조가 해결됩니다
| if (status === 401 && !originalRequest._retry) { | ||
| try { | ||
| await API.authService.refresh(); | ||
| return baseAPI(originalRequest); | ||
| } catch (refreshError) { | ||
| if (isServer) { | ||
| const { redirect } = await import('next/navigation'); | ||
| redirect('/login'); | ||
| } else { | ||
| if (window.location.pathname === '/login') { | ||
| throw errorResponse; | ||
| } | ||
| const currentPath = window.location.pathname + window.location.search; | ||
| window.location.href = `/login?error=unauthorized&path=${encodeURIComponent(currentPath)}`; | ||
| } | ||
| const currentPath = window.location.pathname + window.location.search; | ||
| window.location.href = `/login?error=unauthorized&path=${encodeURIComponent(currentPath)}`; | ||
| throw refreshError; | ||
| } | ||
| } |
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.
_retry 플래그 설정이 누락되어 무한 루프가 발생할 수 있습니다.
originalRequest._retry를 확인하지만, 재시도 전에 true로 설정하지 않습니다. 재시도된 요청이 다시 401을 반환하면 또 다른 refresh 시도가 발생하여 무한 루프에 빠질 수 있습니다.
🔎 수정 제안
if (status === 401 && !originalRequest._retry) {
+ originalRequest._retry = true;
try {
await API.authService.refresh();
return baseAPI(originalRequest);
} catch (refreshError) {🤖 Prompt for AI Agents
In src/api/core/index.ts around lines 53 to 70, the code checks
originalRequest._retry but never sets it, which can cause repeated refresh
attempts and an infinite loop; set originalRequest._retry = true before calling
API.authService.refresh() (so the retried request won’t trigger another
refresh), then proceed with the refresh and re-dispatch of the request; ensure
the flag is set on the same request object passed to baseAPI and do not rely on
external state.
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.
이미 API.authService.refresh 함수에서 _retry: true로 강제 설정하고 있기 때문에 무한 루프가 발생하지 않습니다.
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.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Get the auth-service refresh function implementation
cat src/api/service/auth-service/index.ts
Length of output: 1053
@Chiman2937, 말씀하신 내용을 확인했습니다. 다만 약간의 혼동이 있는 것 같습니다:
현재 구현:
API.authService.refresh()에서{ _retry: true }를 설정하는 것은 refresh 요청 자체 (POST /auth/refresh)에 대한 플래그입니다- 이것은 refresh 엔드포인트가 401을 반환할 때 재시도를 방지합니다
하지만:
originalRequest(처음 401을 받은 요청)에는_retry플래그가 여전히 설정되지 않습니다- 만약 refresh가 성공했지만 토큰이 여전히 유효하지 않거나, 경쟁 조건이 발생하면:
return baseAPI(originalRequest)재시도- 다시 401 응답 받음
!originalRequest._retry조건이 여전히 true (undefined)- 또 다른 refresh 시도 → 잠재적 무한 루프
권장 사항:
if (status === 401 && !originalRequest._retry) {
originalRequest._retry = true; // 이 라인 추가
try {
await API.authService.refresh();
return baseAPI(originalRequest);이렇게 하면 모든 엣지 케이스를 안전하게 방지할 수 있습니다. 현재 로직이 대부분의 경우 동작하더라도, originalRequest._retry를 명시적으로 설정하는 것이 더 안전한 방법입니다.
🧬 Code Graph Analysis Results
[src/api/core/index.ts] 401 handling block (shows use of originalRequest._retry and refresh flow)
if (status === 401 && !originalRequest._retry) {
try {
await API.authService.refresh();
return baseAPI(originalRequest);
} catch (refreshError) {
if (isServer) {
const { redirect } = await import('next/navigation');
redirect('/login');
} else {
if (window.location.pathname === '/login') {
throw errorResponse;
}
const currentPath = window.location.pathname + window.location.search;
window.location.href = /login?error=unauthorized&path=${encodeURIComponent(currentPath)};
}
throw refreshError;
}
}
[src/api/index.ts] provide API service reference (relevant to understanding how API.authService.refresh is accessed)
export const API = provideAPIService();
[src/types/service/common.ts] Common error/success response types (context for error handling in interceptors)
export interface CommonErrorResponse {
type: string;
title: string;
status: number;
detail: string;
instance: string;
errorCode?: string;
}
export interface CommonSuccessResponse {
status: number;
success: boolean;
data: T;
}
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.
해당 내용은 논리적으로 이해 되지 않습니다.
-
GET /api/v1/users/me와POST /api/v1/groups/createapi가 동시에 호출된다고 가정하겠습니다.
이때 originalRequest._retry를 true로 설정한다고 해도 각 api 호출 시 axios 인스턴스가 별도로 생성되기 때문에 _retry 플래그를 true로 설정한다고 해서 서로 공유되지 않습니다. 이 때문에 _retry 플래그를 true를 설정하는 것이 의미가 없다고 생각됩니다. -
애초에 status 401이 발생했을 때는 refresh 호출 로직밖에 없고, 그 후에는 다시 원래 api를 호출하는 것이 아닌 /login 페이지로 리다이렉트 되면서 api 호출을 중단합니다.
여기서 무한루프를 일으킬 여지가 있나요?
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.
@Chiman2937, 좋은 질문입니다. 제가 설명을 더 명확히 드리겠습니다.
1번 포인트에 대한 답변: _retry 플래그의 위치
_retry 플래그는 axios 인스턴스가 아닌 요청 config 객체(error.config)에 설정됩니다.
const originalRequest = error.config; // 이것이 요청별 config 객체입니다동시에 여러 API가 호출되는 경우:
GET /api/v1/users/me의 config 객체 → 각자의_retry플래그 보유POST /api/v1/groups/create의 config 객체 → 각자의_retry플래그 보유
각 요청은 자체 config 객체를 가지므로, 서로 간섭하지 않습니다. 따라서 originalRequest._retry = true 설정은 그 특정 요청에 대한 재시도만 방지합니다.
2번 포인트에 대한 답변: 무한 루프 시나리오
코드를 다시 살펴보면:
if (status === 401 && !originalRequest._retry) {
try {
await API.authService.refresh(); // refresh 성공
return baseAPI(originalRequest); // 원래 요청 재시도 ← 여기가 핵심
} catch (refreshError) {
// refresh 실패 시에만 /login으로 리다이렉트
}
}무한 루프 발생 시나리오:
- 요청 A가 401 반환
!originalRequest._retry체크 통과 (undefined)API.authService.refresh()호출 → 성공return baseAPI(originalRequest)- 요청 A 재시도- 만약 재시도된 요청 A가 다시 401을 반환하면? (예: 경쟁 조건, 네트워크 지연, 서버 문제 등)
- 다시 인터셉터 진입 →
!originalRequest._retry체크 (여전히 undefined) → 통과 - 다시 refresh 시도 → 2번으로 돌아가며 무한 루프
중요한 차이점:
API.authService.refresh()의{ _retry: true }→ refresh 엔드포인트 자체가 401을 받을 때만 보호originalRequest._retry = true→ 원래 실패한 요청(예: users/me)이 재시도 후 다시 401을 받을 때 보호
이것이 표준적인 axios 인터셉터 refresh 패턴입니다. 대부분의 경우 문제없이 작동하지만, 엣지 케이스를 방지하기 위해 originalRequest._retry = true를 설정하는 것이 권장됩니다.
혹시 제 설명에서 여전히 이해가 안 되는 부분이 있으시면 말씀해 주세요! 🙂
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.
refresh api에서 정상적으로 200응답이 온 직후더라도 다시 401 에러가 발생하는 경우를 대비한 설정이라는 건가요?
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.
@Chiman2937, 네, 정확히 이해하셨습니다! 👍
시나리오:
- Refresh API가 200으로 성공 응답
- 새로운 accessToken이 쿠키에 설정됨
return baseAPI(originalRequest)실행 - 원래 요청 재시도- 그런데 이 재시도된 요청이 다시 401을 반환하는 경우
이런 경우가 발생할 수 있는 상황:
- 경쟁 조건(Race Condition): 여러 요청이 동시에 401을 받고 refresh를 시도할 때
- 토큰 전파 지연: 쿠키가 설정되었지만 다음 요청에서 아직 반영되지 않은 경우
- 서버 측 타이밍 이슈: 새 토큰이 아직 서버에서 완전히 활성화되지 않은 경우
- 네트워크 지연: 요청 헤더가 이전 상태를 참조하는 경우
결론:
originalRequest._retry = true 설정은 이런 엣지 케이스를 방어하기 위한 안전장치입니다. 대부분의 경우 문제가 없지만, 예상치 못한 401 재발생 시 무한 루프를 확실하게 방지합니다.
이것이 일반적인 axios 인터셉터 refresh 패턴에서 권장되는 이유입니다. 하지만 최종 결정은 팀의 판단에 맡기겠습니다! 😊
📝 변경 사항
api/core/index.tsapi/service/auth-service/index.ts_retry: true플래그를 추가했습니다.withCredentials: true플래그를 추가했습니다.🔗 관련 이슈
Closes #
🧪 테스트 방법
📸 스크린샷 (선택)
📋 체크리스트
💬 추가 코멘트
CodeRabbit Review는 자동으로 실행되지 않습니다.
Review를 실행하려면 comment에 아래와 같이 작성해주세요
Summary by CodeRabbit
릴리스 노트
✏️ Tip: You can customize this high-level summary in your review settings.