-
Notifications
You must be signed in to change notification settings - Fork 1
feat: 메인페이지 에러바운더리 적용 #65
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Walkthrough이 풀 리퀘스트에서는 여러 파일에 대한 변경 사항이 포함되어 있습니다. Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (5)
src/api/news/index.ts (1)
4-6: 응답 타입 명시와 에러 처리 개선이 필요합니다Response 타입 명시는 좋은 개선이지만, 에러 처리를 더 구체적으로 할 수 있습니다.
async function getNews(): Promise<News[]> { - const response: Response = await fetch( - `${process.env.NEXT_PUBLIC_API_URL}/home/news`, - ); + try { + const response: Response = await fetch( + `${process.env.NEXT_PUBLIC_API_URL}/home/news`, + ); + if (!response.ok) { + const errorData = await response.json().catch(() => ({})); + throw new Error(`뉴스 데이터 조회 실패: ${response.status} ${errorData.message || ''}`); + } + return response.json(); + } catch (error) { + console.error('뉴스 데이터 조회 중 오류 발생:', error); + throw error; + }src/components/common/error-boundary/index.tsx (2)
31-33: 새로고침 로직 개선이 필요합니다현재 상태만 초기화하고 있는데, 실제 데이터 리로드도 함께 수행하면 좋을 것 같습니다.
handleRefresh = () => { this.setState({ hasError: false }); + // 데이터 리로드 로직 추가 + if (this.props.onRefresh) { + this.props.onRefresh(); + } };관련하여 Props 인터페이스도 업데이트가 필요합니다:
interface ErrorBoundaryProps { children: ReactNode; errorMessage?: string; description?: string; className?: string; + onRefresh?: () => void; }
44-60: 접근성 개선이 필요합니다에러 메시지와 새로고침 버튼의 접근성을 개선할 수 있습니다.
return ( <div - className={`flex flex-col items-center justify-center p-16 text-center ${className}`} + className={`flex flex-col items-center justify-center p-16 text-center ${className}`} + role="alert" + aria-live="polite" > <div className="mb-8 text-red-500">⚠️</div> - <h3 className="mb-4 text-18-700">{errorMessage}</h3> + <h3 className="mb-4 text-18-700" id="error-title">{errorMessage}</h3> <p className="mb-6 text-14-400 text-gray-600">{description}</p> <button type="button" onClick={this.handleRefresh} + aria-labelledby="error-title" className="inline-flex items-center gap-2 rounded-md bg-blue-500 px-4 py-2 text-14-600 text-white hover:bg-blue-600 active:bg-blue-700" >src/app/page.tsx (2)
48-83: ErrorBoundary 구현 최적화가 필요합니다각 컴포넌트마다 ErrorBoundary를 개별적으로 적용한 것은 좋은 접근이지만, 몇 가지 개선사항이 있습니다:
- 공통 에러 메시지 관리
- 데이터 리로드 핸들링
- 성능 최적화
먼저 상수로 에러 메시지를 분리하는 것이 좋습니다:
// constants/error-messages.ts export const ERROR_MESSAGES = { SEARCH: "검색 기능을 일시적으로 사용할 수 없습니다", STOCK_INDEX: "주가 정보를 불러올 수 없습니다", TRADING_VOLUME: "거래량 순위를 불러올 수 없습니다", FLUCTUATION: "변동폭 순위를 불러올 수 없습니다", NEWS: "뉴스를 불러올 수 없습니다", ASSET: "자산 정보를 불러올 수 없습니다", MY_STOCK: "관심 종목을 불러올 수 없습니다" } as const;그리고 데이터 리로드를 위한 함수를 추가하는 것이 좋습니다:
const handleNewsRefresh = async () => { const news = await getNews(); queryClient.setQueryData(["news"], news); };적용 예시:
- <ErrorBoundary errorMessage="뉴스를 불러올 수 없습니다"> + <ErrorBoundary + errorMessage={ERROR_MESSAGES.NEWS} + onRefresh={handleNewsRefresh} + > <NewsCarousel initialData={initialNews} /> </ErrorBoundary>
Line range hint
15-41: 데이터 프리페칭 로직 개선이 필요합니다현재 프리페칭 로직에 에러 처리가 부족합니다. 또한 Promise.all을 사용하는 부분과 일관성이 없습니다.
+ const prefetchWithErrorHandling = async ( + queryKey: string, + queryFn: () => Promise<any>, + initialData: any + ) => { + try { + await queryClient.prefetchQuery({ + queryKey: [queryKey], + queryFn, + initialData, + }); + } catch (error) { + console.error(`Error prefetching ${queryKey}:`, error); + } + }; - const initialStockData = await getInitialStockData(); - await queryClient.prefetchQuery({ - queryKey: ["stockIndex"], - queryFn: getInitialStockData, - initialData: initialStockData, - }); + const [ + initialStockData, + initialNews, + traddata, + flucdata + ] = await Promise.all([ + getInitialStockData(), + getNews(), + getTradingVolume(), + getFluctuation(), + ]); + await Promise.all([ + prefetchWithErrorHandling("stockIndex", getInitialStockData, initialStockData), + prefetchWithErrorHandling("news", getNews, initialNews), + ]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
src/api/news/index.ts(1 hunks)src/app/main/_components/news-card.tsx(2 hunks)src/app/page.tsx(2 hunks)src/components/common/dropdown/index.tsx(2 hunks)src/components/common/error-boundary/index.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/app/main/_components/news-card.tsx
- src/components/common/dropdown/index.tsx
| componentDidCatch(error: Error, errorInfo: ErrorInfo): void { | ||
| console.error("Error Boundary caught an error:", error, errorInfo); //eslint-disable-line | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
에러 로깅 개선이 필요합니다
현재 콘솔 로그만 사용하고 있는데, 프로덕션 환경에서는 에러 추적 서비스(예: Sentry)를 활용하는 것이 좋습니다.
componentDidCatch(error: Error, errorInfo: ErrorInfo): void {
- console.error("Error Boundary caught an error:", error, errorInfo); //eslint-disable-line
+ // TODO: Sentry 또는 다른 에러 추적 서비스 연동
+ if (process.env.NODE_ENV === 'production') {
+ // captureException(error, { extra: errorInfo });
+ } else {
+ console.error("Error Boundary caught an error:", error, errorInfo);
+ }
}Committable suggestion skipped: line range outside the PR's diff.
#️⃣ 이슈
📝 작업 내용
메인페이지에 에러바운더리를 적용했습니다. 해당 컴포넌트에서 로딩아이콘이 뜨고 누르면 그 컴포넌트만 새로고침을 합니다
📸 스크린샷
✅ 체크 리스트
👩💻 공유 포인트 및 논의 사항
Summary by CodeRabbit
ErrorBoundary컴포넌트 추가.Wrapper및Item요소의 패딩 조정.NewsCard컴포넌트의 클래스 이름 재정렬.