Merged
Conversation
refactor: 로그아웃시 alert 노출 문제 해결
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthrough인증 토큰 처리 로직을 강화하여 인증 정보 없을 시 토큰 리프레시 시도를 방지하고, alert를 로그인 리다이렉트로 변경했습니다. 또한 미션 카드의 클릭 가능 상태에 ENDED를 추가하고, 모집 마감일 기반으로 지원 버튼을 비활성화하는 UI 로직을 추가했습니다. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Merged
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/api/client/axiosV2.ts (1)
197-211:⚠️ Potential issue | 🟠 Major
axios.ts와 동일한 버그: multipart 인터셉터에서axiosInstanceV2를 사용하여 재시도합니다.Line 206과 225에서
axiosInstanceForMultipartV2대신axiosInstanceV2를 사용하고 있습니다. 토큰 갱신 후 multipart 요청이 잘못된Content-Type으로 재시도됩니다.🐛 수정 제안
if (originalRequest) { originalRequest.headers.Authorization = `Bearer ${token}`; - return axiosInstanceV2(originalRequest); + return axiosInstanceForMultipartV2(originalRequest); }Line 225도 동일하게 수정:
if (originalRequest) { originalRequest.headers.Authorization = `Bearer ${newAccessToken}`; - return axiosInstanceV2(originalRequest); + return axiosInstanceForMultipartV2(originalRequest); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/client/axiosV2.ts` around lines 197 - 211, The multipart interceptor's retry path is using axiosInstanceV2 which causes retried multipart requests to lose the multipart Content-Type; in the isRefreshing branch where failedQueue resolves and in the retry call (currently invoking axiosInstanceV2(originalRequest)), change those retries to use axiosInstanceForMultipartV2 so multipart requests are retried with the proper instance and headers; locate the logic around isRefreshing, failedQueue, originalRequest and replace axiosInstanceV2 with axiosInstanceForMultipartV2 in that multipart interceptor.src/api/client/axios.ts (1)
197-211:⚠️ Potential issue | 🟠 MajorMultipart 인터셉터에서 재시도 시 JSON 인스턴스(
axiosInstance)를 사용하고 있습니다.Line 206과 225에서
axiosInstanceForMultipart대신axiosInstance를 사용하여 재시도하고 있습니다. 토큰 갱신 후 multipart 요청이 JSONContent-Type헤더로 재시도되어 서버에서 실패할 수 있습니다.수정 사항
// Line 206 (isRefreshing 대기열 처리) if (originalRequest) { originalRequest.headers.Authorization = `Bearer ${token}`; - return axiosInstance(originalRequest); + return axiosInstanceForMultipart(originalRequest); }// Line 225 (토큰 갱신 후 재시도) if (originalRequest) { originalRequest.headers.Authorization = `Bearer ${newAccessToken}`; - return axiosInstance(originalRequest); + return axiosInstanceForMultipart(originalRequest); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/client/axios.ts` around lines 197 - 211, The multipart interceptor is retrying requests using axiosInstance instead of the multipart-specific axiosInstanceForMultipart, causing retried requests to use JSON headers; modify the retry logic inside the isRefreshing branch (where failedQueue is pushed and in the then handler that sets originalRequest.headers.Authorization and calls axiosInstance(originalRequest)) to call axiosInstanceForMultipart(originalRequest) when the originalRequest corresponds to a multipart request (i.e., came from the multipart interceptor), ensuring the correct Content-Type and instance are used; preserve the same promise handling for failedQueue resolution/rejection and update any places that enqueue or resolve with the token to consistently use axiosInstanceForMultipart for multipart originalRequest objects.
🧹 Nitpick comments (3)
src/api/client/axios.ts (2)
62-66:refreshAccessToken실패 시 중복 리다이렉트 가능성.
refreshAccessToken내부(line 63)와 호출부(line 155, 161)에서 모두window.location.href = '/login'을 실행합니다.refreshAccessToken이 실패하면 내부에서 먼저 리다이렉트한 후null을 반환하고, 호출부에서도 다시 리다이렉트합니다. 브라우저에서 실질적 문제는 없지만,refreshAccessToken에서 리다이렉트를 제거하고 호출부에서만 처리하는 것이 책임 분리 측면에서 더 깔끔합니다.♻️ 제안: refreshAccessToken에서 리다이렉트 제거
} catch { - window.location.href = '/login'; - return null; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/client/axios.ts` around lines 62 - 66, The refreshAccessToken function currently performs a window.location.href = '/login' in its catch block and also returns null, causing duplicate redirects when callers (the places that call refreshAccessToken and then check for null at the call sites that currently also set window.location.href = '/login') handle the failure; remove the redirect from the catch in refreshAccessToken so it only returns null on failure, leaving the responsibility of redirecting to the callers that already perform window.location.href = '/login' after receiving null (update the catch in refreshAccessToken and verify the calling sites that check its return value still perform the redirect).
104-175: JSON 인터셉터와 multipart 인터셉터가 거의 동일한 코드를 중복하고 있습니다.두 인터셉터의 응답 에러 핸들링 로직이 재시도 인스턴스만 다르고 나머지는 완전히 동일합니다. 공통 로직을 팩토리 함수로 추출하면 유지보수성이 크게 향상됩니다. 위에서 지적한 multipart 인스턴스 버그도 이러한 중복에서 비롯된 것입니다.
♻️ 팩토리 함수 추출 제안
+const createResponseErrorInterceptor = (instance: typeof axiosInstance) => { + return async (error: unknown) => { + if ( + isAxiosError(error) && + error.response && + isApiError(error.response.data) + ) { + const errorResponseBody = error.response.data; + const originalRequest = error.config; + + if (errorResponseBody.errorCode === 'AUTH001') { + if (!hasAuthToken(originalRequest)) { + return Promise.reject(new ApiError(errorResponseBody)); + } + // ... shared refresh logic using `instance` for retries + } + return Promise.reject(new ApiError(errorResponseBody)); + } + return Promise.reject(error); + }; +}; + -axiosInstance.interceptors.response.use((config) => config, async (error) => { ... }); -axiosInstanceForMultipart.interceptors.response.use((config) => config, async (error) => { ... }); +axiosInstance.interceptors.response.use((config) => config, createResponseErrorInterceptor(axiosInstance)); +axiosInstanceForMultipart.interceptors.response.use((config) => config, createResponseErrorInterceptor(axiosInstanceForMultipart));Also applies to: 178-248
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/client/axios.ts` around lines 104 - 175, The response-error handling logic in the axiosInstance interceptor (symbols: axiosInstance, isAxiosError, isApiError, ApiError, refreshAccessToken, isRefreshing, failedQueue, processFailedQueue, hasAuthToken, originalRequest) is duplicated for the multipart instance; extract that logic into a reusable factory function (e.g., createResponseErrorInterceptor(retryInstance)) that accepts the retry axios instance to use for replaying original requests, implements the AUTH001 token-refresh flow including queueing via failedQueue/processFailedQueue and hasAuthToken checks, and returns the error handler to register with both axiosInstance and the multipart instance so both reuse the same implementation and avoid the multipart bug.src/api/client/axiosV2.ts (1)
90-102:hasAuthToken이axios.ts와axiosV2.ts에 동일하게 중복 정의되어 있습니다.두 파일에서 완전히 동일한 구현을 사용하고 있으므로, 공통 유틸리티(예:
src/api/client/auth-utils.ts)로 추출하여 import하는 것을 권장합니다. 단, V2 전환이 완료되면axios.ts자체가 제거될 예정이라면 현 상태로 유지해도 괜찮습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/client/axiosV2.ts` around lines 90 - 102, The hasAuthToken function is duplicated across axios.ts and axiosV2.ts; extract it into a shared utility (e.g., auth-utils) and import it from both modules: move the implementation that uses getCookie('accessToken') and inspects requestConfig?.headers?.Authorization (handling array and string forms) into a new exported function named hasAuthToken in the shared auth-utils module, replace the inline definitions in both files with imports of that function, and keep the same signature (accepting an InternalAxiosRequestConfig-like param) to avoid breaking call sites.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/card/mission-card.tsx`:
- Around line 90-96: Update the outdated inline comments near the
isCardClickable logic: change the comment that currently says "비리더: 진행중/평가완료" to
"비리더: 진행중/제출마감/평가완료" and change the comment that currently says "비리더: 진행예정/제출마감"
to "비리더: 진행예정" so they reflect the new branch behavior in isCardClickable (the
non-leader branch now includes ENDED as clickable); locate comments adjacent to
the isCardClickable function and replace the two phrases accordingly.
---
Outside diff comments:
In `@src/api/client/axios.ts`:
- Around line 197-211: The multipart interceptor is retrying requests using
axiosInstance instead of the multipart-specific axiosInstanceForMultipart,
causing retried requests to use JSON headers; modify the retry logic inside the
isRefreshing branch (where failedQueue is pushed and in the then handler that
sets originalRequest.headers.Authorization and calls
axiosInstance(originalRequest)) to call
axiosInstanceForMultipart(originalRequest) when the originalRequest corresponds
to a multipart request (i.e., came from the multipart interceptor), ensuring the
correct Content-Type and instance are used; preserve the same promise handling
for failedQueue resolution/rejection and update any places that enqueue or
resolve with the token to consistently use axiosInstanceForMultipart for
multipart originalRequest objects.
In `@src/api/client/axiosV2.ts`:
- Around line 197-211: The multipart interceptor's retry path is using
axiosInstanceV2 which causes retried multipart requests to lose the multipart
Content-Type; in the isRefreshing branch where failedQueue resolves and in the
retry call (currently invoking axiosInstanceV2(originalRequest)), change those
retries to use axiosInstanceForMultipartV2 so multipart requests are retried
with the proper instance and headers; locate the logic around isRefreshing,
failedQueue, originalRequest and replace axiosInstanceV2 with
axiosInstanceForMultipartV2 in that multipart interceptor.
---
Duplicate comments:
In `@src/api/client/axiosV2.ts`:
- Around line 104-175: This file duplicates the JSON/multipart axios interceptor
logic found in axios.ts; refactor by extracting the shared retry-on-AUTH001
interceptor into a reusable factory (e.g., createAuthRefreshInterceptor) and
apply it to axiosInstanceV2 via axiosInstanceV2.interceptors.response.use by
reusing the same implementation used for axios.ts; specifically move the logic
that checks isAxiosError, error.response, isApiError, AUTH001 handling
(references: axiosInstanceV2.interceptors.response.use, isRefreshing,
failedQueue, processFailedQueue, refreshAccessToken, hasAuthToken, ApiError,
originalRequest) into the factory so both axios.ts and axiosV2.ts can call it
and eliminate the duplicated code paths (ensure the factory returns the success
and failure handlers and preserves token queueing and redirect behavior).
- Around line 62-66: The refreshAccessToken function currently performs a
window.location.href redirect inside its catch block, duplicating redirect logic
already handled by the caller; remove the redirect from refreshAccessToken (keep
returning null or throwing as before) so that only the caller (the interceptor
or code that invokes refreshAccessToken) is responsible for navigating to
'/login'; update refreshAccessToken's catch to log/propagate the error and
return null, and ensure callers check for null and perform the single redirect
there (reference: refreshAccessToken and the axios V2 request/response
interceptor that invokes it).
---
Nitpick comments:
In `@src/api/client/axios.ts`:
- Around line 62-66: The refreshAccessToken function currently performs a
window.location.href = '/login' in its catch block and also returns null,
causing duplicate redirects when callers (the places that call
refreshAccessToken and then check for null at the call sites that currently also
set window.location.href = '/login') handle the failure; remove the redirect
from the catch in refreshAccessToken so it only returns null on failure, leaving
the responsibility of redirecting to the callers that already perform
window.location.href = '/login' after receiving null (update the catch in
refreshAccessToken and verify the calling sites that check its return value
still perform the redirect).
- Around line 104-175: The response-error handling logic in the axiosInstance
interceptor (symbols: axiosInstance, isAxiosError, isApiError, ApiError,
refreshAccessToken, isRefreshing, failedQueue, processFailedQueue, hasAuthToken,
originalRequest) is duplicated for the multipart instance; extract that logic
into a reusable factory function (e.g.,
createResponseErrorInterceptor(retryInstance)) that accepts the retry axios
instance to use for replaying original requests, implements the AUTH001
token-refresh flow including queueing via failedQueue/processFailedQueue and
hasAuthToken checks, and returns the error handler to register with both
axiosInstance and the multipart instance so both reuse the same implementation
and avoid the multipart bug.
In `@src/api/client/axiosV2.ts`:
- Around line 90-102: The hasAuthToken function is duplicated across axios.ts
and axiosV2.ts; extract it into a shared utility (e.g., auth-utils) and import
it from both modules: move the implementation that uses getCookie('accessToken')
and inspects requestConfig?.headers?.Authorization (handling array and string
forms) into a new exported function named hasAuthToken in the shared auth-utils
module, replace the inline definitions in both files with imports of that
function, and keep the same signature (accepting an
InternalAxiosRequestConfig-like param) to avoid breaking call sites.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🌱 연관된 이슈
리더가 아닌 경우 종료된 스터디 내 미션 목록에서 미션 상세로 접근이 불가능
☘️ 작업 내용
mission-card.tsx에서 비리더인 경우 status === 'ENDED' 인 조건 추가🍀 참고사항
스크린샷 (선택)
Summary by CodeRabbit
릴리스 노트
버그 수정
새로운 기능