[fix] 카드 생성의 비재시도 예외가 실패 로그·상태 전환을 건너뛴다 - #179
Conversation
recordCard 의 error 파라미터가 CardGenerationFailedException 만 받아, 재시도 대상이 아닌 예외로 중단될 때는 원인을 실을 자리조차 없었다. Throwable 로 넓혀 두 경우가 같은 자리에 기록되게 한다. 동작은 바뀌지 않는다 - 지금 부르는 곳은 onExhausted 뿐이다. 다음 커밋에서 onNonRetryable 이 이 자리를 쓴다.
extractEmotion·generateMessage 가 LlmRetryExecutor 에 onExhausted 만 넘기고 onNonRetryable 은 넘기지 않았다. 재시도 대상이 아닌 예외로 중단되면 실패 생성 로그가 남지 않고 cardGenerationStatus 도 PENDING 그대로 남는다. 그러면 사용자의 재시도가 재시도 가능한 503 이 아니라 409(생성 중)로 막힌다 - PendingCardCleanupScheduler 가 되돌릴 때까지 최대 3분(PT3M)이다. 같은 리팩터링을 받은 CommentGenerationService 는 두 호출부 모두 onNonRetryable = recordFailure 를 넘긴다. 카드 쪽만 빠져 있었다. 두 콜백이 남길 것이 같아 failureRecorder 로 묶었다. 하나만 넘겨 나머지 경로가 조용히 빠지는 실수가 이 자리에서 다시 나지 않게 한다. 예외 자체는 변환하지 않고 그대로 올린다 - 예상 밖 결함을 업무 오류로 위장하지 않으려는 것이고, CommentGenerationService 도 같은 계약이다.
두 호출부(감정 분류·대사 생성)에서 재시도 대상이 아닌 예외가 나올 때 FAILED 로 전이하는지, 실패 생성 로그에 원인 클래스명이 남는지, 재시도하지 않고 원래 예외를 그대로 올리는지 확인한다. 앞 커밋의 onNonRetryable 을 되돌리고 돌리면 두 테스트만 실패한다 (23 tests completed, 2 failed) - 나머지 21개는 그대로 통과해 기존 계약을 건드리지 않았음도 함께 확인했다.
|
Warning Review limit reached
Next review available in: 47 minutes Limit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. Walkthrough카드 생성의 감정 분류와 문구 생성에서 비재시도 예외도 실패 처리 대상이 된다. 공통 콜백은 실패 로그를 기록하고 생성 상태를 Changes카드 생성 실패 처리
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: 🔵 Low · up to 비재시도 오류 처리로 실패 로그와 FAILED 상태 전환을 보완하지만, 실패 정리 작업 자체가 예외를 던지면 원래 오류가 가려지거나 상태가 PENDING으로 남아 사용자의 재시도가 일시적으로 막힐 수 있습니다. 범위가 제한적이므로 담당자 확인을 전제로 병합 가능합니다. Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
Test Coverage
|
|
잘 정리해주셨네요! 콜백 하나만 넘겨서 생긴 문제라 두 개를
에 대해서는 예외를 변환하지 않는 쪽 그대로 두시면 될 것 같습니다. 503을 내리면 클라이언트측에서 계속 재시도할 수 있을 것 같아서요. |
|
그리고 같은 패턴이 |
|
마지막으로 PR의 "최대 3분"은 실제로 4분에 가까울 것 같아요. |
recordFailure 는 CommentGenerationService 의 같은 자리와 이름이 겹치는데 하는 일이 다르다. 그쪽은 생성 로그만 남기고 FAILED 전이는 바깥 catch 가 맡지만, 여기는 상태 복구까지 한다. 이름이 같으면 두 서비스가 같은 일을 한다고 읽힌다. failureRecorder -> failureHandler, recordFailure -> handleFailure 로 바꾸고 KDoc 에 차이를 적었다.
persistCard 가 DataIntegrityViolationException 과 CardGenerationStateConflictException 만 잡아, 커넥션 끊김이나 쿼리 타임아웃으로 저장이 실패하면 예외가 그대로 나가고 상태는 PENDING 으로 남았다. 이 자리는 CAS 선점 이후라 재시도가 재시도 가능한 503 이 아니라 409(생성 중)로 막힌다 - 앞 커밋들이 고친 것과 같은 결함이다. 같은 파일의 loadUserMessages 는 DataAccessException 을 이미 막아뒀는데 저장 경로만 빠져 있었다. 상태만 되돌리고 예외는 변환하지 않고 그대로 올린다 - failureHandler 와 같은 계약이다. 구현을 되돌리면 새 테스트만 실패한다(24 tests completed, 1 failed).
2 participants
Closes #178
무엇을 고쳤나
CardService의 두 LLM 호출부가LlmRetryExecutor.execute에onExhausted만 넘기고onNonRetryable은 넘기지 않았다. 같은 리팩터링을 받은 형제 서비스와 어긋나 있던 자리다.CommentGenerationService:257(댓글)CommentGenerationService:331(답글)CardServiceextractEmotionCardServicegenerateMessage빠진 쪽에서 재시도 대상이 아닌 예외로 중단되면 두 가지가 어긋났다.
LlmRetryExecutorKDoc 이 스스로 밝힌 존재 이유가 "마지막 한 시도만 기록해 과소 집계한 버그가 있던 자리" 인데, 그 문제가 카드 경로에서 다시 열려 있었다.cardGenerationStatus가 PENDING 으로 남는다. CAS 선점 이후라 FAILED 로 되돌리지 않으면 사용자의 재시도가 재시도 가능한 503 이 아니라 409CARD_GENERATION_IN_PROGRESS로 막힌다.PendingCardCleanupScheduler가 되돌릴 때까지 최대 4분 가까이 걸린다 — 임계pending-generation-timeout: PT3M에 스캔 주기(fixedDelay = 60_000)가 더해져, 임계를 막 넘긴 직후 스캔을 놓치면 1분을 더 기다린다.같은 파일의
loadUserMessages가DataAccessException에 대해 이미 이 위험을 막고 있다 — "이 조회는 CAS 선점 이후라 실패를 그대로 던지면 상태가 PENDING으로 남아, 재시도가 재시도 가능한 503이 아니라 409(생성 중)로 막힌다". 패턴은 이미 인지하고 있었는데 재시도 실행기 경로에만 적용되지 않았다.어떻게 고쳤나
커밋 3개로 나눴다.
refactor—recordCard의error파라미터를CardGenerationFailedException?→Throwable?로 넓혔다. 비재시도 예외의 원인을 실을 자리를 먼저 연다. 동작 변화 없음fix— 두 호출부에onNonRetryable추가test— 검증두 콜백을 각각 인라인으로 넣는 대신
failureRecorder로 묶었다. 콜백 하나만 넘겨 나머지 경로가 조용히 빠지는 것이 이번 결함의 원인 그 자체라, 같은 실수가 이 자리에서 다시 나지 않게 하는 편이 낫다고 봤다.검증 — 되돌려서 실제로 잡히는지 확인했다
구현(
fix커밋)만 되돌리고 테스트를 돌렸다.새 테스트 2개만 실패하고 기존 21개는 그대로 통과한다 — 테스트가 실제로 이 결함을 잡는다는 것과, 기존 계약을 건드리지 않았다는 것을 함께 확인했다.
리뷰 포인트 — 예외를 변환하지 않는다
비재시도 예외는
CARD_GENERATION_FAILED로 변환하지 않고 원래 예외를 그대로 올린다. 상태 복구와 실패 로그만 같은 계약으로 받는다.LlmRetryExecutorKDoc: "어느 쪽이든 마지막에는 원래 예외를 그대로 던진다 — 호출자가 예외 타입으로 분기하는 기존 동작을 바꾸지 않기 위해서다"CommentGenerationService도 같은 계약이라 형제 서비스와 맞춘다대신 클라이언트는 이 경우 500 을 받는다.
extractEmotionKDoc 의 "어느 쪽이 실패했든 CARD_GENERATION_FAILED 하나로 재시도한다" 와는 어긋나는 면이 있어 KDoc 에 이 결정을 명시해 뒀다. 모든 실패를CARD_GENERATION_FAILED로 묶는 쪽이 낫다는 의견이면 알려주세요 — 다만 API 계약 변경이라 앱 팀과 맞춰야 한다.영향 범위
GeminiEmotionExtractor·GeminiCardMessageGenerator가 LLM 호출을catch (e: Exception)으로 감싸 전부 재시도 대상 예외로 바꾸고,GenerationLogRecorder.record는runCatching으로 전부 삼킨다. 현실적인 탈출구는 try 바깥의 SDK 호출(response.text(),usageMetadata()등)뿐이다배경
#177(prod 승격) 에 CodeRabbit 이 남긴 리뷰 8건 중 유일하게 반영이 필요하다고 판단한 건이다. 나머지 7건은 각 스레드에 근거를 남기고 resolve 했다.
Summary by CodeRabbit
버그 수정
FAILED로 변경됩니다.테스트