[공통] Storage 접근 제한 환경에서 클라이언트 예외 발생#1260
Merged
Merged
Conversation
Walkthrough전체 앱에서 직접 ChangesIsomorphic Storage 마이그레이션
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
src/pages/store/[id].tsx (1)
157-206: ⚡ Quick winduration_time 계산 로직 중복 개선을 고려해보세요.
동일한 패턴의 duration_time 계산이 여러 곳에서 반복되고 있습니다:
- Line 165, 172, 195, 204:
(new Date().getTime() - Number(isomorphicSessionStorage.getItem('...'))) / 1000
components/Store/utils/durationTime.ts에 이미 duration 관련 유틸이 있으니, 다음과 같은 헬퍼 함수를 추가하여 중복을 제거할 수 있습니다:// components/Store/utils/durationTime.ts에 추가 export const getDurationFromStorageKey = (key: string): number => { const startTime = isomorphicSessionStorage.getItem(key); if (!startTime) return 0; return (new Date().getTime() - Number(startTime)) / 1000; };이후 사용 시:
duration_time: getDurationFromStorageKey('enterReviewPage')현재 PR은 storage 마이그레이션이 주 목적이므로 이 개선은 선택적으로 진행하셔도 됩니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/store/`[id].tsx around lines 157 - 206, Extract the repeated duration_time calculation into a helper (e.g., getDurationFromStorageKey) in components/Store/utils/durationTime.ts and replace inline expressions in onClickCallNumber and onClickList (and any other places using (new Date().getTime() - Number(isomorphicSessionStorage.getItem('...'))) / 1000) with duration_time: getDurationFromStorageKey('<storageKey>'); ensure the helper reads the given storage key from isomorphicSessionStorage, returns 0 when missing, and is imported where onClickCallNumber and onClickList are defined so you only call getDurationFromStorageKey('enterReviewPage') and getDurationFromStorageKey('enter_storeDetail') respectively.src/components/TimetablePage/MainTimetablePage/DefaultPage/index.tsx (1)
21-44: ⚡ Quick winduration_time 계산에 fallback 패턴 추가를 권장합니다.
Lines 30, 42에서
Number(isomorphicSessionStorage.getItem('enterTimetablePage'))를 사용하는데,null이 반환되면Number(null) = 0이 되어 부정확한 duration 값이 계산됩니다.♻️ 제안하는 수정안
export default function DefaultPage({ timetableFrameId, setCurrentFrameId }: DefaultPageProps) { const router = useRouter(); const logger = useLogger(); const handlePopState = React.useCallback(() => { + const currentTime = new Date().getTime(); + const enterTime = Number(isomorphicSessionStorage.getItem('enterTimetablePage')) || currentTime; + const durationTime = (currentTime - enterTime) / 1000; + // swipe로 뒤로가기 시 if (isomorphicSessionStorage.getItem('swipeToBack') === 'true') { logger.actionEventSwipe({ team: 'USER', event_label: 'timetable_back', value: 'OS스와이프', previous_page: '시간표', current_page: '메인', - duration_time: (new Date().getTime() - Number(isomorphicSessionStorage.getItem('enterTimetablePage'))) / 1000, + duration_time: durationTime, }); history.back(); return; } // 브라우저의 뒤로가기 버튼 클릭 시 / 마우스 사이드 버튼 누를 시 logger.actionEventClick({ team: 'USER', event_label: 'timetable_back', value: '뒤로가기버튼', previous_page: '시간표', current_page: '메인', - duration_time: (new Date().getTime() - Number(isomorphicSessionStorage.getItem('enterTimetablePage'))) / 1000, + duration_time: durationTime, }); }, [logger]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/TimetablePage/MainTimetablePage/DefaultPage/index.tsx` around lines 21 - 44, handlePopState computes duration_time using Number(isomorphicSessionStorage.getItem('enterTimetablePage')) which yields 0 when the key is missing; update handlePopState to defensively parse the stored timestamp with a fallback (e.g., const enter = Number(isomorphicSessionStorage.getItem('enterTimetablePage')) || Date.now(); or check for null/NaN) and use (Date.now() - enter) / 1000 for duration_time in both logger.actionEventSwipe and logger.actionEventClick so invalid or missing session values don't produce misleading durations.src/components/TimetablePage/components/MainTimetable/index.tsx (1)
139-151: ⚡ Quick winduration_time 계산에 fallback 패턴 추가를 권장합니다.
Line 147에서
Number(isomorphicSessionStorage.getItem('enterTimetablePage'))를 사용하는데,null이 반환되는 경우 부정확한 duration 값이 계산됩니다.durationTime.ts의getTime함수처럼 fallback 패턴을 적용하는 것을 권장합니다.♻️ 제안하는 수정안
const onClickDownloadImage = (e: React.MouseEvent<HTMLButtonElement>) => { e.stopPropagation(); if (checkSemesterAndTimetable(mySemester, timeTableFrameList)) { + const currentTime = new Date().getTime(); + const enterTime = Number(isomorphicSessionStorage.getItem('enterTimetablePage')) || currentTime; logger.actionEventClick({ team: 'USER', event_label: 'timetable', value: '이미지저장', - duration_time: (new Date().getTime() - Number(isomorphicSessionStorage.getItem('enterTimetablePage'))) / 1000, + duration_time: (currentTime - enterTime) / 1000, }); openModal(); } };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/TimetablePage/components/MainTimetable/index.tsx` around lines 139 - 151, The duration_time calculation in onClickDownloadImage uses Number(isomorphicSessionStorage.getItem('enterTimetablePage')) which can be null and produce NaN/incorrect values; update onClickDownloadImage to read the stored timestamp into a variable (using the same fallback pattern as durationTime.getTime), e.g., parse the value and if it's falsy use Date.now() (or 0) as a fallback, then compute duration_time using that safe start value and pass it into logger.actionEventClick so the computed duration is always valid.src/components/Store/StoreReviewPage/ReviewForm/ReviewForm.tsx (1)
69-70: ⚡ Quick winduration_time 계산에 fallback 패턴 추가를 권장합니다.
isomorphicSessionStorage.getItem('enterReview')가null을 반환하는 경우(storage 차단 환경 또는 값 미설정),Number(null)은0이 되어 현재 타임스탬프 전체가 duration으로 계산됩니다. 이는 부정확한 analytics 데이터를 전송할 수 있습니다.
durationTime.ts의getTime함수처럼 fallback 패턴을 적용하는 것을 권장합니다.♻️ 제안하는 수정안
- const getReviewDurationTime = - (new Date().getTime() - Number(isomorphicSessionStorage.getItem('enterReview'))) / 1000; + const currentTime = new Date().getTime(); + const enterTime = Number(isomorphicSessionStorage.getItem('enterReview')) || currentTime; + const getReviewDurationTime = (currentTime - enterTime) / 1000;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Store/StoreReviewPage/ReviewForm/ReviewForm.tsx` around lines 69 - 70, getReviewDurationTime currently computes duration with Number(isomorphicSessionStorage.getItem('enterReview')) which treats null as 0 and yields incorrect durations; update the calculation in the getReviewDurationTime definition to apply a fallback when getItem('enterReview') is null/invalid (e.g., parse value safely and default to Date.now() or undefined start timestamp), e.g., read const stored = isomorphicSessionStorage.getItem('enterReview'), convert with a guarded parser (Number(stored) only if stored != null and !isNaN(Number(stored))) or call the existing getTime utility, and use that fallback to avoid treating null as 0 so the computed duration_time is accurate and consistent with durationTime.ts's getTime behavior.src/utils/ts/env.ts (1)
15-23: ⚡ Quick win원시
Storageexport는 안전 래퍼를 우회하게 만듭니다.
getBrowserStorage를 공개 API로 두면 외부 코드가 다시storage.getItem/setItem을 직접 호출하면서 여기의try/catch보장을 건너뛸 수 있습니다. 이번 PR 목표가 직접 storage 접근을 래퍼 내부로 모으는 것이라면, 이 헬퍼는 내부 구현으로만 두는 편이 안전합니다.♻️ Proposed fix
-export const getBrowserStorage = (variant: StorageVariant): Storage | null => { +const getBrowserStorage = (variant: StorageVariant): Storage | null => { if (!isBrowser) return null; try { return variant === 'local' ? window.localStorage : window.sessionStorage; } catch { return null; } };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/utils/ts/env.ts` around lines 15 - 23, getBrowserStorage currently returns the raw Storage object (using StorageVariant, isBrowser and window.localStorage/window.sessionStorage), which allows external code to bypass the wrapper's try/catch guarantees; make getBrowserStorage internal only (remove or unexport it) and ensure all external storage access goes through the safe wrapper functions (e.g., the module's get/set helpers that call getBrowserStorage internally), updating any consumers to use those safe helpers instead of calling storage.getItem/setItem directly so the try/catch protection is enforced.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/layout/Header/MobileHeader/Panel/index.tsx`:
- Around line 66-67: The duration_time calculation in the Panel component
unconditionally does (new Date().getTime() -
Number(isomorphicSessionStorage.getItem('enterTimetablePage'))) / 1000 which
yields a wrong value when the storage item is missing; instead, read the raw
value from isomorphicSessionStorage.getItem('enterTimetablePage'), parse/convert
it to a Number with a guard (check for null/undefined and isFinite/!isNaN and
positive), and only compute (Date.now() - parsed) / 1000 when valid; otherwise
set duration_time to a safe fallback (e.g., 0 or null). Ensure you update the
assignment where duration_time is set so it uses the guarded parsed value rather
than Number(...) directly.
In `@src/utils/ts/env.ts`:
- Around line 57-61: The current setJSONItem function skips doing anything when
stringifyStorageValue(value) returns null (e.g., value is undefined), leaving
any previous value intact; change setJSONItem so that when serializedValue ===
null it calls storageAdapter.removeItem(key) (instead of returning) to
explicitly delete the key; reference the setJSONItem function and the
stringifyStorageValue and storageAdapter.setItem/removeItem calls when making
the change.
---
Nitpick comments:
In `@src/components/Store/StoreReviewPage/ReviewForm/ReviewForm.tsx`:
- Around line 69-70: getReviewDurationTime currently computes duration with
Number(isomorphicSessionStorage.getItem('enterReview')) which treats null as 0
and yields incorrect durations; update the calculation in the
getReviewDurationTime definition to apply a fallback when getItem('enterReview')
is null/invalid (e.g., parse value safely and default to Date.now() or undefined
start timestamp), e.g., read const stored =
isomorphicSessionStorage.getItem('enterReview'), convert with a guarded parser
(Number(stored) only if stored != null and !isNaN(Number(stored))) or call the
existing getTime utility, and use that fallback to avoid treating null as 0 so
the computed duration_time is accurate and consistent with durationTime.ts's
getTime behavior.
In `@src/components/TimetablePage/components/MainTimetable/index.tsx`:
- Around line 139-151: The duration_time calculation in onClickDownloadImage
uses Number(isomorphicSessionStorage.getItem('enterTimetablePage')) which can be
null and produce NaN/incorrect values; update onClickDownloadImage to read the
stored timestamp into a variable (using the same fallback pattern as
durationTime.getTime), e.g., parse the value and if it's falsy use Date.now()
(or 0) as a fallback, then compute duration_time using that safe start value and
pass it into logger.actionEventClick so the computed duration is always valid.
In `@src/components/TimetablePage/MainTimetablePage/DefaultPage/index.tsx`:
- Around line 21-44: handlePopState computes duration_time using
Number(isomorphicSessionStorage.getItem('enterTimetablePage')) which yields 0
when the key is missing; update handlePopState to defensively parse the stored
timestamp with a fallback (e.g., const enter =
Number(isomorphicSessionStorage.getItem('enterTimetablePage')) || Date.now(); or
check for null/NaN) and use (Date.now() - enter) / 1000 for duration_time in
both logger.actionEventSwipe and logger.actionEventClick so invalid or missing
session values don't produce misleading durations.
In `@src/pages/store/`[id].tsx:
- Around line 157-206: Extract the repeated duration_time calculation into a
helper (e.g., getDurationFromStorageKey) in
components/Store/utils/durationTime.ts and replace inline expressions in
onClickCallNumber and onClickList (and any other places using (new
Date().getTime() - Number(isomorphicSessionStorage.getItem('...'))) / 1000) with
duration_time: getDurationFromStorageKey('<storageKey>'); ensure the helper
reads the given storage key from isomorphicSessionStorage, returns 0 when
missing, and is imported where onClickCallNumber and onClickList are defined so
you only call getDurationFromStorageKey('enterReviewPage') and
getDurationFromStorageKey('enter_storeDetail') respectively.
In `@src/utils/ts/env.ts`:
- Around line 15-23: getBrowserStorage currently returns the raw Storage object
(using StorageVariant, isBrowser and window.localStorage/window.sessionStorage),
which allows external code to bypass the wrapper's try/catch guarantees; make
getBrowserStorage internal only (remove or unexport it) and ensure all external
storage access goes through the safe wrapper functions (e.g., the module's
get/set helpers that call getBrowserStorage internally), updating any consumers
to use those safe helpers instead of calling storage.getItem/setItem directly so
the try/catch protection is enforced.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 19d6c6a1-c353-4406-b086-696c773cd0b7
📒 Files selected for processing (29)
src/components/Course/hooks/useSelectedCourses.tssrc/components/IndexComponents/IndexCafeteria/index.tsxsrc/components/Store/StoreDetailPage/components/Review/components/ReviewButton/index.tsxsrc/components/Store/StoreDetailPage/components/Review/index.tsxsrc/components/Store/StoreReviewPage/ReviewForm/ReviewForm.tsxsrc/components/Store/utils/durationTime.tssrc/components/TimetablePage/MainTimetablePage/DefaultPage/index.tsxsrc/components/TimetablePage/components/MainTimetable/index.tsxsrc/components/TimetablePage/hooks/useTimetableMutation.tssrc/components/layout/Header/MobileHeader/Panel/index.tsxsrc/components/layout/Header/MobileHeader/index.tsxsrc/components/layout/Header/PCHeader/index.tsxsrc/components/ui/IntroToolTip/index.tsxsrc/pages/_app.tsxsrc/pages/auth/modifyinfo/index.tsxsrc/pages/graduation/index.tsxsrc/pages/store/[id].tsxsrc/pages/store/index.tsxsrc/pages/timetable/index.tsxsrc/utils/hooks/abTest/useABTestView.tssrc/utils/hooks/auth/useAuth.tssrc/utils/hooks/auth/useAutoLogin.tssrc/utils/hooks/auth/useLogout.tssrc/utils/hooks/state/useLocalStorage.tssrc/utils/hooks/state/useWebStorage.tssrc/utils/ts/apiClient.tssrc/utils/ts/auth.tssrc/utils/ts/env.tssrc/utils/zustand/myLectures.ts
Comment on lines
+66
to
67
| duration_time: (new Date().getTime() - Number(isomorphicSessionStorage.getItem('enterTimetablePage'))) / 1000, | ||
| }); |
There was a problem hiding this comment.
duration_time 기본값 방어가 필요합니다.
Line 66에서 storage 값이 없거나 접근이 막히면 Number(null)이 0이 되어 비정상적으로 큰 duration_time이 기록됩니다. fallback 계산을 분리해 주세요.
제안 코드
+ const enteredAt = Number(isomorphicSessionStorage.getItem('enterTimetablePage'));
+ const durationTime = Number.isFinite(enteredAt) && enteredAt > 0 ? (Date.now() - enteredAt) / 1000 : 0;
logger.actionEventClick({
team: 'USER',
event_label: 'timetable_back',
value: '햄버거',
previous_page: '시간표',
current_page: title,
- duration_time: (new Date().getTime() - Number(isomorphicSessionStorage.getItem('enterTimetablePage'))) / 1000,
+ duration_time: durationTime,
});🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/layout/Header/MobileHeader/Panel/index.tsx` around lines 66 -
67, The duration_time calculation in the Panel component unconditionally does
(new Date().getTime() -
Number(isomorphicSessionStorage.getItem('enterTimetablePage'))) / 1000 which
yields a wrong value when the storage item is missing; instead, read the raw
value from isomorphicSessionStorage.getItem('enterTimetablePage'), parse/convert
it to a Number with a guard (check for null/undefined and isFinite/!isNaN and
positive), and only compute (Date.now() - parsed) / 1000 when valid; otherwise
set duration_time to a safe fallback (e.g., 0 or null). Ensure you update the
assignment where duration_time is set so it uses the guarded parsed value rather
than Number(...) directly.
Comment on lines
+57
to
+61
| setJSONItem(key: string, value: unknown): void { | ||
| const serializedValue = stringifyStorageValue(value); | ||
| if (serializedValue === null) return; | ||
|
|
||
| storageAdapter.setItem(key, serializedValue); |
There was a problem hiding this comment.
undefined 저장 시 이전 값이 그대로 남습니다.
Line 58에서 JSON.stringify(undefined)는 문자열을 만들지 못해서 현재 분기상 아무 작업도 하지 않습니다. 기존 키가 있던 상태에서 setJSONItem(key, undefined)를 호출하면 저장값이 지워지지 않아 다음 읽기에서 stale 값이 복원됩니다. undefined는 명시적으로 removeItem 처리하는 편이 안전합니다.
🛡️ Proposed fix
setJSONItem(key: string, value: unknown): void {
+ if (value === undefined) {
+ storageAdapter.removeItem(key);
+ return;
+ }
+
const serializedValue = stringifyStorageValue(value);
if (serializedValue === null) return;
storageAdapter.setItem(key, serializedValue);
},As per coding guidelines, src/utils/**: "엣지 케이스(null, undefined, 빈 배열 등) 처리가 되어 있는지 확인해주세요."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/utils/ts/env.ts` around lines 57 - 61, The current setJSONItem function
skips doing anything when stringifyStorageValue(value) returns null (e.g., value
is undefined), leaving any previous value intact; change setJSONItem so that
when serializedValue === null it calls storageAdapter.removeItem(key) (instead
of returning) to explicitly delete the key; reference the setJSONItem function
and the stringifyStorageValue and storageAdapter.setItem/removeItem calls when
making the change.
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
2 participants
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.
What is this PR? 🔍
Changes 📝
localStorage/sessionStorage접근 시SecurityError가 발생해도 앱이 크래시되지 않도록 안전 래퍼를 적용했습니다.getJSONItem,setJSONItem을 추가해 storage JSON 파싱/직렬화 실패를 공통 처리하도록 정리했습니다.localStorage/sessionStorage접근을 안전 래퍼 사용으로 통일했습니다.ScreenShot 📷
Test CheckList ✅
yarn tsc --noEmityarn lint:eslintlocalStorage/sessionStorage직접 접근이 wrapper 내부에만 남는지 확인Precaution
yarn lint:eslint실행 시 기존 Browserslist caniuse-lite outdated 경고가 출력됩니다.✔️ Please check if the PR fulfills these requirements
developbranch unconditionally?main?yarn lintSummary by CodeRabbit
릴리스 노트