diff --git a/src/main/kotlin/com/depromeet/piki/item/service/AsyncImageParsingWorker.kt b/src/main/kotlin/com/depromeet/piki/item/service/AsyncImageParsingWorker.kt index 930b6afe..78fd814e 100644 --- a/src/main/kotlin/com/depromeet/piki/item/service/AsyncImageParsingWorker.kt +++ b/src/main/kotlin/com/depromeet/piki/item/service/AsyncImageParsingWorker.kt @@ -47,7 +47,7 @@ class AsyncImageParsingWorker( // (부모 없는 JDBC 관측은 ObservationConfig 가 거부하므로 아예 사라진다). // 소유권 획득→등록→해제 뼈대는 guarded 가 쥔다. 획득에 실패하면 body 를 건너뛰고 스킵 로그만 남긴다 — // 특히 raw 원본 회수(deleteRaw)를 하지 않는다(소유권을 쥔 새 시도가 그 원본으로 재실행해야 하므로). deleteRaw 는 body 안에만 있다. - // 링크 워커와 같은 이유로 실패 경로가 observation.error() 를 직접 마킹한다 — runCatching 이 예외를 삼켜 + // 링크 워커와 같은 이유로 실패 경로가 observation.error() 를 직접 마킹한다 — runCatchingException 이 예외를 삼켜 // 그냥 두면 실패 span 이 정상(status 미설정)으로 남는다(#902). val observation = Observation.createNotStarted(AsyncItemParsingWorker.PARSE_OBSERVATION, observationRegistry) observation.observe { @@ -59,7 +59,7 @@ class AsyncImageParsingWorker( }, ) { attempt -> val started = System.nanoTime() - runCatching { imageSnapshotExtractor.extract(imageKey) } + runCatchingException { imageSnapshotExtractor.extract(imageKey) } .onSuccess { snapshot -> onExtracted(itemId, snapshotId, imageKey, snapshot, started, attempt, observation) } .onFailure { e -> observation.error(e) @@ -80,7 +80,7 @@ class AsyncImageParsingWorker( ) { val elapsedMs = (System.nanoTime() - started) / 1_000_000 // 일시 DB 오류(데드락·lock timeout)면 추출 재실행 없이 전이 write 만 짧게 재시도한다(TransitionRetry). - runCatching { transitionRetry.execute { itemParsingService.markReady(snapshotId, snapshot, attempt) } } + runCatchingException { transitionRetry.execute { itemParsingService.markReady(snapshotId, snapshot, attempt) } } .onSuccess { applied -> // 좀비 폐기(소유권 상실)면 전이가 스킵된다 — 결과를 성공으로 세지 않고, **특히 raw 를 지우지 않는다**. // 재클레임된 새 시도가 바로 그 원본으로 재실행해야 하므로, 여기서 지우면 되살릴 입력을 잃는다. @@ -169,7 +169,7 @@ class AsyncImageParsingWorker( // raw 원본 회수는 best-effort — 삭제 실패가 파싱 결과(이미 READY/FAILED 확정)를 되돌리지 않는다. // 회수 못 한 raw 와 recover 상한 FAILED·유실분은 items/raw/ S3 lifecycle 이 백업으로 만료한다. private fun deleteRawQuietly(imageKey: String) { - runCatching { imageStorage.delete(imageKey) } + runCatchingException { imageStorage.delete(imageKey) } .onFailure { e -> log.warn("raw 이미지 {} 회수 실패(lifecycle 이 만료): {}", imageKey, e.message) } } @@ -180,7 +180,7 @@ class AsyncImageParsingWorker( snapshotId: Long, attempt: Int, ) { - runCatching { transitionRetry.execute { itemParsingService.release(snapshotId, attempt) } } + runCatchingException { transitionRetry.execute { itemParsingService.release(snapshotId, attempt) } } .onFailure { e -> log.info("item {} 이미지 소유권 반납 생략 (이미 전이됨·소유권 상실): {}", itemId, e.message) } } @@ -191,7 +191,7 @@ class AsyncImageParsingWorker( snapshotId: Long, attempt: Int, ): Boolean = - runCatching { transitionRetry.execute { itemParsingService.markFailed(snapshotId, attempt) } } + runCatchingException { transitionRetry.execute { itemParsingService.markFailed(snapshotId, attempt) } } .onFailure { e -> when (e) { is IllegalStateException -> log.info("item {} 는 이미 전이됨, FAILED 처리 생략: {}", itemId, e.message) diff --git a/src/main/kotlin/com/depromeet/piki/item/service/AsyncItemParsingWorker.kt b/src/main/kotlin/com/depromeet/piki/item/service/AsyncItemParsingWorker.kt index b8aa58b2..121ba239 100644 --- a/src/main/kotlin/com/depromeet/piki/item/service/AsyncItemParsingWorker.kt +++ b/src/main/kotlin/com/depromeet/piki/item/service/AsyncItemParsingWorker.kt @@ -19,7 +19,7 @@ import org.springframework.stereotype.Component // 일시 외부 오류 → 소유권 반납(release, PROCESSING→PENDING)해 다음 tick 이 다시 집게 한다. // 반납은 "이 실행은 결론 없이 끝났다"는 **사실 통지**일 뿐이고, 재시도할지 종결할지의 **정책은 여전히 서비스가 쥔다** // (실행 예산이 남았으면 PENDING 으로 되돌리고, 소진했으면 그 자리에서 FAILED). -// 전이 호출(markReady/markFailed)은 runCatching 으로 감싸 워커 스레드로 예외가 새지 않게 한다 +// 전이 호출(markReady/markFailed)은 runCatchingException 으로 감싸 워커 스레드로 예외가 새지 않게 한다 // (recover 와의 레이스로 이미 전이됐거나, 추출값이 도메인 불변식을 위반하는 경우). @Component class AsyncItemParsingWorker( @@ -44,7 +44,7 @@ class AsyncItemParsingWorker( // fetch·structured·LLM span 은 traceparent 전파로 그 아래 이어져, 단건 파이프라인을 크로스서비스로 끝까지 펼쳐 볼 수 있다. // 디스패처가 @Scheduled 라 들어오는 trace 가 없어, 여기서 만들지 않으면 원격 호출 span 이 따로 떠 묶이지 않는다. // 소유권 획득→등록→해제 뼈대는 guarded 가 쥔다. 획득에 실패하면 body 를 건너뛰고 스킵 로그만 남긴다(ext 호출·부수효과 없음). - // runCatching 이 예외를 삼켜 observation 은 실패를 못 보므로, 실패 경로가 error() 로 직접 마킹해야 + // runCatchingException 이 예외를 삼켜 observation 은 실패를 못 보므로, 실패 경로가 error() 로 직접 마킹해야 // Tempo 에서 `status = error` 검색·실패 표시가 동작한다(#902). 그래서 참조를 잡아 핸들러에 넘긴다. val observation = Observation.createNotStarted(PARSE_OBSERVATION, observationRegistry) observation.observe { @@ -56,7 +56,7 @@ class AsyncItemParsingWorker( }, ) { attempt -> val started = System.nanoTime() - runCatching { productLinkExtractor.extract(link) } + runCatchingException { productLinkExtractor.extract(link) } .onSuccess { snapshot -> onExtracted(itemId, snapshotId, link, snapshot, started, attempt, observation) } .onFailure { e -> observation.error(e) @@ -78,7 +78,7 @@ class AsyncItemParsingWorker( val elapsedMs = (System.nanoTime() - started) / 1_000_000 // 전이가 실패(추출값 도메인 검증 위반·DB 오류·sweeper 와의 레이스로 이미 전이됨)해도 예외를 흡수한다. // 일시 DB 오류(데드락·lock timeout)면 추출 재실행 없이 전이 write 만 짧게 재시도한다(TransitionRetry). - runCatching { transitionRetry.execute { itemParsingService.markReady(snapshotId, snapshot, attempt) } } + runCatchingException { transitionRetry.execute { itemParsingService.markReady(snapshotId, snapshot, attempt) } } .onSuccess { applied -> // 좀비 폐기(소유권 상실)면 이 워커의 결과는 반영되지 않았다 — 결과 원장(로그·메트릭)에 성공으로 세지 않는다. // 폐기 사유 자체는 서비스가 남긴다. @@ -95,7 +95,7 @@ class AsyncItemParsingWorker( // 정체성 기록(#825 관측 단계) — READY 전이가 커밋된 뒤 별도 트랜잭션으로 canonical·별칭을 남긴다. // 전이와 분리하는 이유·병합 시 원자화 계획은 recorder 주석 참고. 기록 실패가 파싱 결과를 해치면 // 안 되므로 예외를 흡수한다(관측 부가 기능). - runCatching { itemIdentityRecorder.recordParsingIdentity(itemId, snapshot.finalUrl) } + runCatchingException { itemIdentityRecorder.recordParsingIdentity(itemId, snapshot.finalUrl) } .onFailure { e -> log.warn("item.identity.error item={} 정체성 기록 실패", itemId, e) } } .onFailure { e -> @@ -178,7 +178,7 @@ class AsyncItemParsingWorker( snapshotId: Long, attempt: Int, ) { - runCatching { transitionRetry.execute { itemParsingService.release(snapshotId, attempt) } } + runCatchingException { transitionRetry.execute { itemParsingService.release(snapshotId, attempt) } } .onFailure { e -> log.info("item {} 소유권 반납 생략 (이미 전이됨·소유권 상실): {}", itemId, e.message) } } @@ -189,7 +189,7 @@ class AsyncItemParsingWorker( snapshotId: Long, attempt: Int, ): Boolean = - runCatching { transitionRetry.execute { itemParsingService.markFailed(snapshotId, attempt) } } + runCatchingException { transitionRetry.execute { itemParsingService.markFailed(snapshotId, attempt) } } .onFailure { e -> when (e) { is IllegalStateException -> log.info("item {} 는 이미 전이됨, FAILED 처리 생략: {}", itemId, e.message) @@ -202,14 +202,15 @@ class AsyncItemParsingWorker( // 이미지 파싱(AsyncImageParsingWorker)도 같은 이름을 공유한다 — 대시보드 필터가 링크·이미지를 한 탭으로 본다. internal const val PARSE_OBSERVATION = "item.parse" - // 재시도(일시)로 볼지 판정. 치명적 JVM 오류(Error: OutOfMemory·StackOverflow 등)는 재시도해도 소용없고 - // runCatching 이 Throwable 을 다 잡아 여기로 들어오므로 먼저 제외한다(재시도 대상 아님, 즉시 종결). 분류 가능한 - // HttpMappable 은 category 로 가르고(RETRYABLE 만 재시도), 그 외 예상 못한 예외(NPE·IllegalStateException 등)는 - // 일시·영구를 단정할 수 없어 보수적으로 재시도 대상으로 둔다(즉시 FAILED 면 일시 오류를 영구로 오판해 사라지므로). + // 재시도(일시)로 볼지 판정. 분류 가능한 HttpMappable 은 category 로 가르고(RETRYABLE 만 재시도), 그 외 + // 예상 못한 예외(NPE·IllegalStateException 등)는 일시·영구를 단정할 수 없어 보수적으로 재시도 대상으로 둔다 + // (즉시 FAILED 면 일시 오류를 영구로 오판해 사라지므로). // recover 가 상한(MAX_ATTEMPTS)까지만 재실행해 bounded 이고 #461 retry-first 기조와 맞는다. 순수 함수라 단위 테스트로 망라한다. + // + // **Error 분기는 없다** — 치명적 JVM 오류는 runCatchingException 이 잡지 않아 여기 도달하지 않는다(#941). + // 도달하지 않는 분기를 남기면 다음 사람이 "여기로 Error 가 온다"고 읽어, 삼키는 설계로 되돌리기 쉬워진다. internal fun isRetryable(e: Throwable): Boolean = when (e) { - is Error -> false is HttpMappable -> e.category == ErrorCategory.RETRYABLE else -> true } diff --git a/src/main/kotlin/com/depromeet/piki/item/service/ItemParsingMetrics.kt b/src/main/kotlin/com/depromeet/piki/item/service/ItemParsingMetrics.kt index 098222d5..5b2b19df 100644 --- a/src/main/kotlin/com/depromeet/piki/item/service/ItemParsingMetrics.kt +++ b/src/main/kotlin/com/depromeet/piki/item/service/ItemParsingMetrics.kt @@ -65,9 +65,13 @@ object ItemParsingMetrics { // 여기서는 그 bucket 을 라벨 문자열로 옮기기만 한다 — 원격 code 가 늘어도 이 함수는 그대로다. // when 이 exhaustive 라, bucket 이 추가되면 라벨을 정하지 않은 채로는 컴파일되지 않는다. // - // bucket 을 못 얻는 경우(분류 밖 예외 — 코드 버그성 NPE·JVM Error, 또는 매핑되지 않은 원격 code)는 - // internal_error 다. 그 자리는 "우리가 이름을 아는 실패"가 아니라 조사 대상이라는 뜻이므로, 이름 없는 - // 실패를 다른 바구니에 섞지 않는다. 링크·이미지 두 워커가 같은 함수를 쓴다(같은 메트릭 모집단). + // bucket 을 못 얻는 경우(분류 밖 예외 — 코드 버그성 NPE, 또는 매핑되지 않은 원격 code)는 internal_error 다. + // 그 자리는 "우리가 이름을 아는 실패"가 아니라 조사 대상이라는 뜻이므로, 이름 없는 실패를 다른 바구니에 + // 섞지 않는다. 링크·이미지 두 워커가 같은 함수를 쓴다(같은 메트릭 모집단). + // + // **JVM Error 는 여기 오지 않는다** — 워커가 runCatchingException 으로 잡아 Error 를 통과시키므로, 치명 오류는 + // 집계 대상이 아니라 바깥 층(JVM 종료·컨테이너 재시작)이 처리할 신호다(#941). 파라미터 타입이 Throwable 인 것은 + // Result.onFailure 의 시그니처를 그대로 받기 위함이지, Error 를 세겠다는 뜻이 아니다. fun reasonOf(e: Throwable): String { val bucket = ((e as? HttpMappable)?.errorCode as? ExtractionFailureCode)?.bucket ?: return REASON_INTERNAL_ERROR return when (bucket) { diff --git a/src/main/kotlin/com/depromeet/piki/item/service/RunCatchingException.kt b/src/main/kotlin/com/depromeet/piki/item/service/RunCatchingException.kt new file mode 100644 index 00000000..bb7425cf --- /dev/null +++ b/src/main/kotlin/com/depromeet/piki/item/service/RunCatchingException.kt @@ -0,0 +1,28 @@ +package com.depromeet.piki.item.service + +// 파싱 워커 전용의 좁은 포획 — `Exception` 만 잡고 `Error` 는 통과시킨다. 표준 `runCatching` 의 대체다. +// +// **왜 표준 runCatching 을 쓰지 않나**: 그것은 `Throwable` 을 전부 잡아 `Error` 까지 삼킨다. `Error` +// (OutOfMemoryError · StackOverflowError · LinkageError 등)는 "이 요청이 실패했다"가 아니라 **"이 프로세스로는 +// 더 진행할 수 없다"** 는 신호다. 그 신호를 받아 처리하는 층은 이미 우리 바깥에 있다 — JVM 의 +// `-XX:+ExitOnOutOfMemoryError`(즉시 종료), 컨테이너 재시작, 배포의 헬스체크·blue-green 전환. +// 워커가 삼키면 그 층들이 아무것도 하지 못하고, 반쯤 망가진 프로세스가 다음 작업을 계속 집어 간다(#941). +// +// **왜 실패로도 세면 안 되나**: 두 워커는 재시도 불가 예외를 "확정 실패"로 분류해 종결하는데, 이미지 경로에서 +// 확정 실패는 **raw 원본 회수(삭제)** 를 동반한다. `Error` 를 확정 실패로 오분류하면 서버 사정으로 죽는 순간 +// 사용자가 올린 원본까지 지워져 재실행할 입력이 사라진다. +// +// **전파하면 어디까지 가나** (실측): `@Async` 워커라 Error 는 Spring 의 async 예외 핸들러까지 올라간다 +// (AsyncConfig 에 커스텀 핸들러가 없어 기본 구현이 ERROR 로그를 남긴다). 힙 OOM 만 그 전에 JVM 이 스스로 종료한다. +// 즉 모든 Error 가 프로세스를 죽이지는 않는다 — 그러나 **확정 실패로 오분류되지 않는 것**이 핵심이다. +// 그 결과 상태 전이·raw 회수·메트릭 오염이 일어나지 않고, 행은 PROCESSING 으로 남아 stale 회수(#461)가 되살린다. +// 박동 레지스트리는 ParsingHeartbeat.guarded 의 finally 가 Error 가 지나가도 정리한다. +// +// 반환 타입을 `Result` 로 맞춰 기존 `onSuccess`/`onFailure` 체인을 그대로 쓴다 — 호출부의 모양은 바뀌지 않고 +// 포획 범위만 좁아진다. +internal inline fun runCatchingException(block: () -> T): Result = + try { + Result.success(block()) + } catch (e: Exception) { + Result.failure(e) + } diff --git a/src/test/kotlin/com/depromeet/piki/item/service/AsyncItemParsingWorkerTest.kt b/src/test/kotlin/com/depromeet/piki/item/service/AsyncItemParsingWorkerTest.kt index d3c96186..cd65edcb 100644 --- a/src/test/kotlin/com/depromeet/piki/item/service/AsyncItemParsingWorkerTest.kt +++ b/src/test/kotlin/com/depromeet/piki/item/service/AsyncItemParsingWorkerTest.kt @@ -35,10 +35,6 @@ class AsyncItemParsingWorkerTest { assertTrue(AsyncItemParsingWorker.isRetryable(NullPointerException())) } - @Test - fun `치명적 JVM 오류(Error)는 재시도 대상이 아니다`() { - // runCatching 이 Throwable 을 다 잡아 Error 도 여기로 온다. 재시도해도 소용없으므로 제외한다. - assertFalse(AsyncItemParsingWorker.isRetryable(OutOfMemoryError())) - assertFalse(AsyncItemParsingWorker.isRetryable(StackOverflowError())) - } + // 치명적 JVM 오류(Error)에 대한 판정은 여기서 다루지 않는다 — 워커가 runCatchingException 으로 잡아 + // Error 를 통과시키므로 이 함수에 도달하지 않는다(#941). 포획 범위 자체는 RunCatchingExceptionTest 가 고정한다. } diff --git a/src/test/kotlin/com/depromeet/piki/item/service/ItemParsingMetricsTest.kt b/src/test/kotlin/com/depromeet/piki/item/service/ItemParsingMetricsTest.kt index 72ab34bf..c2a36d88 100644 --- a/src/test/kotlin/com/depromeet/piki/item/service/ItemParsingMetricsTest.kt +++ b/src/test/kotlin/com/depromeet/piki/item/service/ItemParsingMetricsTest.kt @@ -40,13 +40,13 @@ class ItemParsingMetricsTest { @Test fun `분류 밖 예외는 internal_error 로 집계된다`() { - // 코드 버그성 예외(HttpMappable 아님)·치명적 JVM 오류도 확정 실패 경로로 들어올 수 있다. 이름 없는 실패를 - // 다른 바구니에 섞지 않고 "조사 대상"으로 몰아, 다른 reason 의 추세를 오염시키지 않는다. + // 코드 버그성 예외(HttpMappable 아님)가 확정 실패 경로로 들어올 수 있다. 이름 없는 실패를 다른 바구니에 + // 섞지 않고 "조사 대상"으로 몰아, 다른 reason 의 추세를 오염시키지 않는다. + // 치명적 JVM 오류(Error)는 여기 표본에 없다 — 워커가 잡지 않고 전파하므로 집계 자체에 닿지 않는다(#941). val internalError = ItemParsingMetrics.REASON_INTERNAL_ERROR assertEquals(internalError, ItemParsingMetrics.reasonOf(IllegalStateException("boom"))) assertEquals(internalError, ItemParsingMetrics.reasonOf(NullPointerException())) - assertEquals(internalError, ItemParsingMetrics.reasonOf(OutOfMemoryError())) } @Test diff --git a/src/test/kotlin/com/depromeet/piki/item/service/RunCatchingExceptionTest.kt b/src/test/kotlin/com/depromeet/piki/item/service/RunCatchingExceptionTest.kt new file mode 100644 index 00000000..892fca3a --- /dev/null +++ b/src/test/kotlin/com/depromeet/piki/item/service/RunCatchingExceptionTest.kt @@ -0,0 +1,61 @@ +package com.depromeet.piki.item.service + +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith +import kotlin.test.assertTrue + +// 파싱 워커의 포획 범위를 고정한다(#941). 표준 runCatching 은 Throwable 을 전부 잡아 Error 까지 삼키는데, +// 그러면 치명 오류를 받아 처리하도록 만들어 둔 바깥 층(JVM 종료·컨테이너 재시작·헬스체크)이 작동하지 못한다. +// 여기서 잡히는 것과 통과하는 것의 경계를 못 박아, 다시 넓은 포획으로 되돌아가면 깨지게 한다. +class RunCatchingExceptionTest { + @Test + fun `정상 반환값은 성공 Result 로 감싼다`() { + val result = runCatchingException { 42 } + + assertEquals(42, result.getOrNull()) + } + + @Test + fun `Exception 은 잡아 실패 Result 로 만든다`() { + // 파싱 실패의 대부분 — 추출 실패·전이 거부·DB 오류. 워커가 분류해 재시도·종결을 정한다. + val result = runCatchingException { throw IllegalStateException("boom") } + + assertTrue(result.isFailure) + assertTrue(result.exceptionOrNull() is IllegalStateException) + } + + @Test + fun `치명적 JVM 오류(Error)는 잡지 않고 그대로 전파한다`() { + // 이 테스트가 이 파일의 존재 이유다. Error 를 잡으면 워커가 그것을 "확정 실패" 로 분류해 종결하고, + // 이미지 경로에서는 raw 원본까지 회수해 재실행할 입력이 사라진다. + assertFailsWith { runCatchingException { throw OutOfMemoryError() } } + assertFailsWith { runCatchingException { throw StackOverflowError() } } + assertFailsWith { runCatchingException { throw NoClassDefFoundError() } } + } + + @Test + fun `파싱 워커는 표준 runCatching 을 쓰지 않는다`() { + // 위 포획 범위는 워커가 이 함수를 쓸 때만 의미가 있다. 한 곳이라도 표준 runCatching 으로 되돌아가면 + // 그 자리에서만 Error 가 다시 삼켜지는데, 그건 리뷰로만 걸러야 해서 조용히 새기 쉽다 — 기계로 못 박는다. + val workers = + listOf( + "AsyncItemParsingWorker.kt", + "AsyncImageParsingWorker.kt", + ).map { java.io.File("src/main/kotlin/com/depromeet/piki/item/service/$it") } + + val offenders = + workers.filter { file -> + file.readLines().any { line -> + // 주석이 아닌 실제 호출만 본다 — 이 규칙을 설명하는 주석에 이름이 등장하는 것은 위반이 아니다. + val code = line.substringBefore("//") + Regex("""(^|[^a-zA-Z])runCatching\s*\{""").containsMatchIn(code) + } + } + + assertTrue( + offenders.isEmpty(), + "표준 runCatching 은 Error 까지 삼킨다. runCatchingException 으로 바꿔라: ${offenders.map { it.name }}", + ) + } +}