Fix(client, extension): interceptor refresh 로직 제거 및 isRedirecting flag 추가 #226
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Walkthrough인증 엔드포인트를 API v1에서 v2로 업데이트하고, 재시도 요청 시 리다이렉트 로직이 중복 실행되지 않도록 방지 로직을 추가했습니다. 기존 온보딩 리다이렉트 흐름은 유지됩니다. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)📓 Common learnings⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (2)
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 |
![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
🧹 Nitpick comments (7)
apps/client/src/shared/apis/setting/axiosInstance.ts (3)
12-20: 불필요한async키워드 제거 권장인터셉터 내부에
await호출이 없으므로async키워드는 불필요합니다.🔎 제안된 수정
-apiRequest.interceptors.request.use(async (config) => { +apiRequest.interceptors.request.use((config) => { const token = localStorage.getItem('token'); if (token) { config.headers.Authorization = `Bearer ${token}`; } return config; });
38-50: Extension과 달리isRedirecting가드 누락PR 설명에 따르면 Extension에서는 다중 API 호출이 동시에 401/403을 반환할 때 여러 온보딩 탭이 열리는 것을 방지하기 위해
isRedirecting플래그를 추가했습니다. 클라이언트에서는 여러 탭이 열리는 문제는 없지만, 동시에 여러 401/403 응답이 발생할 경우window.location.href가 여러 번 설정되고localStorage.removeItem이 중복 호출될 수 있습니다.동일한 패턴을 적용하여 일관성을 유지하는 것을 고려해 주세요.
🔎 제안된 수정
+let isRedirecting = false; + apiRequest.interceptors.response.use( (response) => response, - async (error) => { + (error) => { const originalRequest = error.config; const noAuthNeeded = [ '/api/v1/auth/token', '/api/v2/auth/signup', '/api/v2/auth/google', ]; const isNoAuth = noAuthNeeded.some((url) => originalRequest.url?.includes(url) ); if ( error.response && (error.response.status === 401 || error.response.status === 403) && !originalRequest._retry && - !isNoAuth + !isNoAuth && + !isRedirecting ) { - originalRequest._retry = true; + isRedirecting = true; localStorage.removeItem('token'); window.location.href = '/onboarding?step=SOCIAL_LOGIN'; return Promise.reject(error); } return Promise.reject(error); } );
44-44:_retry플래그는 더 이상 필요하지 않음
_retry플래그는 원래 토큰 새로고침 후 요청 재시도 시 무한 루프를 방지하기 위한 것이었습니다. 재시도 로직이 제거되었으므로 이 플래그도 제거할 수 있습니다. 단, 위에서 제안한isRedirecting가드를 추가하면 이 줄은 자연스럽게 대체됩니다.apps/extension/src/apis/axiosInstance.ts (4)
18-26: 토큰 조회 실패 처리 추가 권장
getTokenFromStorage()가 실패할 경우 요청이 중단될 수 있습니다. try-catch로 오류를 처리하고 토큰 없이 요청을 계속하거나 적절한 오류 로그를 남기는 것이 좋습니다.🔎 오류 처리를 추가한 수정안
apiRequest.interceptors.request.use(async (config) => { - const token = await getTokenFromStorage(); + try { + const token = await getTokenFromStorage(); - if (token) { - config.headers.Authorization = `Bearer ${token}`; - } + if (token) { + config.headers.Authorization = `Bearer ${token}`; + } + } catch (error) { + // 토큰 조회 실패 시 로그만 남기고 요청은 계속 진행 + console.error('Failed to get token from storage:', error); + } return config; });
28-32: 온보딩 URL 환경 변수 분리 작업 추적 필요하드코딩된 URL을 환경 변수로 분리하는 것이 좋습니다. 이미 TODO 주석이 있으므로, 별도 이슈로 생성하여 추적하는 것을 권장합니다.
이슈 생성을 도와드릴까요?
61-63: 타임아웃 값 재검토 필요2초 타임아웃은 다소 임의적입니다. PR 설명에서 피드백을 요청하신 것으로 보이는데, 다음 사항을 고려하세요:
- 여러 요청이 시간차를 두고 실패하는 경우 2초로는 부족할 수 있습니다
- 사용자가 온보딩을 완료하고 돌아올 때까지의 시간을 고려하지 않습니다
- 더 나은 접근: 탭이 닫히거나 인증이 성공할 때까지 플래그를 유지하거나, 위에서 제안한 Promise 기반 패턴을 사용하세요
현재 구현의 문제를 완화하려면, 최소 5초 이상으로 늘리거나 여러 요청의 배치 윈도우를 측정한 데이터를 기반으로 설정하는 것이 좋습니다.
36-70: 사용자 경험 고려사항: 실패한 요청의 재시도 없음현재 구현은 인증 실패 시 온보딩으로 리다이렉트하지만, 실패한 원래 요청들은 재시도되지 않습니다. 토큰이 만료될 때 여러 API 호출이 진행 중이었다면 모두 실패하게 되며, 사용자가 재인증 후에도 수동으로 새로고침하거나 재시도해야 합니다.
다음 개선 사항을 고려해보세요:
- 실패한 요청을 큐에 저장하고 재인증 후 재시도
- 또는 최소한 사용자에게 "재인증 완료 후 페이지를 새로고침하세요"와 같은 안내 제공
- 애플리케이션 레벨에서 인증 상태 변경 이벤트를 리스닝하여 자동 재시도
이는 현재 PR 범위를 벗어날 수 있지만, 향후 UX 개선을 위해 추적하는 것이 좋습니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/client/src/shared/apis/setting/axiosInstance.tsapps/extension/src/apis/axiosInstance.ts
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: constantly-dev
Repo: Pinback-Team/pinback-client PR: 2
File: pnpm-workspace.yaml:3-3
Timestamp: 2025-08-18T13:48:59.065Z
Learning: constantly-dev는 docs 디렉터리를 컨벤션 문서 추가용으로 사용할 예정이라고 명시했습니다.
Learnt from: constantly-dev
Repo: Pinback-Team/pinback-client PR: 5
File: apps/extension/src/index.css:1-1
Timestamp: 2025-08-19T17:18:57.678Z
Learning: constantly-dev는 디자인 시스템 설정 PR 머지 후 `import 'pinback/tailwind-config/shared-styles.css';`를 `app.css`나 `index.css`에서 사용하여 공유 스타일을 관리할 계획입니다.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: storybook
🔇 Additional comments (2)
apps/client/src/shared/apis/setting/axiosInstance.ts (1)
1-9: LGTM!Axios 인스턴스 생성 설정이 표준적이고 적절합니다.
apps/extension/src/apis/axiosInstance.ts (1)
41-48: LGTM: Google OAuth 엔드포인트 추가인증이 필요 없는 엔드포인트 목록에 Google OAuth 경로가 올바르게 추가되었습니다. 로직도 적절합니다.
| const getTokenFromStorage = () => { | ||
| return new Promise<string | undefined>((resolve) => { | ||
| chrome.storage.local.get('token', (result) => { | ||
| resolve(result.token); | ||
| }); | ||
| }); | ||
| }; |
There was a problem hiding this comment.
chrome.storage API 오류 처리 추가 필요
chrome.storage.local.get은 스토리지 할당량 초과, 권한 문제 등으로 실패할 수 있습니다. chrome.runtime.lastError를 확인하거나 Promise를 reject하여 오류를 처리해야 합니다.
🔎 오류 처리를 추가한 수정안
const getTokenFromStorage = () => {
return new Promise<string | undefined>((resolve, reject) => {
chrome.storage.local.get('token', (result) => {
+ if (chrome.runtime.lastError) {
+ reject(chrome.runtime.lastError);
+ return;
+ }
resolve(result.token);
});
});
};📝 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.
| const getTokenFromStorage = () => { | |
| return new Promise<string | undefined>((resolve) => { | |
| chrome.storage.local.get('token', (result) => { | |
| resolve(result.token); | |
| }); | |
| }); | |
| }; | |
| const getTokenFromStorage = () => { | |
| return new Promise<string | undefined>((resolve, reject) => { | |
| chrome.storage.local.get('token', (result) => { | |
| if (chrome.runtime.lastError) { | |
| reject(chrome.runtime.lastError); | |
| return; | |
| } | |
| resolve(result.token); | |
| }); | |
| }); | |
| }; |
🤖 Prompt for AI Agents
In apps/extension/src/apis/axiosInstance.ts around lines 10 to 16, the Promise
wrapper around chrome.storage.local.get does not handle chrome.runtime.lastError
or other failures; update the function to check chrome.runtime.lastError inside
the callback and reject the Promise with that error (or a constructed Error)
when present, and resolve with the token only when no error exists; ensure the
Promise signature remains Promise<string | undefined> and propagate non-storage
exceptions by rejecting so callers can handle failures.
| ? 'http://localhost:5173/onboarding?step=SOCIAL_LOGIN' | ||
| : 'https://pinback.today/onboarding?step=SOCIAL_LOGIN'; | ||
|
|
||
| let isRedirecting = false; |
There was a problem hiding this comment.
모듈 레벨 플래그의 동시성 제약
isRedirecting 플래그는 모듈 레벨 변수로, 동일한 시점에 여러 API 호출이 401/403을 반환할 경우 race condition이 발생할 수 있습니다. 모든 요청이 isRedirecting === false를 확인한 후 거의 동시에 탭 생성을 시도할 수 있습니다.
더 견고한 방법으로 Promise 기반 잠금 메커니즘이나 단일 리다이렉트 Promise를 재사용하는 패턴을 고려해보세요.
🔎 Promise 기반 리다이렉트 가드 예시
-let isRedirecting = false;
+let redirectPromise: Promise<void> | null = null;
+
+const redirectToOnboarding = async () => {
+ if (redirectPromise) {
+ return redirectPromise;
+ }
+
+ redirectPromise = new Promise<void>((resolve) => {
+ chrome.storage.local.remove(['token', 'email'], () => {
+ if (chrome.runtime.lastError) {
+ console.error('Failed to clear storage:', chrome.runtime.lastError);
+ }
+
+ chrome.tabs.create({ url: onboardingUrl }, () => {
+ if (chrome.runtime.lastError) {
+ console.error('Failed to create tab:', chrome.runtime.lastError);
+ }
+ setTimeout(() => {
+ redirectPromise = null;
+ resolve();
+ }, 2000);
+ });
+ });
+ });
+
+ return redirectPromise;
+};그리고 응답 인터셉터에서:
if (
error.response &&
(error.response.status === 401 || error.response.status === 403) &&
!isNoAuth
) {
- if (!isRedirecting) {
- isRedirecting = true;
-
- chrome.storage.local.remove(['token', 'email'], () => {});
-
- chrome.tabs.create({ url: onboardingUrl }, () => {
- setTimeout(() => {
- isRedirecting = false;
- }, 2000);
- });
- }
+ await redirectToOnboarding();
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/extension/src/apis/axiosInstance.ts around line 34, the module-level
boolean isRedirecting causes race conditions when multiple requests concurrently
get 401/403; replace it with a reusable Promise-based guard (e.g., a single
redirectPromise variable and a getOrCreateRedirectPromise function) so only the
first failing request creates and runs the redirect flow and others await that
Promise; ensure the redirectPromise is cleared (set to null) on completion or
failure so future redirects can proceed, and update the response interceptor to
await the shared redirectPromise instead of checking the boolean.
| chrome.storage.local.remove(['token', 'email'], () => {}); | ||
|
|
||
| chrome.tabs.create({ url: onboardingUrl }, () => { | ||
| setTimeout(() => { | ||
| isRedirecting = false; | ||
| }, 2000); | ||
| }); |
There was a problem hiding this comment.
Chrome API 오류 처리 누락
chrome.storage.local.remove와 chrome.tabs.create 모두 실패할 수 있지만 오류 처리가 없습니다. 콜백에서 chrome.runtime.lastError를 확인하여 실패를 감지하고 로깅해야 합니다.
🔎 오류 처리를 추가한 수정안
- chrome.storage.local.remove(['token', 'email'], () => {});
+ chrome.storage.local.remove(['token', 'email'], () => {
+ if (chrome.runtime.lastError) {
+ console.error('Failed to clear storage:', chrome.runtime.lastError);
+ }
+ });
chrome.tabs.create({ url: onboardingUrl }, () => {
+ if (chrome.runtime.lastError) {
+ console.error('Failed to create onboarding tab:', chrome.runtime.lastError);
+ }
setTimeout(() => {
isRedirecting = false;
}, 2000);
});🤖 Prompt for AI Agents
In apps/extension/src/apis/axiosInstance.ts around lines 58-64, the
chrome.storage.local.remove and chrome.tabs.create callbacks don't check for
errors; update both callbacks to check chrome.runtime.lastError and log any
errors (e.g., console.error) so failures are visible, and ensure isRedirecting
is reset (or handled) even if an error occurs; for chrome.tabs.create also
handle the case where the tab callback isn't called by using defensive checks
and logging to aid debugging.
| if (!isRedirecting) { | ||
| isRedirecting = true; | ||
|
|
There was a problem hiding this comment.
flag처리로, 다른 api 흐름 제어한 부분 인상깊네요! 2초정도 적당한 것 같습니다!
| localStorage.removeItem('token'); | ||
| window.location.href = '/onboarding?step=SOCIAL_LOGIN'; | ||
|
|
|
✅ Storybook chromatic 배포 확인: |
jjangminii
left a comment
jjangminii
left a comment
There was a problem hiding this comment.
확실히 코드가 깔끔해졌네요 고생하셨습니다-! 주소 바뀐것만 확인해주세요
|
|
||
| const noAuthNeeded = [ | ||
| '/api/v1/auth/token', | ||
| '/api/v1/auth/signup', |
There was a problem hiding this comment.
/api/v1/auth/signup 이거 주소 /api/v2/auth/signup로 바뀌었어요
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
apps/extension/src/apis/axiosInstance.ts (3)
10-16: chrome.storage API 오류 처리 누락
getTokenFromStorage에서chrome.runtime.lastError확인이 없습니다. 스토리지 할당량 초과나 권한 문제 등으로 실패할 수 있으므로 오류 처리를 추가해야 합니다.🔎 오류 처리를 추가한 수정안
const getTokenFromStorage = () => { return new Promise<string | undefined>((resolve, reject) => { chrome.storage.local.get('token', (result) => { + if (chrome.runtime.lastError) { + reject(chrome.runtime.lastError); + return; + } resolve(result.token); }); }); };Based on past review comments.
34-34: 모듈 레벨 플래그의 동시성 제약
isRedirecting플래그는 여러 API 호출이 동시에 401/403을 반환할 때 race condition이 발생할 수 있습니다. Promise 기반 잠금 메커니즘을 사용하면 더 안전하게 단일 리다이렉트를 보장할 수 있습니다.🔎 Promise 기반 리다이렉트 가드 예시
-let isRedirecting = false; +let redirectPromise: Promise<void> | null = null; + +const redirectToOnboarding = async () => { + if (redirectPromise) { + return redirectPromise; + } + + redirectPromise = new Promise<void>((resolve) => { + chrome.storage.local.remove(['token', 'email'], () => { + if (chrome.runtime.lastError) { + console.error('Failed to clear storage:', chrome.runtime.lastError); + } + + chrome.tabs.create({ url: onboardingUrl }, () => { + if (chrome.runtime.lastError) { + console.error('Failed to create tab:', chrome.runtime.lastError); + } + setTimeout(() => { + redirectPromise = null; + resolve(); + }, 2000); + }); + }); + }); + + return redirectPromise; +};응답 인터셉터에서 사용:
await redirectToOnboarding();Based on past review comments.
55-65: Chrome API 오류 처리 추가 필요
chrome.storage.local.remove와chrome.tabs.create모두 실패할 수 있지만 오류 처리가 없습니다. 콜백에서chrome.runtime.lastError를 확인하여 실패를 감지하고 로깅해야 합니다.참고: 2초 timeout 설정은 적절해 보입니다.
🔎 오류 처리를 추가한 수정안
- chrome.storage.local.remove(['token', 'email'], () => {}); + chrome.storage.local.remove(['token', 'email'], () => { + if (chrome.runtime.lastError) { + console.error('Failed to clear storage:', chrome.runtime.lastError); + } + }); chrome.tabs.create({ url: onboardingUrl }, () => { + if (chrome.runtime.lastError) { + console.error('Failed to create onboarding tab:', chrome.runtime.lastError); + } setTimeout(() => { isRedirecting = false; }, 2000); });Based on past review comments.
🧹 Nitpick comments (1)
apps/extension/src/apis/axiosInstance.ts (1)
28-32: 환경변수 분리 권장TODO 주석에 언급된 대로, 온보딩 URL을 환경변수로 분리하면 설정 관리가 더 명확해집니다.
.env파일에VITE_ONBOARDING_URL을 추가하는 것을 고려해보세요.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/extension/src/apis/axiosInstance.ts
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: constantly-dev
Repo: Pinback-Team/pinback-client PR: 2
File: pnpm-workspace.yaml:3-3
Timestamp: 2025-08-18T13:48:59.065Z
Learning: constantly-dev는 docs 디렉터리를 컨벤션 문서 추가용으로 사용할 예정이라고 명시했습니다.
Learnt from: constantly-dev
Repo: Pinback-Team/pinback-client PR: 5
File: apps/extension/src/index.css:1-1
Timestamp: 2025-08-19T17:18:57.678Z
Learning: constantly-dev는 디자인 시스템 설정 PR 머지 후 `import 'pinback/tailwind-config/shared-styles.css';`를 `app.css`나 `index.css`에서 사용하여 공유 스타일을 관리할 계획입니다.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: storybook
🔇 Additional comments (2)
apps/extension/src/apis/axiosInstance.ts (2)
18-26: 요청 인터셉터 구현 확인토큰을 스토리지에서 비동기로 가져와 Authorization 헤더에 첨부하는 로직이 올바르게 구현되었습니다. 리프레시 로직 제거 후 단순화된 흐름이 명확합니다.
41-48: 인증 불필요 엔드포인트 목록 확인인증이 필요없는 엔드포인트 목록이 적절하게 확장되었습니다. 로그인 및 회원가입 관련 경로가 올바르게 포함되어 있습니다.
📌 Related Issues
📄 Tasks
⭐ PR Point (To Reviewer)
이제 refresh token API가 불필요해서 관련 로직 client와 extension 모두
interceptor에서 지웠어요.그리고 401, 403일때 extension에서 onboarding tab을 열게 되는데, 이때 여러개의 API에서 에러가 인증 관련 에러가 뜨는 경우 온보딩 탭이 여러개가 뜨는 에러가 날 수 있어요.
이를 방지하기 위해
isRedirectingflag를 추가해서 한 API로 tab이 열리는 동안에는 다른 API는 영향이 없도록 로직을 추가했어요.그리고 조금 더 안전하게
setTimeout으로 2초 뒤에 다시flag가false가 되도록 설정했어요. 더 짧게 해도 될 것 같은데 의견이 있다면 편하게 주세요!!📷 Screenshot
2025-12-22.12.03.37.mov
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.