Skip to content
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

fix(@yourssu/logging-system): unload event 시 서버로 log 전송 #44

Closed
wants to merge 7 commits into from

Conversation

owl1753
Copy link
Collaborator

@owl1753 owl1753 commented Jun 24, 2024

1️⃣ 어떤 작업을 했나요? (Summary)

기존 코드에 영향을 미치지 않는 변경사항

  • 없습니다.

기존 코드에 영향을 미치는 변경사항

  • demoYLSWrapper를 추가했습니다.
  • unload 이벤트를 YLSProvider에서 useEffect를 통해 추가합니다.

버그 픽스

  • 페이지 이탈 시 unload 이벤트가 정상적으로 실행됩니다.

2️⃣ 알아두시면 좋아요!

기존의 문제점은 크게 두 가지입니다.

  • getEventListeners(window)를 찍어봤을 때 전송 이벤트가 추가되지 않았었습니다.
  • useYLSContextContext Provider 외부에서 사용합니다. (문제가 되는 지는 잘 모르겠네요...)

따라서 Provider 내부에서 useEffect 훅을 통해 이벤트 리스너를 추가하는 방식으로 변경했습니다.
결과적으로 페이지를 이탈했을 때 이벤트가 잘 실행됩니다.

3️⃣ 추후 작업

api 호출 이후에 localStorage를 비움으로써 생기는 문제점에 대해 의논한 후, 중복 로그 전송 문제를 해결

4️⃣ 체크리스트 (Checklist)

  • main 브랜치의 최신 코드를 pull 받았나요?

@owl1753 owl1753 added fix YLS @yourssu/logging-system-react labels Jun 24, 2024
@owl1753 owl1753 self-assigned this Jun 24, 2024
@owl1753 owl1753 changed the title fix: unload event 시 log 서버로 전송 fix: unload event 시 서버로 log 전송 Jun 24, 2024
@owl1753 owl1753 changed the title fix: unload event 시 서버로 log 전송 fix: unload event 시 서버로 log 전송 Jun 24, 2024
Copy link
Member

@intersoom intersoom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수고하셨습니다 >< !!
코멘트는 제 생각일 뿐이니까 답변 주세요 !!
그리구,, 제가 머지한 이전 PR에서 lock 파일이 업데이트가 안되어서 ,, 해당 PR에 hoxy lock file update 커밋을 슬쩍 껴주실 수 있을까요 🥹

apps/demo/src/App.tsx Outdated Show resolved Hide resolved
packages/logging-system/src/Logger.ts Outdated Show resolved Hide resolved
packages/logging-system/src/Logger.ts Show resolved Hide resolved
packages/logging-system/src/contexts/YLSProvider.tsx Outdated Show resolved Hide resolved
@owl1753 owl1753 requested a review from intersoom June 25, 2024 05:38
Copy link
Member

@Hanna922 Hanna922 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

제롬 혹시 demo에서 창 닫았을 때 로그 잘 보내지나요? 저는 왜 그대로,,,,일까요

Comment on lines +55 to +59
window.addEventListener('unload', handleUnload);

return () => {
window.removeEventListener('unload', handleUnload);
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
window.addEventListener('unload', handleUnload);
return () => {
window.removeEventListener('unload', handleUnload);
};
window.addEventListener('unload', handleUnload, { once: true });

@owl1753
Copy link
Collaborator Author

owl1753 commented Jun 25, 2024

제롬 혹시 demo에서 창 닫았을 때 로그 잘 보내지나요? 저는 왜 그대로,,,,일까요

baseURL이 다르게 설정되어 있어서 요청이 실패해서 그런 것 아닐까요...? 로컬 스토리지를 먼저 비우는 것으로 바꿔서 테스트 해봤을 때 로컬 스토리지가 잘 비워지는 것으로 봤었습니당

@Hanna922
Copy link
Member

오.. 아뇨 저 baseURL 지금 YLS 서버로 보내고 있고 요청은 성공해요
요거 이탈 후에 재진입 시 로그가 로컬에 남아있으면 안 되는거 맞죠..?

@owl1753
Copy link
Collaborator Author

owl1753 commented Jun 25, 2024

오.. 아뇨 저 baseURL 지금 YLS 서버로 보내고 있고 요청은 성공해요

요거 이탈 후에 재진입 시 로그가 로컬에 남아있으면 안 되는거 맞죠..?

다시 확인해봐야겠네유...

@owl1753
Copy link
Collaborator Author

owl1753 commented Jun 26, 2024

@Hanna922 혹시 테스트할 때 새로고침 하는 상황에서도 로그 전송이 이루어졌나요? 저는 새로고침할 때도 unload event가 발생하네요.... 그리고 지금 혹시 CORS 에러 뜨지 않나용?

@Hanna922 Hanna922 changed the title fix: unload event 시 서버로 log 전송 fix(@yourssu/logging-system): unload event 시 서버로 log 전송 Jun 26, 2024
@Hanna922 Hanna922 closed this Jun 26, 2024
@Hanna922 Hanna922 deleted the fix/#43-not-sent-log branch June 26, 2024 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix YLS @yourssu/logging-system-react
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: 페이지 이탈 시 로그 전송 안되는 현상
3 participants