Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
Walkthrough404 및 5xx 전용 페이지와 공통 레이아웃 추가, 서버-에러 감지/리다이렉트 유틸 도입, API 클라이언트의 네트워크·서버 에러 표준화, Auth 초기화 스킵 로직 및 react-query 재시도 정책 도입으로 전반적인 에러 처리 흐름을 확장했습니다. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/auth/AuthGuard.tsx (1)
20-24:⚠️ Potential issue | 🟡 Minor로딩 스피너 접근성 속성을 추가해 주세요.
Line 20~24의 로딩 UI는 시각적으로만 표현되어 스크린리더가 상태 변화를 인지하기 어렵습니다.
role="status"와 보조 텍스트를 넣어 주세요.♿ 제안 코드
- <div className="flex h-screen items-center justify-center"> - <div className="h-8 w-8 animate-spin rounded-full border-4 border-indigo-200 border-t-indigo-600" /> + <div className="flex h-screen items-center justify-center" role="status" aria-live="polite"> + <span className="sr-only">로딩 중</span> + <div + aria-hidden="true" + className="h-8 w-8 animate-spin rounded-full border-4 border-indigo-200 border-t-indigo-600" + /> </div>As per coding guidelines,
src/components/**:접근성(aria-*, role, 키보드 탐색)이 적절히 처리되는지.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/auth/AuthGuard.tsx` around lines 20 - 24, The loading UI returned from the AuthGuard component when isLoading && !shouldSkipInitialize is true is currently only visual; update that JSX to be accessible by adding role="status" on the container (the div wrapping the spinner) and include an offscreen/visually-hidden assistive text (e.g., a <span> with a "sr-only" class or aria-live text) such as "Loading…" so screen readers announce the state; locate the conditional return in AuthGuard (the block that renders the div with className="flex h-screen items-center justify-center" and the spinner element with className including "animate-spin") and add the role and hidden text accordingly.
🧹 Nitpick comments (5)
src/utils/ts/errorRedirect.ts (1)
1-1: 경로 상수를 export 해서 전역 재사용해 주세요.Line 1의
SERVER_ERROR_PATH를 export하면 라우트/가드/리다이렉트 간 문자열 중복을 없앨 수 있습니다.♻️ 제안 코드
-const SERVER_ERROR_PATH = '/server-error'; +export const SERVER_ERROR_PATH = '/server-error';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/ts/errorRedirect.ts` at line 1, Export the SERVER_ERROR_PATH constant so it can be reused globally: change the declaration in errorRedirect.ts to an exported constant (e.g., export const SERVER_ERROR_PATH = '/server-error') and then update any places that use the literal string (routes, guards, redirects) to import SERVER_ERROR_PATH from this module to remove duplicated string literals and centralize the path.src/pages/NotFound/index.tsx (1)
5-37: 에러 페이지 UI를 공통 컴포넌트로 추출해 주세요.
src/pages/ServerError/index.tsx와 레이아웃/버튼 스타일이 거의 동일합니다. 공통ErrorPageLayout로 추출하면 유지보수가 쉬워집니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/NotFound/index.tsx` around lines 5 - 37, Create a reusable ErrorPageLayout component and replace duplicate UI in NotFoundPage and ServerError pages: extract the shared JSX (container section, image, title, message block, and primary button) from NotFoundPage into a new ErrorPageLayout component that accepts props like imageSrc (NotFoundCatImage), title ("오류가 발생했어요"), message (multi-line description), primaryLabel ("홈으로 가기"), and onPrimaryClick (handleGoHome / server error handler); then update NotFoundPage (function NotFoundPage) and ServerError page to import and render <ErrorPageLayout .../> passing the appropriate props and move navigation logic (useNavigate + useAuthStore) into the pages so ErrorPageLayout is presentation-only.src/apis/client.ts (1)
152-175: 오류 처리 분기를 헬퍼로 추출해 중복을 줄여 주세요.
sendRequest와sendRequestWithoutRetry의 서버에러 리다이렉트/네트워크 에러 매핑 로직이 동일하게 반복됩니다. 공통 함수로 묶어두면 정책 변경 시 누락 위험이 줄어듭니다.Also applies to: 268-291
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/apis/client.ts` around lines 152 - 175, Extract the duplicated error-handling branches from sendRequest and sendRequestWithoutRetry into a single helper (e.g., handleApiFetchError or mapFetchError) that accepts the Response (or thrown error) plus the request url, and performs: 1) the server-error check via isServerErrorStatus and calls redirectToServerErrorPage(); 2) parsing the response with parseErrorResponse and constructing/throwing the ApiError (setting status, statusText, url, apiError) when response.ok is false; and 3) mapping network errors using isFetchNetworkError and createNetworkApiError (and handling AbortError). Replace the duplicated blocks in sendRequest and sendRequestWithoutRetry to call this helper (use parseResponse only on success).src/components/auth/AuthGuard.tsx (1)
12-12: 서버 에러 경로 문자열을 상수로 통일해 주세요.Line 12의
'/server-error'하드코딩은 라우트/리다이렉트 경로와 드리프트가 생기기 쉽습니다.errorRedirect.ts의 경로 상수를 재사용하는 쪽이 안전합니다.♻️ 제안 코드
+import { SERVER_ERROR_PATH } from '@/utils/ts/errorRedirect'; ... - const shouldSkipInitialize = pathname === '/server-error'; + const shouldSkipInitialize = pathname === SERVER_ERROR_PATH;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/auth/AuthGuard.tsx` at line 12, Replace the hardcoded '/server-error' string in the shouldSkipInitialize assignment with the shared route constant exported from errorRedirect.ts (import the exported constant and use it instead of the literal), update the AuthGuard.tsx import list to bring in that constant, and ensure shouldSkipInitialize = pathname === <ERROR_ROUTE_CONSTANT> so the component reuses the canonical server-error route definition.src/apis/auth/index.ts (1)
14-20: 토큰 갱신 실패 에러도 표준 API 에러 형태로 맞춰 주세요.Line 19에서 일반
Error를 던지면, 다른 API 레이어와 달리 상태코드 기반 처리 일관성이 깨집니다.ApiError형태로 매핑해 전달하는 편이 안전합니다.♻️ 제안 코드
+import type { ApiError } from '@/interface/error'; ... - throw new Error('토큰 갱신 실패'); + const error = new Error('토큰 갱신 실패') as ApiError; + error.status = response.status; + error.statusText = response.statusText; + error.url = response.url; + throw error;As per coding guidelines,
src/apis/**:에러가 UI 친화적인 형태로 매핑/전달되는지및공통 API 클라이언트 설정과 일관된지.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/apis/auth/index.ts` around lines 14 - 20, Replace the plain Error thrown on token refresh failure with the standard ApiError so the API layer remains consistent: inside the block that checks if (!response.ok) (the same scope using isServerErrorStatus(response.status) and redirectToServerErrorPage()), construct and throw an ApiError instance that includes the HTTP status code and a UI-friendly message (e.g., new ApiError(response.status, '토큰 갱신 실패')) and preserve any response body/error details if available so other layers can handle it uniformly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/components/auth/AuthGuard.tsx`:
- Around line 20-24: The loading UI returned from the AuthGuard component when
isLoading && !shouldSkipInitialize is true is currently only visual; update that
JSX to be accessible by adding role="status" on the container (the div wrapping
the spinner) and include an offscreen/visually-hidden assistive text (e.g., a
<span> with a "sr-only" class or aria-live text) such as "Loading…" so screen
readers announce the state; locate the conditional return in AuthGuard (the
block that renders the div with className="flex h-screen items-center
justify-center" and the spinner element with className including "animate-spin")
and add the role and hidden text accordingly.
---
Nitpick comments:
In `@src/apis/auth/index.ts`:
- Around line 14-20: Replace the plain Error thrown on token refresh failure
with the standard ApiError so the API layer remains consistent: inside the block
that checks if (!response.ok) (the same scope using
isServerErrorStatus(response.status) and redirectToServerErrorPage()), construct
and throw an ApiError instance that includes the HTTP status code and a
UI-friendly message (e.g., new ApiError(response.status, '토큰 갱신 실패')) and
preserve any response body/error details if available so other layers can handle
it uniformly.
In `@src/apis/client.ts`:
- Around line 152-175: Extract the duplicated error-handling branches from
sendRequest and sendRequestWithoutRetry into a single helper (e.g.,
handleApiFetchError or mapFetchError) that accepts the Response (or thrown
error) plus the request url, and performs: 1) the server-error check via
isServerErrorStatus and calls redirectToServerErrorPage(); 2) parsing the
response with parseErrorResponse and constructing/throwing the ApiError (setting
status, statusText, url, apiError) when response.ok is false; and 3) mapping
network errors using isFetchNetworkError and createNetworkApiError (and handling
AbortError). Replace the duplicated blocks in sendRequest and
sendRequestWithoutRetry to call this helper (use parseResponse only on success).
In `@src/components/auth/AuthGuard.tsx`:
- Line 12: Replace the hardcoded '/server-error' string in the
shouldSkipInitialize assignment with the shared route constant exported from
errorRedirect.ts (import the exported constant and use it instead of the
literal), update the AuthGuard.tsx import list to bring in that constant, and
ensure shouldSkipInitialize = pathname === <ERROR_ROUTE_CONSTANT> so the
component reuses the canonical server-error route definition.
In `@src/pages/NotFound/index.tsx`:
- Around line 5-37: Create a reusable ErrorPageLayout component and replace
duplicate UI in NotFoundPage and ServerError pages: extract the shared JSX
(container section, image, title, message block, and primary button) from
NotFoundPage into a new ErrorPageLayout component that accepts props like
imageSrc (NotFoundCatImage), title ("오류가 발생했어요"), message (multi-line
description), primaryLabel ("홈으로 가기"), and onPrimaryClick (handleGoHome / server
error handler); then update NotFoundPage (function NotFoundPage) and ServerError
page to import and render <ErrorPageLayout .../> passing the appropriate props
and move navigation logic (useNavigate + useAuthStore) into the pages so
ErrorPageLayout is presentation-only.
In `@src/utils/ts/errorRedirect.ts`:
- Line 1: Export the SERVER_ERROR_PATH constant so it can be reused globally:
change the declaration in errorRedirect.ts to an exported constant (e.g., export
const SERVER_ERROR_PATH = '/server-error') and then update any places that use
the literal string (routes, guards, redirects) to import SERVER_ERROR_PATH from
this module to remove duplicated string literals and centralize the path.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/assets/image/not-found-cat.webpis excluded by!src/assets/**and included by**
📒 Files selected for processing (8)
src/App.tsxsrc/apis/auth/index.tssrc/apis/client.tssrc/components/auth/AuthGuard.tsxsrc/main.tsxsrc/pages/NotFound/index.tsxsrc/pages/ServerError/index.tsxsrc/utils/ts/errorRedirect.ts
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/apis/auth/index.ts (1)
8-24:⚠️ Potential issue | 🟠 Major
refreshAccessToken는 네트워크 오류 처리가 부재하고 공통 API 패턴과 일관되지 않습니다.
fetch네트워크 실패 시try/catch로 잡지 않아ApiError표준화가 누락됩니다. 같은 파일의signup,getMyInfo,logout처럼apiClient.post를 사용하거나, 불가피하게 직접fetch를 쓸 경우apiClient의try/catch+rethrowFetchError패턴을 따라 네트워크 오류를 일관되게 처리해야 합니다.수정 예시
-export const refreshAccessToken = async (): Promise<string> => { - const url = `${BASE_URL.replace(/\/+$/, '')}/users/refresh`; - const response = await fetch(url, { - method: 'POST', - credentials: 'include', - }); +export const refreshAccessToken = async (): Promise<string> => { + const response = await apiClient.post<RefreshTokenResponse>('users/refresh', { + requiresAuth: false, + }); - if (!response.ok) { - if (isServerErrorStatus(response.status)) { - redirectToServerErrorPage(); - } - - const error = new Error('토큰 갱신 실패') as ApiError; - error.status = response.status; - error.statusText = response.statusText; - error.url = url; - throw error; - } - - const data: RefreshTokenResponse = await response.json(); - return data.accessToken; + return response.accessToken;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/apis/auth/index.ts` around lines 8 - 24, refreshAccessToken currently calls fetch directly without handling network errors or following the module's ApiError normalization; either replace the direct fetch with the shared apiClient.post call used by signup/getMyInfo/logout or wrap the existing fetch in a try/catch and call the shared rethrowFetchError logic so network failures are converted into ApiError consistently: update refreshAccessToken to use apiClient.post('/users/refresh', { credentials: 'include' }) or surround the fetch(...) call with try { ... } catch (err) { rethrowFetchError(err, 'refreshAccessToken', url) } and ensure any thrown ApiError has status, statusText and url set as done elsewhere.
🧹 Nitpick comments (2)
src/apis/client.ts (1)
59-62: API 레이어에서 즉시 리다이렉트는 분리 권장합니다.
throwApiError에서 네비게이션을 직접 수행하면 API 계층에 UI 책임이 섞입니다. 여기서는 표준화된 에러만 던지고, 이동은 상위(라우터/에러 바운더리)에서status기반으로 처리하는 편이 안전합니다.♻️ 제안 diff
async function throwApiError(response: Response): Promise<never> { - if (isServerErrorStatus(response.status)) { - redirectToServerErrorPage(); - } - const errorData = await parseErrorResponse(response);As per coding guidelines
src/apis/**: 비즈니스 로직이나 React 훅 의존성이 섞이지 않았는지.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/apis/client.ts` around lines 59 - 62, The function throwApiError currently performs UI navigation (calls redirectToServerErrorPage) which mixes API layer with UI; change throwApiError to only create and throw a standardized error object (include response.status and any parsed body/details) and remove the redirectToServerErrorPage call from it, and let higher-level code (router/error boundary or the caller) inspect isServerErrorStatus(response.status) and perform navigation or handling; update any callers of throwApiError to catch the thrown error and perform redirect when appropriate.src/pages/ServerError/index.tsx (1)
6-12: 홈 이동 비즈니스 로직은 커스텀 훅으로 분리해 주세요.
useAuthStore조회 + 분기 네비게이션 로직이 페이지에 직접 있어, NotFound 페이지와 함께useErrorPageHomeNavigation같은 훅으로 묶는 편이 컨벤션/중복 측면에서 더 좋습니다.As per coding guidelines
src/pages/**/index.tsx: 비즈니스 로직은 커스텀 훅으로 분리되어 있는지.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/ServerError/index.tsx` around lines 6 - 12, Extract the home-navigation business logic from ServerErrorPage into a custom hook (e.g., useErrorPageHomeNavigation) that encapsulates useAuthStore and useNavigate; replace the inline handleGoHome and the direct useAuthStore call in ServerErrorPage with a hook call that returns a goHome function (or navigate handler) and any needed state, so NotFound and other error pages can reuse the same logic and ServerErrorPage only handles rendering.
🤖 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/apis/auth/index.ts`:
- Around line 16-24: The server-error redirect call redirectToServerErrorPage()
currently returns void so execution continues and the ApiError is still
constructed and thrown; to fix, either make redirectToServerErrorPage() have a
never return type (so TypeScript and readers know it never returns) or, more
simply, add an explicit early return immediately after the
redirectToServerErrorPage() call in the block where
isServerErrorStatus(response.status) is true to prevent the subsequent
construction/throw of the ApiError (adjust code in the function containing
isServerErrorStatus check and the error/throw logic).
In `@src/apis/client.ts`:
- Around line 75-92: rethrowFetchError currently throws a plain Error for
AbortError which breaks the ApiError contract; update the AbortError branch to
construct and throw an ApiError (same shape as createNetworkApiError) with a
localized timeout message, set error.name (e.g., 'TimeoutError'), status (e.g.,
0), statusText (e.g., 'TIMEOUT'), and url (pass through the url param) so the UI
receives the same standardized error shape; you can reuse the pattern in
createNetworkApiError to build the ApiError for the timeout case inside
rethrowFetchError.
In `@src/components/auth/AuthGuard.tsx`:
- Around line 23-24: In AuthGuard.tsx update the screen-reader-only loading text
to use localization instead of the fixed English string: replace the hardcoded
<span className="sr-only">Loading…</span> with a localized message (e.g., use
your i18n helper like t('loading') or a localized constant) so the sr-only
content reflects the user's locale and matches the app's
accessibility/localization standards.
---
Outside diff comments:
In `@src/apis/auth/index.ts`:
- Around line 8-24: refreshAccessToken currently calls fetch directly without
handling network errors or following the module's ApiError normalization; either
replace the direct fetch with the shared apiClient.post call used by
signup/getMyInfo/logout or wrap the existing fetch in a try/catch and call the
shared rethrowFetchError logic so network failures are converted into ApiError
consistently: update refreshAccessToken to use apiClient.post('/users/refresh',
{ credentials: 'include' }) or surround the fetch(...) call with try { ... }
catch (err) { rethrowFetchError(err, 'refreshAccessToken', url) } and ensure any
thrown ApiError has status, statusText and url set as done elsewhere.
---
Nitpick comments:
In `@src/apis/client.ts`:
- Around line 59-62: The function throwApiError currently performs UI navigation
(calls redirectToServerErrorPage) which mixes API layer with UI; change
throwApiError to only create and throw a standardized error object (include
response.status and any parsed body/details) and remove the
redirectToServerErrorPage call from it, and let higher-level code (router/error
boundary or the caller) inspect isServerErrorStatus(response.status) and perform
navigation or handling; update any callers of throwApiError to catch the thrown
error and perform redirect when appropriate.
In `@src/pages/ServerError/index.tsx`:
- Around line 6-12: Extract the home-navigation business logic from
ServerErrorPage into a custom hook (e.g., useErrorPageHomeNavigation) that
encapsulates useAuthStore and useNavigate; replace the inline handleGoHome and
the direct useAuthStore call in ServerErrorPage with a hook call that returns a
goHome function (or navigate handler) and any needed state, so NotFound and
other error pages can reuse the same logic and ServerErrorPage only handles
rendering.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/apis/auth/index.tssrc/apis/client.tssrc/components/auth/AuthGuard.tsxsrc/components/common/ErrorPageLayout.tsxsrc/pages/NotFound/index.tsxsrc/pages/ServerError/index.tsxsrc/utils/ts/errorRedirect.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/utils/ts/errorRedirect.ts
src/components/auth/AuthGuard.tsx
Outdated
Comment on lines
+23
to
+24
| <div role="status" className="flex h-screen items-center justify-center"> | ||
| <span className="sr-only">Loading…</span> |
There was a problem hiding this comment.
로딩 보조 텍스트 로컬라이징 권장
스크린리더 전용 문구(Loading…)를 한국어로 맞추면 접근성/로케일 일관성이 더 좋아집니다.
As per coding guidelines src/components/**: 접근성(aria-*, role, 키보드 탐색)이 적절히 처리되는지.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/auth/AuthGuard.tsx` around lines 23 - 24, In AuthGuard.tsx
update the screen-reader-only loading text to use localization instead of the
fixed English string: replace the hardcoded <span
className="sr-only">Loading…</span> with a localized message (e.g., use your
i18n helper like t('loading') or a localized constant) so the sr-only content
reflects the user's locale and matches the app's accessibility/localization
standards.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/apis/auth/index.ts`:
- Around line 30-33: The current branch in isServerErrorStatus handling calls
redirectToServerErrorPage() but then does "return undefined as never", which
still resolves and lets callers receive undefined; replace that return with an
explicit throw (e.g., throw new Error("Redirecting to server error page") or a
dedicated RedirectError) so execution stops immediately after calling
redirectToServerErrorPage(); update any surrounding try/catch if needed in
callers of the function containing isServerErrorStatus to handle the thrown
error.
In `@src/apis/client.ts`:
- Around line 75-83: The AbortError is being unconditionally mapped to a
TimeoutError in rethrowFetchError; change rethrowFetchError(error: unknown, url:
string) to accept an additional optional boolean flag (e.g., isTimeout = false)
and only create/throw a TimeoutError when error is an AbortError AND isTimeout
is true; otherwise map AbortError to a generic cancel/abort ApiError (e.g., name
'Canceled' or preserve original) so caller-cancellations aren’t mislabeled as
timeouts. Update all callers (including the other occurrences you noted around
lines 189 and 287) to pass true when the abort originates from a timeout path
and false (or omit) when the abort comes from a caller-provided signal.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/apis/auth/index.tssrc/apis/client.tssrc/components/auth/AuthGuard.tsxsrc/pages/NotFound/index.tsxsrc/pages/ServerError/index.tsxsrc/utils/hooks/useErrorPageHomeNavigation.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/pages/ServerError/index.tsx
- src/pages/NotFound/index.tsx
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/apis/client.ts (1)
59-73: 서버 에러 시 불필요한parseErrorResponse호출.
isServerErrorStatus분기에서 리다이렉트 후에도parseErrorResponse가 실행됩니다. 서버 에러 페이지로 이동할 예정이므로 에러 파싱 없이 즉시 throw하는 것이 효율적입니다.♻️ 제안 diff
async function throwApiError(response: Response): Promise<never> { if (isServerErrorStatus(response.status)) { redirectToServerErrorPage(); + throw new Error('서버 오류가 발생했습니다.'); } const errorData = await parseErrorResponse(response); const error = new Error(errorData?.message ?? 'API 요청 실패') as ApiError; + error.name = 'ApiError'; error.status = response.status; error.statusText = response.statusText; error.url = response.url; error.apiError = errorData ?? undefined; throw error; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/apis/client.ts` around lines 59 - 73, The server-error branch in throwApiError currently calls redirectToServerErrorPage() but still continues to parse the response; change throwApiError so that if isServerErrorStatus(response.status) is true you call redirectToServerErrorPage() and then immediately stop execution (throw/return) without calling parseErrorResponse; move the parseErrorResponse and construction of the ApiError below that branch so parseErrorResponse is only invoked for non-server errors, and reference the throwApiError, isServerErrorStatus, parseErrorResponse, and redirectToServerErrorPage symbols when making the change.src/apis/auth/index.ts (1)
35-39:error.name속성 누락으로 에러 타입 식별 불가.네트워크 에러(Line 20)에서는
name = 'NetworkError'를 설정하지만, 토큰 갱신 실패 에러는name이 없어 에러 핸들러에서 타입 구분이 어렵습니다.♻️ 제안 diff
const error = new Error('토큰 갱신 실패') as ApiError; + error.name = 'AuthError'; error.status = response.status; error.statusText = response.statusText; error.url = url; throw error;As per coding guidelines
src/apis/**: 에러가 UI 친화적인 형태로 매핑/전달되는지.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/apis/auth/index.ts` around lines 35 - 39, The token-refresh error creation sets ApiError fields but omits error.name, making runtime handlers unable to distinguish types; update the block that constructs the ApiError (the const error = new Error('토큰 갱신 실패') as ApiError) to set error.name to a consistent value (e.g., 'TokenRefreshError')—matching the pattern used for network errors (name = 'NetworkError')—so downstream error handlers can branch on error.name along with status/statusText/url.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/apis/auth/index.ts`:
- Around line 35-39: The token-refresh error creation sets ApiError fields but
omits error.name, making runtime handlers unable to distinguish types; update
the block that constructs the ApiError (the const error = new Error('토큰 갱신 실패')
as ApiError) to set error.name to a consistent value (e.g.,
'TokenRefreshError')—matching the pattern used for network errors (name =
'NetworkError')—so downstream error handlers can branch on error.name along with
status/statusText/url.
In `@src/apis/client.ts`:
- Around line 59-73: The server-error branch in throwApiError currently calls
redirectToServerErrorPage() but still continues to parse the response; change
throwApiError so that if isServerErrorStatus(response.status) is true you call
redirectToServerErrorPage() and then immediately stop execution (throw/return)
without calling parseErrorResponse; move the parseErrorResponse and construction
of the ApiError below that branch so parseErrorResponse is only invoked for
non-server errors, and reference the throwApiError, isServerErrorStatus,
parseErrorResponse, and redirectToServerErrorPage symbols when making the
change.
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
1 participant
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.
Summary by CodeRabbit
새로운 기능
버그 수정