Skip to content

[fix/MAT-524] autosave 데이터 유실 방지#308

Closed
b0nsu wants to merge 0 commit intofix/mat-349-handwriting-api-cache-strategyfrom
fix/mat-524-autosave-data-loss
Closed

[fix/MAT-524] autosave 데이터 유실 방지#308
b0nsu wants to merge 0 commit intofix/mat-349-handwriting-api-cache-strategyfrom
fix/mat-524-autosave-data-loss

Conversation

@b0nsu
Copy link
Copy Markdown
Collaborator

@b0nsu b0nsu commented May 2, 2026

Summary

staleTime: Infinity (MAT-349 / PR #288) 도입으로 인한 필기 데이터 유실 regression 수정 — sterdsterd 리뷰 후 전면 재설계 v3까지 진행. 13개 라인 코멘트 중 11개 해결, 2개는 후속 이슈로 분리.

Linear

  • MAT-524 — 본 PR
  • MAT-542 — C11 base64 wrapping 제거 (후속, 서버 합의 필요)
  • MAT-543 — C12 autosave debounce (후속)

Changes

신규

  • features/student/scrap/hooks/saveCoordinator.ts — module-level pendingSaves Map + refireInFlight Set + mutationCache.subscribe 기반 cleanup. TanStack Query v5 per-call callback unmount drop bug 우회.
  • features/student/scrap/hooks/__tests__/saveCoordinator.test.ts — 단위 테스트 11개 (enqueue / coalesce / refire 성공/실패 / scrapId 독립성).

수정

  • useHandwritingManager.ts전면 재작성:
    • Derived loadStatus (no useReducer), decodeError / canvasTimeoutError / appliedTick state
    • Fire-and-forget flushIfDirty(scrapId) (sync, no await)
    • mutationCache.subscribe로 mutation success/error 감지 → currentEncode === response.data 비교 후 markAsSaved (sterdsterd C1)
    • Load effect의 hasUnsavedRef guard로 pending 중 사용자 stroke 보존
    • raf 기반 canvas mount polling (max 30 frames)
    • useFocusEffect invalidate
  • ScrapDetailScreen.tsx — 호출부 정리:
    • scrapId NaN 가드
    • await handleSave 패턴 모두 제거 → flushIfDirty + navigation
    • 100ms isSaving polling 제거
    • hook props: onSaveSuccess/onDataLoadedonSavedMatch/onSaveError
    • error overlay (drawing area 내부, retry 버튼)
  • putUpdateHandwriting.tshandwritingSaveMutationFn export + mutationKey: ['handwriting-save']
  • useGetHandwriting.tsgcTime: 1h 추가 (탭 전환 후 30분 내 재진입 시 cache hit 보장)

sterdsterd 코멘트 매핑

ID 본 PR 해결
C1 (save pending markAsSaved 오탐) ✅ mutationCache.subscribe + currentEncode 비교
C2 (initialLoadDoneRef invalidate 무시) ✅ same-data guard, 다른 데이터는 적용
C3 (decode 실패 빈 화면 덮어씀) ✅ decodeError state + PUT 차단 + error overlay
C4 (isSaving stale closure) ✅ ref 동기화
C5 (50ms polling 무한 루프) ✅ raf + 30 frames timeout
C6×3 (nav 막힘) ✅ fire-and-forget
C7 (autosave interval reset) ✅ flushIfDirtyRef + deps []
C8 (focus invalidate 부재) ✅ useFocusEffect
C9 (scrapId 가드) ✅ Number.isFinite + > 0
C10 (undo 같은 인코딩 무한 호출) ✅ onSavedMatch callback
C13 (scrapId 결합 4곳) ✅ hook 캡슐화 + saveCoordinator 모듈
C11 (base64 wrapping) MAT-542 (서버 합의)
C12 (debounce) MAT-543

Testing

  • pnpm typecheck 통과
  • pnpm lint 변경 파일 0 errors
  • pnpm test --testPathPatterns=saveCoordinator 11/11 통과
  • 시나리오 1: 기존 필기 스크랩 → 뒤로가기 → 재진입 → 필기 유지
  • 시나리오 2: 새 스크랩 필기 저장 → 앱 종료 → 재시작 → 필기 유지
  • 시나리오 3: 백그라운드 진입 시 즉시 PUT → 복귀 후 필기 유지
  • 시나리오 4: 탭 A 수정 → 탭 B → 30분 후 탭 A 복귀 → dirty 유지 (cache hit)
  • 시나리오 5: 탭 전환 → 필기 유지 + 즉시 전환 (nav blocking 0)
  • 시나리오 6: mutation in-flight 중 탭 전환 → 다음 진입 시 데이터 보존
  • decode 실패 시 error overlay + retry 동작
  • mutation 네트워크 실패 시 toast + dirty 유지 → 다음 autosave retry

Risk / Impact

  • 영향 범위: 스크랩 상세 화면 필기 저장/로드 (study handwriting 미적용)
  • 호환성: 기존 base64 인코더 그대로, 데이터 호환 ✅
  • 머지 순서: MAT-349 (PR fix: handwriting API 캐시 전략 개선 #288) 먼저 머지 필요 (stacked PR base)
  • Phase 3 Step 1에서 공유 hook으로 재작성될 가능성 — 본 spec에서 수용 결정

@linear
Copy link
Copy Markdown

linear Bot commented May 2, 2026

@vercel
Copy link
Copy Markdown

vercel Bot commented May 2, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
pointer-admin Ready Ready Preview, Comment May 2, 2026 7:56am

Comment on lines 112 to 115
onSuccess: () => {
console.log('[HW] saved scrapId:', saveScrapId);
lastSavedDataRef.current = base64Data;
onSaveSuccess?.();
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

save 요청 보내고 mutation이 pending일 때 필기가 더 수정되면,
여기 콜백은 요청 보낼 때 시점의 base64DatalastSavedDataRef에 넣어서,
canvas 위의 수정된 stroke/text가 남아있어도 onSaveSuccess에서 hasUnsavedChanges를 false로 만드는데,
이러면 auto save 조건이 꺼지니까 다음 수정 전까지 저장이 안 될 수도 있음.
성공 시점에서 현재 캔버스를 다시 인코딩해서 요청 시점의 base64Data랑 같을 때만 saved하고 다르면 unsaved 유지나 예약이나 뭐 그런 로직을 추가로 넣어야할듯

}, 50); // 50ms 지연으로 clear() 완료 보장

return () => clearTimeout(loadTimer);
if (isLoading || initialLoadDoneRef.current) return;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

initialLoadDoneReftrue가 되면 handwritigngData가 바뀌더라도 바로 return 됨.
staleTime을 Intinity로 뒀으면 mutation/cache 갱신을 해줘야 화면이 동기화 되는데,
이렇게 하면 같은 scrapId에 대해서 invalidate, refetch 같은거 걸어서 GET 캐시 갱신해줘도 canvas 갱신이 안될수도 있음.
로드 중 autosave 차단이랑 서버 데이터 적용 여부를 분리하든가, updatedAt, data 변경시에 unsaved 상태 고려해서 재적용이나 무시하든가 해야할듯.

Comment on lines +71 to +72
initialLoadDoneRef.current = true;
onDataLoadedRef.current?.();
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

decodeHandwritingData가 실패해도 여기서 initialLoadDoneRef를 true로 만들고, onDataLoaded가 호출됨.
이러면 빈 화면이 덮어써질수도 있음.
decode 실패해도 로드완료랑 저장 가능 상태로 두지말고 에러 상태로 넘기거나 아니면 상태를 안전하게 유지하는 로직이 필요해보임

if (handwriting.isSaving) {
// 저장이 완료될 때까지 대기
const checkSaveComplete = setInterval(() => {
if (!handwriting.isSaving) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

이거 stale closure일수도? 별도 effect로 분리하든 issaving을 ref로 동기화하든

Comment on lines +76 to +82
if (apply()) return;

const timer = setInterval(() => {
if (apply()) clearInterval(timer);
}, 50);

return () => clearInterval(timer);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

이거 canvasref 안 잡히면 계속 돌텐데 max 횟수를 정하든 raf같은걸로 돌리든... 50ms면 초당 20번인데 애매해보임.

const route = useRoute<ScrapDetailRouteProp>();
const navigation = useNavigation<NativeStackNavigationProp<StudentRootStackParamList>>();
const { id } = route.params;
const scrapId = Number(id);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

0이랑 NaN 가드좀

Comment on lines 96 to 101
if (base64Data === lastSavedDataRef.current) {
if (!isAutoSave) {
Alert.alert('알림', '변경사항이 없습니다.');
}
return Promise.resolve(true);
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

하나 쓰고 undo하면 encoding한게 lastSavedDataRef랑 같을텐데
reducer에서 undo할 때 false로 안바꿔주니까 hasunsavedchanges가 true겠지
그리고 5초 autosave 돌렸으니까 가만히 냅두면 5초마다 계속 여기 돌텐데

onsavesuccess를 호출해주든 drawingstate에 콜백을 하나 더 추가를 하든.. 해봐

const texts = canvasRef.current.getTexts();

try {
const base64Data = encodeHandwritingData(strokes || [], texts || []);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

5초마다 매번 메인 스레드에서 serializing을 돌린다? 부하가 너무 심하지 않을까?

근데 근본적으로 base64로 데이터를 저장하는 이유가 뭐야?
그냥 json object인데 그걸 왜 base64로 바꿔?
불필요한 wrapping 아닌가?

[scrapId, canvasRef, updateHandwriting, onSaveSuccess, onSaveError, isSaving]
);

// 5초마다 자동 저장
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

근데 왜 debounce 안 걸고 쌩 5초마다 자동저장 하는거임? 웬만하면 debounce를 거는게 나을거같은데 더 필요하면 stroke 개수나 시간 limit도 추가로 걸고

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

scrapId에 묶여있는 코드가 너무 많음..

  • (unmount-mount)
  • useHandwritingManager의 useEffect([scrapId]) (ref 리셋)
  • ScrapDetailScreen의 useEffect([scrapId]) + setInterval(100) polling으로 isSaving 대기 후 reset
  • 데이터 로드 effect의 setInterval(50) polling으로 canvasRef 대기

대충 찾아봐도 이정돈데 이러니까 timing이 자꾸 꼬여서 니가 apply에 polling 집어넣은거처럼 자꾸 덕지덕지 붙잖아...
scrapId 전환을 useHandwritingManager 내부 single responsibility로 묶고, screen에서는 뭐 scrapid의 hook instance만 쓰도록 하든가.. issaving 중에 전환은 큐에 넣고 ui 전환하도록 하든가.. 해봐..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants