-
Notifications
You must be signed in to change notification settings - Fork 1
[25.02.28 / TASK-116,TASK-103] Modify - 캐시 충돌 문제 해결 #18
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
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 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:
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 (
|
} catch (err: unknown) { | ||
const context = err as Response; | ||
if (location && !context.ok && context.status === 403) { | ||
if (location && !context.ok && context.status === 401) { |
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.
일단 이걸 fix 한게 아니라 403도 넣을까요? API 쪽에서 대응개발한게 아니다 보니..
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.
@cheonHwi 님 생각도 궁금해요!
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.
@cheonHwi 님 생각도 궁금해요!
403도 넣어두는 방향이 괜찮을것 같아요! API쪽도 나중에 확인해볼게요
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.
좋았던 점
- 로그아웃을 캐싱 클리어 기점 잡는거 좋았습니다! 함수 관심사 분리, utils 에 뺀것도 좋았어요
- 401, 403 대응같이 저희가 더 디테일한 부분을 더 신경써야 겠다 싶네요!
아쉬운 점
- utils 는 잘 아시다시피 함수가 길을 잃어버리는 경우가 많더라구요, 그에 따라 오히려 jsdocs 와 같은 도큐멘테이션이 강화되면 좋을 것 같아요.
- 추가로 로그아웃안하고 다시 재로그인 하는 경우는 어떻게 되는지 궁금해요.
로그아웃되지 않은 상태에서 재로그인 하는 경우면, 토큰이 만료된게 아닌 상황이라면 그냥 로그인될 것으로 보입니다! |
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.
좋았던 점
- 캐싱 충돌 문제를 해결하는 과정과 방법을 엿볼 수 있어서 좋았습니다!
- utils 클래스를 활용해 특정 비즈니스 로직에 의존하지 않는 범용적인 코드를 구현하신 부분이 좋았습니다👍
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.
로그아웃 시점에 캐시를 없애는 부분 괜찮은 방법 같네요!
캐시 처리 로직을 따로 Util 함수로 분리한 것도 앞으로 로직이 변경될 때 좋게 작용할 것 같아요!
원래 발생하던 유저 캐시 충돌 문제를 해결하였습니다.
다만 해결과 동시에 쿼리캐시 초기화 로직을 로그인 페이지로 이동시켰는데, NextJS 서버 액션의 redirect 함수가 사실상의 return 역할을 하면서 promise나 콜백 함수를 붙일 수가 없더군요.그래서 해당 문제의 해결을 위해서 그냥 로그아웃 후 로그인 페이지로 이동했을 시의 캐시 처리는 로그인 페이지에 맡기기로 했습니다..더 좋은 방법이 있는 경우 추천해주시면 바로 반영해보겠습니다!원래는 로그인 페이지에서 캐시 초기화를 처리하는 방식으로 구현했으나, 근본적인 문제점인 me 쿼리의 refetching을 해결해서 로그인 페이지에 있던 캐시 초기화 로직은 정리하였습니다!