[Feature] 계층적 에러 핸들 (Global/Content/Api ErrorBoundary) 도입#1214
Open
seongwon030 wants to merge 17 commits intodevelop-fefrom
Open
[Feature] 계층적 에러 핸들 (Global/Content/Api ErrorBoundary) 도입#1214seongwon030 wants to merge 17 commits intodevelop-fefrom
seongwon030 wants to merge 17 commits intodevelop-fefrom
Conversation
- global 에러, content에러, api에러 테스트 추가
- 개발모드에서만 에러메세지 표시
- HttpError를 상속받아 errorCode와 data 필드를 포함하는 ApiError 클래스 정의
- resetKeys를 pathname으로 설정
- status, statusText, message 정의
- React 클래스 컴포넌트 기반의 BaseErrorBoundary 구현 - 외부에서 리셋 로직을 주입받을 수 있도록 onReset, resetKeys prop 지원
- handleResponse 유틸 함수에서 일반 Error 대신 커스텀 ApiError를 throw하도록 변경 - 백엔드 응답의 status, statusText, errorCode, message 등을 구조화하여 전달
- 최상위 GlobalBoundary 하위에 라우트 레벨의 ContentErrorBoundary 적용 - 각 페이지 단위로 에러를 격리하여 앱 전체 크래시 방지
- VITE_ENABLE_SENTRY_IN_DEV 환경변수로 개발 환경에서 Sentry 활성화 제어 기능 추가
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Warning
|
| Cohort / File(s) | Summary |
|---|---|
앱 라우트 통합 frontend/src/App.tsx |
여러 Route를 ContentErrorBoundary로 래핑하고 기존의 GlobalBoundary를 유지. |
API 응답 처리 frontend/src/apis/utils/apiHelpers.ts |
비정상 응답에서 ApiError 생성/throw로 변경(본문 JSON 파싱, errorCode·message 추출 포함). |
에러 타입 정의 frontend/src/errors/HttpError.ts, frontend/src/errors/ApiError.ts, frontend/src/errors/NetworkError.ts, frontend/src/errors/index.ts |
HttpError/ApiError/NetworkError 클래스 추가 및 중앙 재수출. |
기본 에러 바운더리 인프라 frontend/src/components/common/ErrorBoundary/BaseErrorBoundary.tsx, frontend/src/components/common/ErrorBoundary/index.ts |
재사용 가능한 BaseErrorBoundary 추가 및 ErrorBoundary 모듈의 명시적 export 재구성. |
콘텐츠 레벨 바운더리 및 폴백 frontend/src/components/common/ErrorBoundary/ContentError/ContentErrorBoundary.tsx, .../ContentErrorFallback.tsx, .../ContentErrorFallback.styles.ts |
페이지 수준 ContentErrorBoundary와 폴백 UI/스타일 추가(경로를 resetKey로 사용, 재시도/홈 버튼 포함). |
API 레벨 바운더리 및 폴백 frontend/src/components/common/ErrorBoundary/ApiError/ApiErrorBoundary.tsx, .../ApiErrorFallback.tsx, .../ApiErrorFallback.styles.ts |
데이터 페치용 ApiErrorBoundary와 HTTP/Network 오류 유형별 폴백 UI·스타일 추가. |
글로벌 경로 수정 frontend/src/components/common/ErrorBoundary/GlobalError/GlobalBoundary.tsx |
Spinner 상대경로 조정(../ → ../../). |
에러 테스트 페이지 frontend/src/pages/ErrorTestPage/ErrorTestPage.tsx, .../ErrorTestPage.styles.ts |
계층별(글로벌/콘텐츠/API) 테스트 컴포넌트·Sentry 수동 테스트 추가 및 스타일 업데이트. |
Sentry 초기화 변경 frontend/src/utils/initSDK.ts |
개발 환경에서 Sentry 활성화 토글(VITE_ENABLE_SENTRY_IN_DEV) 추가 및 environment 필드 설정. |
에러 바운더리 인덱스 재구성 frontend/src/components/common/ErrorBoundary/index.ts |
Base/Global/Content/Api 바운더리 및 폴백을 명시적 exports로 정리. |
Sequence Diagram(s)
sequenceDiagram
participant User
participant Router
participant ContentBoundary as ContentErrorBoundary
participant Page as PageComponent
participant ApiBoundary as ApiErrorBoundary
participant API as BackendAPI
participant GlobalBoundary as GlobalBoundary
User->>Router: 경로 요청
Router->>ContentBoundary: Route 렌더링
ContentBoundary->>Page: children 렌더
Page->>ApiBoundary: 데이터 요청 컴포넌트 렌더
ApiBoundary->>API: fetch 요청
API-->>ApiBoundary: 200 / 4xx / 5xx 응답
alt 응답 오류 (4xx/5xx) 또는 네트워크 오류
ApiBoundary-->>ContentBoundary: throw ApiError / NetworkError
ContentBoundary-->>GlobalBoundary: 버블업(포착되지 않으면)
else 정상 응답
ApiBoundary-->>Page: 데이터 전달
end
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
- [Feature] 전역 에러 처리 및 Sentry 연동 #1196: App.tsx와 ErrorBoundary 통합 관련 변경과 코드 레벨 연관성(글로벌 바운더리 관련).
- [Feature] Promotion API 추가 및 테스트 인프라 구축 #1136:
apiHelpers.ts의 handleResponse 경로 및 에러 처리 변경과 직접적인 충돌 가능성 있음. - [release] FE v1.1.24 #1203: 에러 바운더리 구조 및 App 통합에 관한 PR로 기능적 유사성 및 통합 영향.
Suggested labels
🔨 Refactor
Suggested reviewers
- oesnuj
- lepitaaar
- suhyun113
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | PR 제목이 변경사항의 핵심(계층적 에러 바운더리 도입)을 정확하고 명확하게 요약하고 있습니다. |
| Linked Issues check | ✅ Passed | PR의 모든 코드 변경사항이 MOA-660의 목표(계층적 에러바운더리 설계 및 구현)를 완전히 충족합니다. |
| Out of Scope Changes check | ✅ Passed | 모든 변경사항이 에러 바운더리 구현, 에러 클래스 정의, API 헬퍼 개선과 관련되어 있으며 범위 내에 있습니다. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
- 📝 Generate docstrings (stacked PR)
- 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
- Commit unit tests in branch
feature/#1211-error-boundary-setup-MOA-660
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 help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (10)
frontend/src/components/common/ErrorBoundary/ContentError/ContentErrorBoundary.tsx (1)
4-4: 불필요하게 상위 디렉토리를 경유하는 임포트 경로이 파일이 이미
ContentError/디렉토리 안에 있으므로,../ContentError/ContentErrorFallback대신./ContentErrorFallback이 적절합니다.♻️ 수정 제안
-import ContentErrorFallback from '../ContentError/ContentErrorFallback'; +import ContentErrorFallback from './ContentErrorFallback';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/common/ErrorBoundary/ContentError/ContentErrorBoundary.tsx` at line 4, The import in ContentErrorBoundary.tsx unnecessarily traverses up one level; update the import for ContentErrorFallback in the ContentErrorBoundary component from '../ContentError/ContentErrorFallback' to a relative import './ContentErrorFallback' so the module resolves from the current ContentError directory (locate the import statement at the top of ContentErrorBoundary.tsx and replace the path accordingly).frontend/src/components/common/ErrorBoundary/ContentError/ContentErrorFallback.styles.ts (1)
3-109: 테마 시스템 미사용: 하드코딩된 색상값을 디자인 토큰으로 대체 필요이 파일의 모든 색상값(
#ff5414,#111111,#787878,#f5f5f5,#ebebeb등)이frontend/src/styles/theme/colors.ts에 이미 정의되어 있습니다. 예를 들어#ff5414는colors.primary[900],#787878은colors.gray[700]으로 정의되어 있습니다.또한 ApiErrorFallback.styles.ts와 GlobalErrorFallback.styles.ts 모두 동일한 구조(Container, Content, IconWrapper, Title, Message, ErrorDetails, ButtonGroup, BaseButton)를 가지고 있어 상당한 스타일 중복이 있습니다.
테마 토큰을 활용하고 공통 스타일을 추출하면 유지보수성과 일관성을 크게 개선할 수 있습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/common/ErrorBoundary/ContentError/ContentErrorFallback.styles.ts` around lines 3 - 109, The styles file uses hardcoded hex colors and duplicates layout across many components; replace all literal color values in Container, Content, IconWrapper, Title, Message, ErrorDetails, ErrorMessage, ButtonGroup, BaseButton, PrimaryButton, and SecondaryButton with the corresponding tokens from frontend/src/styles/theme/colors.ts (e.g., use colors.primary[900] for `#ff5414`, colors.gray[700] for `#787878`, colors.neutral[...] for `#f5f5f5/`#ebebeb, etc.), and refactor the repeated structure by extracting the shared styled components (Container, Content, IconWrapper, Title, Message, ErrorDetails, ButtonGroup, BaseButton) into a new common styles module that ApiErrorFallback.styles.ts, GlobalErrorFallback.styles.ts, and this file import from to remove duplication and ensure consistent theme usage.frontend/src/App.tsx (1)
50-121: 각<Route>마다<ContentErrorBoundary>를 반복 래핑하고 있어 중복이 많습니다.Layout Route 패턴을 활용하면 중복을 크게 줄일 수 있습니다. 예를 들어:
Layout Route 패턴 예시
// ContentErrorLayout.tsx const ContentErrorLayout = () => ( <ContentErrorBoundary> <Outlet /> </ContentErrorBoundary> ); // App.tsx <Routes> <Route element={<ContentErrorLayout />}> <Route path='/' element={<MainPage />} /> <Route path='/club/:clubId' element={<LegacyClubDetailPage />} /> {/* ...나머지 라우트 */} </Route> {/* ContentErrorBoundary가 필요 없는 라우트 */} <Route path='/admin/login' element={<LoginTab />} /> </Routes>단, Layout Route로 감싸면
ContentErrorBoundary인스턴스가 하나로 공유되므로, 라우트 변경 시resetKeys기반 리셋이 정상 동작하는지 확인이 필요합니다. 현재 구조에서는 각 라우트마다 별도 인스턴스가 생성되어 언마운트/리마운트로 자연스럽게 리셋됩니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/App.tsx` around lines 50 - 121, Refactor to eliminate repetitive wrapping of routes with ContentErrorBoundary by introducing a layout route component (e.g., ContentErrorLayout) that renders <ContentErrorBoundary> around an <Outlet />, then wrap all routes that currently use ContentErrorBoundary (paths rendering MainPage, LegacyClubDetailPage, ClubDetailPage, IntroducePage, AdminRoutes inside AdminClubProvider/PrivateRoute, ApplicationFormPage, ClubUnionPage) inside a single <Route element={<ContentErrorLayout />}> within your <Routes>. Keep routes that must remain unwrapped (e.g., LoginTab at '/admin/login') outside the layout, and after implementing, verify that ContentErrorBoundary’s reset behavior (resetKeys or unmount/remount semantics) still works as expected when navigating between these routes.frontend/src/components/common/ErrorBoundary/ContentError/ContentErrorFallback.tsx (2)
41-45:errornull 체크가 불필요합니다.
ErrorFallbackProps에서error는Error타입으로 non-nullable하게 정의되어 있으므로,isDev && error조건에서error체크는 항상 truthy입니다.isDev만으로 충분합니다.제안
- {isDev && error && ( + {isDev && (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/common/ErrorBoundary/ContentError/ContentErrorFallback.tsx` around lines 41 - 45, The conditional currently checks both isDev and error even though ErrorFallbackProps types error as non-nullable; remove the redundant error check and render the debug block guarded only by isDev (i.e., change the condition to isDev && ...), keeping the existing children Styled.ErrorDetails and Styled.ErrorMessage that reference error.message so the non-nullable error from ErrorFallbackProps is used directly.
4-18:WarningIcon과ApiErrorFallback의AlertIcon이 거의 동일한 패턴입니다.두 폴백 컴포넌트 모두 인라인 SVG 아이콘을 각각 정의하고 있습니다. 공통 아이콘 컴포넌트로 추출하거나, 프로젝트에서 사용 중인 아이콘 시스템으로 통합하면 중복을 줄일 수 있습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/common/ErrorBoundary/ContentError/ContentErrorFallback.tsx` around lines 4 - 18, WarningIcon and the AlertIcon used in ApiErrorFallback are duplicate inline SVGs; extract a shared icon component (e.g., CommonAlertIcon or AlertIcon) and replace both WarningIcon and the inline AlertIcon in ApiErrorFallback to import and use that single component so the SVG lives in one place and the two fallbacks consume the shared symbol. Locate the WarningIcon function and the AlertIcon JSX inside ApiErrorFallback, move the SVG markup into a new exported component (or into the project's icon system) and update both references to use the new component.frontend/src/components/common/ErrorBoundary/ApiError/ApiErrorFallback.tsx (1)
77-83:errornull 체크가 불필요합니다.
ContentErrorFallback과 동일하게,ErrorFallbackProps에서error는 non-nullableError타입이므로isDev만으로 충분합니다.제안
- {isDev && error && ( + {isDev && (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/common/ErrorBoundary/ApiError/ApiErrorFallback.tsx` around lines 77 - 83, Remove the redundant null check on error in ApiErrorFallback: inside the ApiErrorFallback component where the JSX currently guards with "{isDev && error && (...)}", change the condition to only "{isDev && (...)}" because ErrorFallbackProps types error as a non-nullable Error (same pattern as ContentErrorFallback); update the referenced block that renders Styled.ErrorDetails / Styled.ErrorDetailsMessage accordingly so it relies on error being present per the types.frontend/src/errors/ApiError.ts (1)
3-15: 생성자 파라미터 순서 개선을 고려해 주세요.
message가 마지막 파라미터인데,errorCode와data없이message만 전달하려면 중간 인자에undefined/null을 채워야 합니다. 실제로ErrorTestPage.tsx에서new ApiError(404, 'Not Found', 'RESOURCE_NOT_FOUND', null, '...')처럼data에null을 명시적으로 전달하고 있습니다.옵션 객체 패턴을 사용하면 호출부가 더 명확해집니다:
옵션 객체 패턴 예시
+interface ApiErrorOptions { + errorCode?: string; + data?: unknown; + message?: string; +} + export class ApiError extends HttpError { + public readonly errorCode?: string; + public readonly data?: unknown; + constructor( status: number, statusText: string, - public readonly errorCode?: string, - public readonly data?: unknown, - message?: string, + options?: ApiErrorOptions, ) { - super(status, statusText, message); + super(status, statusText, options?.message); this.name = 'ApiError'; + this.errorCode = options?.errorCode; + this.data = options?.data; Object.setPrototypeOf(this, ApiError.prototype); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/errors/ApiError.ts` around lines 3 - 15, Change ApiError to use an options-object pattern so callers can pass message without needing placeholders: update the constructor signature of ApiError to something like constructor(status: number, statusText: string, message?: string, options?: { errorCode?: string; data?: unknown }) and map options.errorCode/options.data to the class readonly fields; keep calling super(status, statusText, message), maintain this.name and Object.setPrototypeOf(this, ApiError.prototype), and update callers (e.g., ErrorTestPage.tsx) to use new ApiError(404, 'Not Found', 'human message', { errorCode: 'RESOURCE_NOT_FOUND', data: null }) instead of stuffing undefined into positional args.frontend/src/pages/ErrorTestPage/ErrorTestPage.tsx (1)
109-122: Sentry 테스트 함수에서alert()를 사용하고 있습니다.개발 전용 테스트 페이지이므로 큰 문제는 아니지만,
alert()는 메인 스레드를 블로킹합니다. 향후에는 토스트 메시지나 페이지 내 알림으로 교체하는 것이 더 나은 UX를 제공합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/pages/ErrorTestPage/ErrorTestPage.tsx` around lines 109 - 122, testSentryCapture and testSentryMessage use blocking alert() calls; replace those alert(...) calls with the app's non-blocking toast/notification API (e.g., useToast, ToastContext, enqueueSnackbar, or the project's Notification component) so UX isn't blocked; keep the Sentry.captureException/captureMessage calls intact, then call the toast with a success/info message (and appropriate severity) instead of alert, and ensure any imports/hooks needed for the toast are added to ErrorTestPage.tsx and used inside testSentryCapture and testSentryMessage.frontend/src/components/common/ErrorBoundary/index.ts (1)
1-8: 명명 일관성:GlobalBoundaryvsContentErrorBoundary/ApiErrorBoundary
ContentErrorBoundary,ApiErrorBoundary에는Error가 포함되어 있으나GlobalBoundary(Line 3)에는 빠져 있습니다. 3계층 구조의 일관성을 위해GlobalErrorBoundary로 통일하는 것을 고려해 주세요. 같은 모듈의 폴백 컴포넌트인GlobalErrorFallback도 "Error"를 포함하고 있으므로 경계 컴포넌트도 맞춰주면 좋습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/common/ErrorBoundary/index.ts` around lines 1 - 8, The export name is inconsistent: change the re-export of GlobalBoundary to GlobalErrorBoundary to match the other boundary names and its fallback GlobalErrorFallback; update the export statement currently exporting default as GlobalBoundary in this file to export default as GlobalErrorBoundary (symbol: GlobalErrorBoundary) and ensure the module path './GlobalError/GlobalBoundary' still points to the component (or rename the component's default export in that module if necessary) so all boundary exports are consistently named (ContentErrorBoundary, ApiErrorBoundary, GlobalErrorBoundary).frontend/src/components/common/ErrorBoundary/ApiError/ApiErrorFallback.styles.ts (1)
1-82: 하드코딩된 컬러값을 테마 토큰으로 교체하세요.프로젝트의 테마 시스템(
@/styles/theme/colors)이 잘 정의되어 있으나, 이 파일은 직접 색상값을 하드코딩하고 있습니다. 다음과 같이 매핑하여 교체해주세요:
#989898→theme.colors.gray[600]#333333→theme.colors.gray[900]#ebebeb→theme.colors.gray[300]#f8f8f8→theme.colors.gray[100]#ff5414→theme.colors.primary[900]#dcdcdc→theme.colors.gray[400]또한
font-family: 'Pretendard'는 Global.styles.ts의 createGlobalStyle에서 이미 전역으로 선언되어 있으므로, RetryButton(Line 72)에서 중복 선언을 제거하는 것이 좋습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/common/ErrorBoundary/ApiError/ApiErrorFallback.styles.ts` around lines 1 - 82, Replace hardcoded hex colors in this styled-components file with the theme tokens and remove the duplicate font-family on RetryButton: update IconWrapper color to theme.colors.gray[600], Title color to theme.colors.gray[900], ErrorDetails border to theme.colors.gray[300], ErrorDetails background to theme.colors.gray[100], ErrorDetailsMessage color to theme.colors.primary[900], and RetryButton border color to theme.colors.gray[400]; ensure you reference the theme (props.theme or styled-components theme) within Container/Content/IconWrapper/Title/Message/ErrorDetails/ErrorDetailsMessage/RetryButton and delete the font-family declaration from RetryButton since global styles already set it.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@frontend/src/components/common/ErrorBoundary/ApiError/ApiErrorFallback.styles.ts`:
- Around line 62-82: RetryButton is missing a :focus-visible state so keyboard
users get no visual focus cue; update the styled component RetryButton to add a
:focus-visible rule that provides a clear visible outline (or box-shadow and
adjusted border-color) consistent with :hover/:active styles and ensure it does
not appear on mouse focus only (use :focus-visible pseudo-class). Modify the
RetryButton styled definition to include the :focus-visible selector and mirror
the component's visual language (e.g., solid outline color, increased z-index if
needed) so keyboard navigation accessibility is restored.
In `@frontend/src/pages/ErrorTestPage/ErrorTestPage.tsx`:
- Around line 223-255: The ApiErrorBoundary is not resetting when the external
"Api 리셋" button updates apiErrorType because the boundary lacks resetKeys;
update the usage around ApiErrorBoundary (wrapping ApiErrorTest) to pass
resetKeys={[apiErrorType]} so the boundary resets whenever apiErrorType changes;
for ContentErrorBoundary, add support for an optional resetKeys prop in the
ContentErrorBoundary component (in addition to its existing pathname-based
reset) and then pass resetKeys={[contentError]} from the page when you want the
boundary to auto-reset on contentError changes.
In `@frontend/src/utils/initSDK.ts`:
- Line 64: The environment field is always falling back to 'production' because
import.meta.env.NODE_ENV is undefined; change the assignment in initSDK.ts (the
environment property) to use the Vite mode variable first (e.g.,
import.meta.env.MODE) and fall back to import.meta.env.NODE_ENV and then
'production' so dev builds are labeled correctly (for example: environment:
import.meta.env.MODE || import.meta.env.NODE_ENV || 'production').
- Around line 45-52: The environment guard is using import.meta.env.NODE_ENV
which Vite does not populate, so replace that check with Vite-provided variables
(e.g., import.meta.env.DEV or import.meta.env.MODE === 'development') to
correctly detect dev mode; update the conditional around enableInDev (the block
that reads import.meta.env.VITE_ENABLE_SENTRY_IN_DEV and the if statement
currently referencing NODE_ENV) so it returns early only when running in
development according to import.meta.env.DEV (or MODE) and enableInDev is false,
preserving the existing console message flow.
---
Nitpick comments:
In `@frontend/src/App.tsx`:
- Around line 50-121: Refactor to eliminate repetitive wrapping of routes with
ContentErrorBoundary by introducing a layout route component (e.g.,
ContentErrorLayout) that renders <ContentErrorBoundary> around an <Outlet />,
then wrap all routes that currently use ContentErrorBoundary (paths rendering
MainPage, LegacyClubDetailPage, ClubDetailPage, IntroducePage, AdminRoutes
inside AdminClubProvider/PrivateRoute, ApplicationFormPage, ClubUnionPage)
inside a single <Route element={<ContentErrorLayout />}> within your <Routes>.
Keep routes that must remain unwrapped (e.g., LoginTab at '/admin/login')
outside the layout, and after implementing, verify that ContentErrorBoundary’s
reset behavior (resetKeys or unmount/remount semantics) still works as expected
when navigating between these routes.
In
`@frontend/src/components/common/ErrorBoundary/ApiError/ApiErrorFallback.styles.ts`:
- Around line 1-82: Replace hardcoded hex colors in this styled-components file
with the theme tokens and remove the duplicate font-family on RetryButton:
update IconWrapper color to theme.colors.gray[600], Title color to
theme.colors.gray[900], ErrorDetails border to theme.colors.gray[300],
ErrorDetails background to theme.colors.gray[100], ErrorDetailsMessage color to
theme.colors.primary[900], and RetryButton border color to
theme.colors.gray[400]; ensure you reference the theme (props.theme or
styled-components theme) within
Container/Content/IconWrapper/Title/Message/ErrorDetails/ErrorDetailsMessage/RetryButton
and delete the font-family declaration from RetryButton since global styles
already set it.
In `@frontend/src/components/common/ErrorBoundary/ApiError/ApiErrorFallback.tsx`:
- Around line 77-83: Remove the redundant null check on error in
ApiErrorFallback: inside the ApiErrorFallback component where the JSX currently
guards with "{isDev && error && (...)}", change the condition to only "{isDev &&
(...)}" because ErrorFallbackProps types error as a non-nullable Error (same
pattern as ContentErrorFallback); update the referenced block that renders
Styled.ErrorDetails / Styled.ErrorDetailsMessage accordingly so it relies on
error being present per the types.
In
`@frontend/src/components/common/ErrorBoundary/ContentError/ContentErrorBoundary.tsx`:
- Line 4: The import in ContentErrorBoundary.tsx unnecessarily traverses up one
level; update the import for ContentErrorFallback in the ContentErrorBoundary
component from '../ContentError/ContentErrorFallback' to a relative import
'./ContentErrorFallback' so the module resolves from the current ContentError
directory (locate the import statement at the top of ContentErrorBoundary.tsx
and replace the path accordingly).
In
`@frontend/src/components/common/ErrorBoundary/ContentError/ContentErrorFallback.styles.ts`:
- Around line 3-109: The styles file uses hardcoded hex colors and duplicates
layout across many components; replace all literal color values in Container,
Content, IconWrapper, Title, Message, ErrorDetails, ErrorMessage, ButtonGroup,
BaseButton, PrimaryButton, and SecondaryButton with the corresponding tokens
from frontend/src/styles/theme/colors.ts (e.g., use colors.primary[900] for
`#ff5414`, colors.gray[700] for `#787878`, colors.neutral[...] for `#f5f5f5/`#ebebeb,
etc.), and refactor the repeated structure by extracting the shared styled
components (Container, Content, IconWrapper, Title, Message, ErrorDetails,
ButtonGroup, BaseButton) into a new common styles module that
ApiErrorFallback.styles.ts, GlobalErrorFallback.styles.ts, and this file import
from to remove duplication and ensure consistent theme usage.
In
`@frontend/src/components/common/ErrorBoundary/ContentError/ContentErrorFallback.tsx`:
- Around line 41-45: The conditional currently checks both isDev and error even
though ErrorFallbackProps types error as non-nullable; remove the redundant
error check and render the debug block guarded only by isDev (i.e., change the
condition to isDev && ...), keeping the existing children Styled.ErrorDetails
and Styled.ErrorMessage that reference error.message so the non-nullable error
from ErrorFallbackProps is used directly.
- Around line 4-18: WarningIcon and the AlertIcon used in ApiErrorFallback are
duplicate inline SVGs; extract a shared icon component (e.g., CommonAlertIcon or
AlertIcon) and replace both WarningIcon and the inline AlertIcon in
ApiErrorFallback to import and use that single component so the SVG lives in one
place and the two fallbacks consume the shared symbol. Locate the WarningIcon
function and the AlertIcon JSX inside ApiErrorFallback, move the SVG markup into
a new exported component (or into the project's icon system) and update both
references to use the new component.
In `@frontend/src/components/common/ErrorBoundary/index.ts`:
- Around line 1-8: The export name is inconsistent: change the re-export of
GlobalBoundary to GlobalErrorBoundary to match the other boundary names and its
fallback GlobalErrorFallback; update the export statement currently exporting
default as GlobalBoundary in this file to export default as GlobalErrorBoundary
(symbol: GlobalErrorBoundary) and ensure the module path
'./GlobalError/GlobalBoundary' still points to the component (or rename the
component's default export in that module if necessary) so all boundary exports
are consistently named (ContentErrorBoundary, ApiErrorBoundary,
GlobalErrorBoundary).
In `@frontend/src/errors/ApiError.ts`:
- Around line 3-15: Change ApiError to use an options-object pattern so callers
can pass message without needing placeholders: update the constructor signature
of ApiError to something like constructor(status: number, statusText: string,
message?: string, options?: { errorCode?: string; data?: unknown }) and map
options.errorCode/options.data to the class readonly fields; keep calling
super(status, statusText, message), maintain this.name and
Object.setPrototypeOf(this, ApiError.prototype), and update callers (e.g.,
ErrorTestPage.tsx) to use new ApiError(404, 'Not Found', 'human message', {
errorCode: 'RESOURCE_NOT_FOUND', data: null }) instead of stuffing undefined
into positional args.
In `@frontend/src/pages/ErrorTestPage/ErrorTestPage.tsx`:
- Around line 109-122: testSentryCapture and testSentryMessage use blocking
alert() calls; replace those alert(...) calls with the app's non-blocking
toast/notification API (e.g., useToast, ToastContext, enqueueSnackbar, or the
project's Notification component) so UX isn't blocked; keep the
Sentry.captureException/captureMessage calls intact, then call the toast with a
success/info message (and appropriate severity) instead of alert, and ensure any
imports/hooks needed for the toast are added to ErrorTestPage.tsx and used
inside testSentryCapture and testSentryMessage.
Comment on lines
+62
to
+82
| export const RetryButton = styled.button` | ||
| padding: 8px 24px; | ||
| font-size: 14px; | ||
| font-weight: 600; | ||
| border-radius: 6px; | ||
| border: 1px solid #dcdcdc; | ||
| background: white; | ||
| color: #4b4b4b; | ||
| cursor: pointer; | ||
| transition: all 0.2s ease; | ||
| font-family: 'Pretendard', sans-serif; | ||
|
|
||
| &:hover { | ||
| background: #f5f5f5; | ||
| border-color: #c5c5c5; | ||
| } | ||
|
|
||
| &:active { | ||
| transform: scale(0.98); | ||
| } | ||
| `; |
Contributor
There was a problem hiding this comment.
RetryButton에 :focus-visible 스타일이 누락되어 있습니다.
키보드 내비게이션 사용자가 버튼에 포커스했을 때 시각적 피드백이 없습니다. :hover와 :active는 정의되어 있으나 :focus-visible 상태가 빠져 있어 접근성(a11y) 이슈가 됩니다.
제안: focus-visible 스타일 추가
&:active {
transform: scale(0.98);
}
+
+ &:focus-visible {
+ outline: 2px solid `#4b4b4b`;
+ outline-offset: 2px;
+ }
`;📝 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.
Suggested change
| export const RetryButton = styled.button` | |
| padding: 8px 24px; | |
| font-size: 14px; | |
| font-weight: 600; | |
| border-radius: 6px; | |
| border: 1px solid #dcdcdc; | |
| background: white; | |
| color: #4b4b4b; | |
| cursor: pointer; | |
| transition: all 0.2s ease; | |
| font-family: 'Pretendard', sans-serif; | |
| &:hover { | |
| background: #f5f5f5; | |
| border-color: #c5c5c5; | |
| } | |
| &:active { | |
| transform: scale(0.98); | |
| } | |
| `; | |
| export const RetryButton = styled.button` | |
| padding: 8px 24px; | |
| font-size: 14px; | |
| font-weight: 600; | |
| border-radius: 6px; | |
| border: 1px solid `#dcdcdc`; | |
| background: white; | |
| color: `#4b4b4b`; | |
| cursor: pointer; | |
| transition: all 0.2s ease; | |
| font-family: 'Pretendard', sans-serif; | |
| &:hover { | |
| background: `#f5f5f5`; | |
| border-color: `#c5c5c5`; | |
| } | |
| &:active { | |
| transform: scale(0.98); | |
| } | |
| &:focus-visible { | |
| outline: 2px solid `#4b4b4b`; | |
| outline-offset: 2px; | |
| } | |
| `; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@frontend/src/components/common/ErrorBoundary/ApiError/ApiErrorFallback.styles.ts`
around lines 62 - 82, RetryButton is missing a :focus-visible state so keyboard
users get no visual focus cue; update the styled component RetryButton to add a
:focus-visible rule that provides a clear visible outline (or box-shadow and
adjusted border-color) consistent with :hover/:active styles and ensure it does
not appear on mouse focus only (use :focus-visible pseudo-class). Modify the
RetryButton styled definition to include the :focus-visible selector and mirror
the component's visual language (e.g., solid outline color, increased z-index if
needed) so keyboard navigation accessibility is restored.
Comment on lines
+223
to
+255
| <ApiErrorBoundary> | ||
| <ApiErrorTest errorType={apiErrorType} /> | ||
| </ApiErrorBoundary> | ||
|
|
||
| <Styled.ButtonGroup> | ||
| <Styled.TestButton | ||
| onClick={() => testApiError('404')} | ||
| $variant='danger' | ||
| > | ||
| 404 Not Found | ||
| </Styled.TestButton> | ||
| <Styled.TestButton | ||
| onClick={() => testApiError('403')} | ||
| $variant='danger' | ||
| > | ||
| 403 Forbidden | ||
| </Styled.TestButton> | ||
| <Styled.TestButton | ||
| onClick={() => testApiError('500')} | ||
| $variant='danger' | ||
| > | ||
| 500 Server Error | ||
| </Styled.TestButton> | ||
| <Styled.TestButton | ||
| onClick={() => testApiError('network')} | ||
| $variant='danger' | ||
| > | ||
| Network Error | ||
| </Styled.TestButton> | ||
| <Styled.TestButton onClick={resetApiError} $variant='info'> | ||
| Api 리셋 | ||
| </Styled.TestButton> | ||
| </Styled.ButtonGroup> |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd "ApiErrorBoundary" --type f | head -20Repository: Moadong/moadong
Length of output: 133
🏁 Script executed:
fd "ErrorBoundary" --type f | grep -E "(tsx|ts)$" | head -20Repository: Moadong/moadong
Length of output: 283
🏁 Script executed:
find . -type f -name "*.tsx" -o -name "*.ts" | xargs grep -l "ApiErrorBoundary" | head -10Repository: Moadong/moadong
Length of output: 244
🏁 Script executed:
cat -n frontend/src/components/common/ErrorBoundary/ApiError/ApiErrorBoundary.tsxRepository: Moadong/moadong
Length of output: 802
🏁 Script executed:
cat -n frontend/src/components/common/ErrorBoundary/BaseErrorBoundary.tsxRepository: Moadong/moadong
Length of output: 2277
🏁 Script executed:
cat -n frontend/src/components/common/ErrorBoundary/ContentError/ContentErrorBoundary.tsxRepository: Moadong/moadong
Length of output: 804
외부 리셋 버튼이 ApiErrorBoundary를 실제로 리셋하지 못합니다.
"Api 리셋" 버튼(Line 252)은 ApiErrorBoundary 바깥에 있으므로 apiErrorType 상태만 변경할 뿐 바운더리 자체는 여전히 에러 상태입니다. 사용자가 폴백 UI의 "다시 시도" 버튼까지 클릭해야 바운더리가 리셋됩니다.
ApiErrorBoundary에 resetKeys={[apiErrorType]}을 전달하면 외부 상태 변경 시 바운더리가 자동으로 리셋됩니다.
제안
- <ApiErrorBoundary>
+ <ApiErrorBoundary resetKeys={[apiErrorType]}>
<ApiErrorTest errorType={apiErrorType} />
</ApiErrorBoundary>동일한 문제가 ContentErrorBoundary(Line 194-196)에도 적용됩니다. 다만 ContentErrorBoundary는 현재 pathname을 기반으로 리셋되도록 설계되어 있으므로, contentError 상태 변경 시 자동으로 리셋하려면 ContentErrorBoundary 컴포넌트 자체를 수정하여 추가 resetKeys를 지원하도록 해야 합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/pages/ErrorTestPage/ErrorTestPage.tsx` around lines 223 - 255,
The ApiErrorBoundary is not resetting when the external "Api 리셋" button updates
apiErrorType because the boundary lacks resetKeys; update the usage around
ApiErrorBoundary (wrapping ApiErrorTest) to pass resetKeys={[apiErrorType]} so
the boundary resets whenever apiErrorType changes; for ContentErrorBoundary, add
support for an optional resetKeys prop in the ContentErrorBoundary component (in
addition to its existing pathname-based reset) and then pass
resetKeys={[contentError]} from the page when you want the boundary to
auto-reset on contentError changes.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
frontend/src/components/common/ErrorBoundary/ContentError/ContentErrorBoundary.tsx (1)
4-4: 불필요하게 우회하는 임포트 경로를 단순화하세요.
ContentErrorBoundary.tsx는ContentError/디렉터리 안에 있으므로,../ContentError/ContentErrorFallback은 결국./ContentErrorFallback과 동일한 경로로 해석됩니다. 한 단계 올라갔다가 다시 같은 폴더로 내려오는 우회 경로는 혼란을 줄 수 있습니다.♻️ 제안: 임포트 경로 단순화
-import ContentErrorFallback from '../ContentError/ContentErrorFallback'; +import ContentErrorFallback from './ContentErrorFallback';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/common/ErrorBoundary/ContentError/ContentErrorBoundary.tsx` at line 4, The import in ContentErrorBoundary.tsx uses an unnecessary roundabout path; update the import of ContentErrorFallback in ContentErrorBoundary.tsx to a direct relative path (replace the "../ContentError/ContentErrorFallback" style import with the local "./ContentErrorFallback" form) so it references the sibling file in the same directory without going up and back down.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@frontend/src/components/common/ErrorBoundary/ContentError/ContentErrorBoundary.tsx`:
- Line 4: The import in ContentErrorBoundary.tsx uses an unnecessary roundabout
path; update the import of ContentErrorFallback in ContentErrorBoundary.tsx to a
direct relative path (replace the "../ContentError/ContentErrorFallback" style
import with the local "./ContentErrorFallback" form) so it references the
sibling file in the same directory without going up and back down.
- 개발 환경에서도 VITE_ENABLE_SENTRY_IN_DEV=true 환경변수로 Sentry를 활성화할 수 있도록 변경 - Sentry environment 설정을 import.meta.env.MODE 기반으로 명시하여 환경 구분 강
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.
#️⃣연관된 이슈
📝작업 내용
전역 에러바운더리 하나로는 어떤 부분에서 에러가 났는지 구체적으로 알기 어려웠습니다. 그래서 Api에러 -> 페이지(or컴포넌트)에러
-> 전역에러 이렇게 3개의 계층으로 나누어, 최상위로 에러가 전파되도록 설계했습니다.
1. 에러 바운더리 계층화
BaseErrorBoundary: 공통 로직(에러 캐치, 리셋, 상태 관리)을 담당하는 클래스 컴포넌트 구현.GlobalErrorBoundary (Level 1):ContentErrorBoundary (Level 2):ApiErrorBoundary (Level 3):2. 커스텀 에러 클래스 정의
3. 유틸리티 및 설정 개선
VITE_ENABLE_SENTRY_IN_DEV=true로 활성화할 수 있도록 설정 개선.중점적으로 리뷰받고 싶은 부분(선택)
논의하고 싶은 부분(선택)
🫡 참고사항
Summary by CodeRabbit
새로운 기능
버그 수정