실시간 알림(SSE) 버그 수정 ( issue #310 ) #325
Conversation
|
""" Walkthrough실시간 알림 기능이 컨텍스트 기반으로 리팩토링되었습니다. 알림 상태와 관련 함수들이 Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Header
participant NotificationProvider
participant useNotification
participant SseContext
User->>Header: 페이지 진입
Header->>SseContext: useNotificationContext()로 signalData 접근
User->>NotificationProvider: 앱 루트에서 감쌈
NotificationProvider->>SseContext: signalData/clearSignal/setSignal 제공
NotificationProvider->>useNotification: NotificationInitializer 통해 훅 실행
useNotification->>SseContext: setSignal로 실시간 알림 데이터 갱신
Header->>SseContext: 알림 벨 클릭 시 clearSignal 호출
SseContext->>Header: signalData 업데이트 시 UI 반영
Assessment against linked issues
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/context/SseContext.tsx (1)
1-17: 잘 구현된 컨텍스트 구조입니다.SSE 알림 관리를 위한 컨텍스트가 깔끔하게 설계되었습니다. 타입 안전성과 기본값 설정이 적절합니다.
향후 개선을 위한 제안: 에러 상태나 로딩 상태도 컨텍스트에 포함하는 것을 고려해보세요.
export interface SseContextProps { signalData: AlarmLive | null; + isConnecting?: boolean; + error?: string | null; clearSignal: () => void; setSignal: (data: AlarmLive | null) => void; }src/hooks/user/useNotification.ts (2)
29-35: EventSource 생성 방식 개선과 보안 고려사항CORS preflight 문제 해결을 위한 접근은 적절하지만, 몇 가지 개선점이 있습니다:
삼항 연산자 단순화: 라인 33의
accessToken ? accessToken : ''는accessToken || ''로 단순화할 수 있습니다.URL 파라미터 보안 고려: 액세스 토큰을 URL 파라미터로 전송하면 서버 로그에 노출될 수 있습니다. 가능하다면 서버 측에서 쿠키 기반 인증을 고려해보세요.
- `${import.meta.env.VITE_APP_API_BASE_URL}user/sse?accessToken=${ - accessToken ? accessToken : '' - }` + `${import.meta.env.VITE_APP_API_BASE_URL}user/sse?accessToken=${accessToken || ''}`
39-42: 프로덕션 코드에서 console.log 제거 필요개발 중 디버깅을 위한 console.log 문들이 남아있습니다. 프로덕션 배포 전에 제거해주세요.
- eventSource.onopen = () => { - console.log('확인'); - console.log(eventSource.readyState); - }; + eventSource.onopen = () => { + // SSE connection established successfully + };- console.log(eventData);Also applies to: 48-48
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/components/common/header/Header.tsx(3 hunks)src/components/user/notificationLive/NotificationInitializer.tsx(1 hunks)src/components/user/notificationLive/NotificationProvider.tsx(1 hunks)src/context/SseContext.tsx(1 hunks)src/hooks/user/useNotification.ts(2 hunks)src/routes/AppRoutes.tsx(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
src/context/SseContext.tsx (1)
src/models/alarm.ts (1)
AlarmLive(28-33)
src/components/user/notificationLive/NotificationProvider.tsx (2)
src/models/alarm.ts (1)
AlarmLive(28-33)src/context/SseContext.tsx (1)
SseContext(10-14)
src/components/common/header/Header.tsx (3)
src/context/SseContext.tsx (1)
useNotificationContext(16-16)src/util/formatImgPath.ts (1)
formatImgPath(1-4)src/api/alarm.api.ts (1)
testLiveAlarm(38-49)
src/hooks/user/useNotification.ts (4)
src/hooks/useToast.ts (1)
useToast(4-7)src/context/SseContext.tsx (1)
useNotificationContext(16-16)src/models/alarm.ts (1)
AlarmLive(28-33)src/hooks/queries/user/keys.ts (1)
AlarmList(31-33)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: accessibility-test
🔇 Additional comments (7)
src/components/user/notificationLive/NotificationProvider.tsx (1)
5-16: 컨텍스트 프로바이더가 올바르게 구현되었습니다.상태 관리와 함수 정의가 깔끔하고, React 컨텍스트 패턴을 적절히 따르고 있습니다.
src/routes/AppRoutes.tsx (1)
387-402:Details
✅ Verification successful
라우팅 구조 변경이 전역 컨텍스트 제공에 적합합니다.
createBrowserRouter와 루트 래퍼를 사용한 구조 변경으로 알림 시스템이 앱 전체에서 사용 가능해졌습니다. 프로바이더 순서와Outlet패턴 사용이 적절합니다.기존 중첩 라우팅이 새로운 구조에서도 정상 작동하는지 확인해주세요.
🏁 Script executed:
#!/bin/bash # 중첩 라우팅이 있는 라우트들 확인 rg -A 3 "children:" src/routes/AppRoutes.tsxLength of output: 1014
중첩 라우팅 정상 동작 확인 완료
rg -A 3 "children:" src/routes/AppRoutes.tsx결과, 기존의 모든 중첩된children배열이 유지되어 있음을 확인했습니다.newRouteList에 기존 라우트 정의가 포함되어 있고, 루트 레벨에{ path: '*', element: <NotFoundPage /> }가 추가된 구조로, 와일드카드 404 처리도 올바르게 적용됩니다.- Outlet 패턴하에 프로바이더 래퍼(
NotificationProvider,ToastProvider,NotificationInitializer)가 앱 전역에 정상적으로 적용될 것으로 보입니다.위 확인 결과를 바탕으로 변경된 라우터 설정을 승인합니다.
src/hooks/user/useNotification.ts (2)
1-1: 컨텍스트 기반 접근 방식으로의 전환이 적절합니다.새로운
useNotificationContext훅을 통해 전역 상태 관리로 전환하는 것이 올바른 접근입니다.Also applies to: 7-7
14-14: 컨텍스트에서 필요한 함수만 구조분해할당하는 것이 좋습니다.전역 상태 관리를 위해
setSignal함수를 올바르게 가져오고 있습니다.src/components/common/header/Header.tsx (3)
14-20: 알림 기능 관련 임포트 복원이 적절합니다.실시간 알림 시스템 구현에 필요한 모든 컴포넌트와 훅들이 올바르게 임포트되었습니다.
30-30: 컨텍스트에서 필요한 상태와 함수를 올바르게 가져오고 있습니다.
signalData와clearSignal을 통해 알림 상태 관리가 적절히 구현되었습니다.
62-78: 알림 벨 UI 통합이 잘 구현되었습니다.로그인 상태에 따른 조건부 렌더링, 새 알림 표시를 위한 점 표시, 클릭 시 알림 클리어 기능이 모두 적절히 구현되었습니다. 접근성 속성도 잘 유지되었습니다.
| const NotificationInitializer = () => { | ||
| useNotification(); | ||
|
|
||
| return null; | ||
| }; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
초기화 컴포넌트 패턴이 적절합니다.
전역 SSE 연결 초기화를 위한 깔끔한 접근 방식입니다.
useNotification 훅에서 SSE 연결의 적절한 정리(cleanup)가 이루어지는지 확인해주세요. EventSource 연결이 컴포넌트 언마운트 시 제대로 닫히지 않으면 메모리 누수가 발생할 수 있습니다.
🏁 Script executed:
#!/bin/bash
# useNotification 훅에서 EventSource cleanup 확인
ast-grep --pattern $'useEffect(() => {
$$$
return () => {
$$$
}
}, $$$)'Length of output: 3614
useNotification 훅 정의 위치와 EventSource 사용 여부를 확인하기 위해 다음 스크립트를 실행해주세요:
🏁 Script executed:
#!/bin/bash
# useNotification 훅이 정의된 파일 찾기
rg "useNotification" -n src
# EventSource 인스턴스 생성 및 cleanup 확인
rg "EventSource" -n srcLength of output: 1094
useNotification 훅의 useEffect 내부를 확인하기 위해 아래 스크립트를 실행해주세요:
🏁 Script executed:
#!/bin/bash
# useNotification.ts의 useEffect 블록 및 EventSource close 호출 여부 확인
rg "useEffect" -n src/hooks/user/useNotification.ts -A20 -B5Length of output: 1413
useNotification 훅의 전체 cleanup 로직을 확인하기 위해 아래 스크립트를 실행해주세요:
🏁 Script executed:
#!/bin/bash
# useNotification.ts의 return cleanup 블록 확인
rg "return \\(\\) =>" -n src/hooks/user/useNotification.ts -A20 -B5Length of output: 67
useNotification 훅의 useEffect 전체 블록(cleanup 반환 함수 포함)을 확인하기 위해 아래 스크립트를 실행해주세요:
🏁 Script executed:
#!/bin/bash
# useNotification.ts의 useEffect 블록 전체 출력 (cleanup 포함)
rg "useEffect" -n src/hooks/user/useNotification.ts -A150 -B5Length of output: 2278
useNotification 훅에 SSE 연결 정리(cleanup) 로직 추가가 필요합니다.
현재 useEffect 내부에 EventSource를 생성만 하고, 컴포넌트 언마운트 시나 의존성 변경 시 연결을 닫는 반환 함수가 없습니다. 이로 인해 메모리 누수가 발생할 수 있습니다.
다음 부분에 cleanup 반환 함수를 추가해주세요.
- 파일:
src/hooks/user/useNotification.ts - 위치:
useEffect(() => { … }, [queryClient, userId]);내부 끝부분
예시 수정안:
useEffect(() => {
if (!userId) {
if (eventSourceRef.current) {
eventSourceRef.current.close();
eventSourceRef.current = null;
}
return;
}
if (eventSourceRef.current) {
return;
} else {
const eventSource = new EventSource(/* … */);
eventSourceRef.current = eventSource;
// 이벤트 핸들러 등록…
}
+ // 컴포넌트 언마운트 또는 userId/queryClient 변경 시 SSE 연결 닫기
+ return () => {
+ if (eventSourceRef.current) {
+ eventSourceRef.current.close();
+ eventSourceRef.current = null;
+ }
+ };
}, [queryClient, userId]);위 변경으로 언마운트 시점 및 의존성 변경 시 EventSource가 정상적으로 종료됩니다.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/hooks/user/useNotification.ts around the useEffect block with
dependencies [queryClient, userId], the EventSource connection is created but
lacks a cleanup function to close it on component unmount or dependency change.
To fix this, add a return cleanup function inside the useEffect that calls
EventSource.close() to properly close the connection and prevent memory leaks.
| useEffect(() => { | ||
| testLiveAlarm(); | ||
| }, []); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
프로덕션에서 자동 테스트 알림 호출 확인 필요
헤더 컴포넌트가 마운트될 때마다 testLiveAlarm()이 자동으로 호출됩니다. 이는 프로덕션 환경에서 의도하지 않은 알림을 발송할 수 있습니다.
개발/테스트 환경에서만 실행되도록 제한하거나, 다른 트리거 방식을 고려해보세요.
다음 스크립트로 다른 컴포넌트에서도 비슷한 패턴이 있는지 확인해보겠습니다:
🏁 Script executed:
#!/bin/bash
# testLiveAlarm 함수의 다른 사용처 확인
rg -A 3 -B 3 "testLiveAlarm"Length of output: 1278
❗ 프로덕션 환경 자동 테스트 알람 호출 차단 필요
헤더 컴포넌트가 마운트될 때마다 testLiveAlarm()을 무조건 호출하고 있어, 프로덕션 배포 시 의도치 않게 알림이 발송될 수 있습니다. 개발/테스트 환경에서만 동작하도록 환경 변수를 체크하거나 별도 플래그로 제어하세요.
수정 위치:
- src/components/common/header/Header.tsx:44-46
제안 변경:
useEffect(() => {
- testLiveAlarm();
+ if (process.env.NODE_ENV === 'development') {
+ testLiveAlarm();
+ }
}, []);📝 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.
| useEffect(() => { | |
| testLiveAlarm(); | |
| }, []); | |
| // src/components/common/header/Header.tsx | |
| useEffect(() => { | |
| if (process.env.NODE_ENV === 'development') { | |
| testLiveAlarm(); | |
| } | |
| }, []); |
🤖 Prompt for AI Agents
In src/components/common/header/Header.tsx around lines 44 to 46, the useEffect
hook calls testLiveAlarm() unconditionally on component mount, which can trigger
unwanted alerts in production. Modify the code to check the environment before
calling testLiveAlarm(), ensuring it only runs in development or test
environments by using environment variables or a feature flag to conditionally
execute the function.
구현내용
하고, 실시간 알림 데이터를 contextAPI를 통해 전역에서 관리
연관이슈
close #310
Summary by CodeRabbit
신규 기능
리팩터
버그 수정