Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Walkthrough클라이언트와 익스텐션의 인증 흐름을 단순화하고, 로그인/회원가입 콜백에서 토큰을 브라우저 확장 저장소(chrome.storage.local)에 추가로 저장하도록 변경했습니다. 또한 클라이언트·익스텐션의 axios 인터셉터에서 토큰 재발급 로직을 제거하고 리다이렉트 흐름을 간소화했습니다. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 chromatic 배포 확인: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
apps/client/src/pages/onBoarding/GoogleCallback.tsx (1)
30-34: Chrome storage 저장 완료 전에 페이지 이동이 발생합니다.토큰이 Chrome storage에 저장되기 전에
navigate('/')가 실행될 수 있습니다. 온보딩 과정에서의 초기 토큰 설정이므로, 이 시점에서 저장이 완료되지 않으면 이후 페이지에서 Chrome storage의 토큰을 사용할 수 없을 수 있습니다.이는
axiosInstance.ts의 lines 24-28에서 플래그한 것과 동일한 패턴입니다.🔎 비동기 저장 완료 후 이동하도록 수정 제안
if (accessToken) { localStorage.setItem('token', accessToken); if (typeof chrome !== 'undefined' && chrome.storage?.local) { - chrome.storage.local.set({ token: accessToken }, () => { - console.log('Token saved to chrome storage'); - }); + try { + await chrome.storage.local.set({ token: accessToken }); + console.log('Token saved to chrome storage'); + } catch (error) { + console.error('Chrome storage 저장 실패:', error); + } } }apps/client/src/shared/apis/queries.ts (1)
83-87: Chrome storage 저장이 완료되기 전에 다음 작업이 실행됩니다.
chrome.storage.local.set이 완료되기 전에sendTokenToExtension이 호출됩니다. 이는axiosInstance.tslines 24-28에서 플래그한 것과 동일한 race condition 패턴입니다.중앙화된 토큰 저장 유틸리티 함수를 만들어 세 곳 모두에서 일관되게 사용하는 것을 권장합니다.
📜 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 (4)
apps/client/package.jsonapps/client/src/pages/onBoarding/GoogleCallback.tsxapps/client/src/shared/apis/queries.tsapps/client/src/shared/apis/setting/axiosInstance.ts
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: constantly-dev
Repo: Pinback-Team/pinback-client PR: 75
File: apps/extension/src/apis/axiosInstance.ts:30-34
Timestamp: 2025-09-11T11:48:10.615Z
Learning: Pinback 프로젝트에서는 사용자 이메일 저장 시 'email' 키를 사용하도록 통일했습니다 (localStorage 및 chrome.storage.local 모두).
⏰ 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 (1)
apps/client/package.json (1)
32-32: LGTM! Chrome API 타입 정의 추가가 적절합니다.Chrome storage API 사용을 위한 타입 정의가 devDependency로 올바르게 추가되었습니다.
| if (typeof chrome !== 'undefined' && chrome.storage?.local) { | ||
| chrome.storage.local.set({ token: newToken }, () => { | ||
| console.log('Token saved to chrome storage'); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Chrome storage 저장이 완료되기 전에 토큰이 반환될 수 있습니다.
chrome.storage.local.set은 비동기 작업이지만 대기하지 않고 즉시 토큰을 반환합니다. 이로 인해 Chrome storage에 저장이 완료되기 전에 토큰이 사용될 수 있으며, 저장 실패 시에도 감지할 수 없습니다.
동일한 패턴이 다음 파일에도 적용됩니다:
GoogleCallback.tsx(lines 30-34)queries.ts(lines 83-87)
🔎 Promise 기반으로 변경하는 제안
if (newToken) {
localStorage.setItem('token', newToken);
if (typeof chrome !== 'undefined' && chrome.storage?.local) {
- chrome.storage.local.set({ token: newToken }, () => {
- console.log('Token saved to chrome storage');
- });
+ try {
+ await chrome.storage.local.set({ token: newToken });
+ console.log('Token saved to chrome storage');
+ } catch (error) {
+ console.error('Chrome storage 저장 실패:', error);
+ }
}
return newToken;
}또는 중앙화된 유틸리티 함수를 만드는 것을 권장합니다:
// shared/utils/storage.ts
export const saveToken = async (token: string): Promise<void> => {
localStorage.setItem('token', token);
if (typeof chrome !== 'undefined' && chrome.storage?.local) {
try {
await chrome.storage.local.set({ token });
console.log('Token saved to chrome storage');
} catch (error) {
console.error('Chrome storage 저장 실패:', error);
// localStorage는 성공했으므로 계속 진행
}
}
};Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/client/src/shared/apis/setting/axiosInstance.ts around lines 24-28 (and
similarly in GoogleCallback.tsx lines 30-34 and queries.ts lines 83-87), the
chrome.storage.local.set call is asynchronous but the code returns the token
immediately; change the implementation to an async/promise-based flow so callers
await storage completion and detect failures: create a centralized utility
(e.g., shared/utils/storage.ts) that writes to localStorage synchronously, wraps
chrome.storage.local.set in a Promise (using the callback or chrome.storage API
that returns a Promise), awaits it, logs success, and catches/logs errors
without blocking localStorage success; update these three files to import and
await that saveToken(token) before returning/using the token.
jjangminii
left a comment
jjangminii
left a comment
There was a problem hiding this comment.
로그인할 때 따로 익스텐션도 설정해야하는지 까먹고있었는데 추가해주셔서 감사합니당..
| if (isUser) { | ||
| if (accessToken) { | ||
| localStorage.setItem('token', accessToken); | ||
|
|
There was a problem hiding this comment.
크론도 따로 추가해줘야하는지 몰랐어요..반성하겠습니다
constantly-dev
left a comment
constantly-dev
left a comment
There was a problem hiding this comment.
수고하셨습니다! 간단한 코멘트만 확인부탁드려요~~
| if (typeof chrome !== 'undefined' && chrome.storage?.local) { | ||
| chrome.storage.local.set({ token: accessToken }, () => { | ||
| console.log('Token saved to chrome storage'); | ||
| }); | ||
| } |
There was a problem hiding this comment.
크게 console이 필요없다면 지워도 될 것 같아요!
그리고 추가로 typeof chrome !== 'undefined' && chrome.storage?.local이 조건은 어떤 조건인가요??
이름을 붙이는 것도 좋을 것 같아요!
| if (newToken) { | ||
| localStorage.setItem('token', newToken); | ||
| if (typeof chrome !== 'undefined' && chrome.storage?.local) { | ||
| chrome.storage.local.set({ token: newToken }, () => { | ||
| console.log('Token saved to chrome storage'); | ||
| }); | ||
| } |
There was a problem hiding this comment.
localStorage set 하는 곳에서 chrome storage도 다 같이 해줘야 하니, 애초에 이 두개를 같이 하는 util을 추상화 시켜서 사용하면 좋을 것 같아요.
스프린트 작업 대충 끝나면 Storage를 class형태나 util로 빼서 재사용 하도록 만드는 것이 좋을 것 같은데 어떻게 생각하시나요?
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
apps/extension/src/apis/axiosInstance.ts (1)
28-32: 환경변수 분리에 대한 TODO가 확인됩니다.하드코딩된 URL을 환경변수로 분리하는 것이 좋겠지만, TODO 주석으로 이미 인지하고 있으므로 향후 개선 사항으로 처리 가능합니다.
📜 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
⏰ 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 (3)
apps/client/src/shared/apis/setting/axiosInstance.ts (1)
12-20: 요청 인터셉터 단순화가 적절합니다.토큰 재발급 로직을 제거하고 localStorage에서 토큰을 읽어 Bearer 토큰으로 첨부하는 표준 패턴으로 단순화되었습니다. 구현이 올바르고 명확합니다.
apps/extension/src/apis/axiosInstance.ts (2)
18-26: 요청 인터셉터 로직이 올바르게 구현되었습니다.토큰이 존재할 때만 Authorization 헤더를 추가하는 조건부 로직이 적절합니다.
getTokenFromStorage의 에러 핸들링이 개선되면 더욱 안정적으로 동작할 것입니다.
41-48: 인증 불필요 경로 목록에 Google OAuth 엔드포인트가 올바르게 추가되었습니다.
/api/v2/auth/google경로를noAuthNeeded배열에 추가한 것은 Google 소셜 로그인 플로우에 적합합니다.
| localStorage.removeItem('token'); | ||
| window.location.href = '/onboarding?step=SOCIAL_LOGIN'; |
There was a problem hiding this comment.
Chrome storage도 함께 정리해야 합니다.
이 PR은 온보딩 시 토큰을 localStorage와 chrome.storage 양쪽에 저장하도록 변경합니다. 하지만 인증 오류(401/403) 발생 시 localStorage만 삭제하고 chrome.storage는 정리하지 않아 두 저장소 간 불일치가 발생합니다.
🔎 chrome.storage도 정리하는 수정안
originalRequest._retry = true;
localStorage.removeItem('token');
+ if (typeof chrome !== 'undefined' && chrome.storage?.local) {
+ chrome.storage.local.remove('token', () => {
+ console.log('Token removed from chrome storage');
+ });
+ }
window.location.href = '/onboarding?step=SOCIAL_LOGIN';
return Promise.reject(error);또는 이전 리뷰 코멘트에서 제안된 중앙화된 유틸리티 함수를 사용하는 것을 권장합니다:
// shared/utils/storage.ts에 추가
export const removeToken = async (): Promise<void> => {
localStorage.removeItem('token');
if (typeof chrome !== 'undefined' && chrome.storage?.local) {
try {
await chrome.storage.local.remove('token');
console.log('Token removed from chrome storage');
} catch (error) {
console.error('Chrome storage 삭제 실패:', error);
}
}
};그 다음 이 파일에서:
originalRequest._retry = true;
- localStorage.removeItem('token');
+ await removeToken();
window.location.href = '/onboarding?step=SOCIAL_LOGIN';
return Promise.reject(error);📝 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.
| localStorage.removeItem('token'); | |
| window.location.href = '/onboarding?step=SOCIAL_LOGIN'; | |
| localStorage.removeItem('token'); | |
| if (typeof chrome !== 'undefined' && chrome.storage?.local) { | |
| chrome.storage.local.remove('token', () => { | |
| console.log('Token removed from chrome storage'); | |
| }); | |
| } | |
| window.location.href = '/onboarding?step=SOCIAL_LOGIN'; |
🤖 Prompt for AI Agents
In apps/client/src/shared/apis/setting/axiosInstance.ts around lines 46-47, the
code only clears localStorage on auth errors but leaves chrome.storage.token
intact; update the error handling to also remove the token from chrome.storage
(or call the centralized utility shared/utils/storage.removeToken if available)
and await its completion before redirecting to '/onboarding?step=SOCIAL_LOGIN';
ensure you guard for typeof chrome !== 'undefined' and chrome.storage?.local and
catch/log any chrome.storage errors so redirect still occurs even if chrome
removal fails.
| const getTokenFromStorage = () => { | ||
| return new Promise<string | undefined>((resolve) => { | ||
| chrome.storage.local.get('token', (result) => { | ||
| resolve(result.token); | ||
| }); | ||
| }); | ||
| }; |
There was a problem hiding this comment.
런타임 안전성 검사 및 에러 핸들링이 누락되었습니다.
PR objectives에서 명시한 대로 chrome API 호출 전에 TypeError 방지를 위한 조건문이 필요합니다. 또한 chrome.storage.local.get 실패 시 에러 처리가 없어 Promise가 영원히 pending 상태가 될 수 있습니다.
🔎 제안하는 수정안
const getTokenFromStorage = () => {
return new Promise<string | undefined>((resolve, reject) => {
+ if (typeof chrome === 'undefined' || !chrome.storage?.local) {
+ resolve(undefined);
+ return;
+ }
+
- chrome.storage.local.get('token', (result) => {
+ 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-16,
getTokenFromStorage lacks runtime checks for the chrome API and has no error
handling which can cause TypeError or a permanently pending Promise; update the
function to first check that window.chrome and chrome.storage and
chrome.storage.local exist (return undefined if absent), wrap
chrome.storage.local.get in a try/catch or provide the optional error callback
to handle failures and ensure the Promise always resolves or rejects (resolve
undefined on error), and consider adding a short timeout fallback so the Promise
cannot remain pending indefinitely.
| ? '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 플래그는 여러 요청이 동시에 401/403을 받을 경우 제대로 동작하지 않을 수 있습니다. 또한 고정된 2초 타임아웃에 의존하여 플래그를 리셋하는 것은 탭 생성이 실패하거나 지연될 경우 문제가 될 수 있습니다.
💡 개선 제안
-let isRedirecting = false;
+let redirectPromise: Promise<void> | null = null;그리고 응답 인터셉터에서:
- if (!isRedirecting) {
- isRedirecting = true;
+ if (!redirectPromise) {
+ redirectPromise = new Promise((resolve) => {
+ chrome.storage.local.remove(['token', 'email'], () => {});
- chrome.storage.local.remove(['token', 'email'], () => {});
-
- chrome.tabs.create({ url: onboardingUrl }, () => {
- setTimeout(() => {
- isRedirecting = false;
- }, 2000);
- });
+ chrome.tabs.create({ url: onboardingUrl }, () => {
+ redirectPromise = null;
+ resolve();
+ });
+ });
}Committable suggestion skipped: line range outside the PR's diff.
| if (!isRedirecting) { | ||
| isRedirecting = true; | ||
|
|
||
| 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 호출에서 실패 케이스를 처리하지 않고 있으며, PR objectives에서 명시한 런타임 안전성 검사도 없습니다.
🔎 제안하는 수정안
if (!isRedirecting) {
isRedirecting = true;
- chrome.storage.local.remove(['token', 'email'], () => {});
+ if (typeof chrome !== 'undefined' && chrome.storage?.local) {
+ chrome.storage.local.remove(['token', 'email'], () => {
+ if (chrome.runtime.lastError) {
+ console.error('토큰 제거 실패:', chrome.runtime.lastError);
+ }
+ });
+ }
- chrome.tabs.create({ url: onboardingUrl }, () => {
- setTimeout(() => {
- isRedirecting = false;
- }, 2000);
- });
+ if (typeof chrome !== 'undefined' && chrome.tabs?.create) {
+ chrome.tabs.create({ url: onboardingUrl }, () => {
+ if (chrome.runtime.lastError) {
+ console.error('탭 생성 실패:', chrome.runtime.lastError);
+ }
+ setTimeout(() => {
+ isRedirecting = false;
+ }, 2000);
+ });
+ } else {
+ isRedirecting = false;
+ }
}📝 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.
| if (!isRedirecting) { | |
| isRedirecting = true; | |
| chrome.storage.local.remove(['token', 'email'], () => {}); | |
| chrome.tabs.create({ url: onboardingUrl }, () => { | |
| setTimeout(() => { | |
| isRedirecting = false; | |
| }, 2000); | |
| }); | |
| } | |
| if (!isRedirecting) { | |
| isRedirecting = true; | |
| if (typeof chrome !== 'undefined' && chrome.storage?.local) { | |
| chrome.storage.local.remove(['token', 'email'], () => { | |
| if (chrome.runtime.lastError) { | |
| console.error('토큰 제거 실패:', chrome.runtime.lastError); | |
| } | |
| }); | |
| } | |
| if (typeof chrome !== 'undefined' && chrome.tabs?.create) { | |
| chrome.tabs.create({ url: onboardingUrl }, () => { | |
| if (chrome.runtime.lastError) { | |
| console.error('탭 생성 실패:', chrome.runtime.lastError); | |
| } | |
| setTimeout(() => { | |
| isRedirecting = false; | |
| }, 2000); | |
| }); | |
| } else { | |
| isRedirecting = false; | |
| } | |
| } |
🤖 Prompt for AI Agents
In apps/extension/src/apis/axiosInstance.ts around lines 55-65, the chrome API
calls lack runtime existence checks and error handling; guard that chrome,
chrome.storage?.local and chrome.tabs exist before calling, check
chrome.runtime?.lastError inside each callback, and handle failures (log/report
and ensure isRedirecting is reset on error); also propagate or handle callback
errors instead of ignoring empty callbacks and ensure setTimeout still clears
isRedirecting if chrome.tabs.create fails.
3 participants
📌 Related Issues
📄 Tasks
⭐ PR Point (To Reviewer)
TypeError 방지 환경 조건 문 추가
📷 Screenshot
Summary by CodeRabbit
새로운 기능
개선사항
잡무(Chore)
✏️ Tip: You can customize this high-level summary in your review settings.