From cedf833426472dc46d830461c68c4eddf7853a51 Mon Sep 17 00:00:00 2001 From: "@tanya_r" Date: Fri, 19 Jun 2026 20:26:33 -0300 Subject: [PATCH] fix(payments): recover stuck initiated payments in the retry sweep The retry sweep previously only re-routed payments stuck in SCREENING. A crash between an initiate commit and the after-commit orchestration could leave a payment stranded in INITIATED with nothing to advance it. Sweep both states through a shared loop: stuck INITIATED payments are orchestrated from the start, stuck SCREENING payments are resumed, and either is failed once past the deadline. Closes #318 --- .../retry/PaymentRetryServiceImpl.kt | 26 ++++++--- .../retry/PaymentRetryServiceTest.kt | 57 ++++++++++++++++--- 2 files changed, 68 insertions(+), 15 deletions(-) diff --git a/services/payments/src/main/kotlin/com/fincore/payments/application/retry/PaymentRetryServiceImpl.kt b/services/payments/src/main/kotlin/com/fincore/payments/application/retry/PaymentRetryServiceImpl.kt index 3870de3..c23991a 100644 --- a/services/payments/src/main/kotlin/com/fincore/payments/application/retry/PaymentRetryServiceImpl.kt +++ b/services/payments/src/main/kotlin/com/fincore/payments/application/retry/PaymentRetryServiceImpl.kt @@ -7,6 +7,7 @@ import com.fincore.core.PaymentId import com.fincore.payments.application.PaymentOrchestrator import com.fincore.payments.application.PaymentService import com.fincore.payments.domain.enum.PaymentStatus +import com.fincore.payments.infrastructure.persistence.PaymentEntity import com.fincore.payments.infrastructure.persistence.PaymentPersistenceAdapter import com.fincore.payments.infrastructure.persistence.PaymentRepository import org.slf4j.LoggerFactory @@ -15,10 +16,11 @@ import java.time.Instant import java.util.UUID /** - * Re-routes payments stuck in SCREENING (a transient bank failure left them there) within a bounded age window, - * failing them once past the deadline. NOT transactional: the bank re-submit happens inside [PaymentOrchestrator] - * outside any transaction, and each transition is its own short transaction. Assumes a single scheduler instance; - * overlapping ticks rely on an idempotent bank submit (the sandbox is deterministic by payment id). + * Advances payments that stalled in INITIATED or SCREENING (a crash dropped the after-commit orchestration, or a + * transient bank failure left them screening) within a bounded age window, failing them once past the deadline. NOT + * transactional: the bank call happens inside [PaymentOrchestrator] outside any transaction, and each transition is its + * own short transaction. Assumes a single scheduler instance; overlapping ticks rely on an idempotent bank submit (the + * sandbox is deterministic by payment id). */ @Service class PaymentRetryServiceImpl( @@ -32,15 +34,25 @@ class PaymentRetryServiceImpl( override fun retryStuck() { val now = Instant.now() - val stuck = paymentRepository.findByStatusAndCreatedAtBefore(PaymentStatus.SCREENING, now.minus(properties.stuckAfter)) + val cutoff = now.minus(properties.stuckAfter) val deadline = now.minus(properties.maxAge) - for (entity in stuck) { + sweep(PaymentStatus.INITIATED, cutoff, deadline) { entity -> orchestrator.process(PaymentId(entity.id)) } + sweep(PaymentStatus.SCREENING, cutoff, deadline) { entity -> orchestrator.resume(adapter.toDomain(entity)) } + } + + private fun sweep( + status: PaymentStatus, + cutoff: Instant, + deadline: Instant, + recover: (PaymentEntity) -> Unit, + ) { + for (entity in paymentRepository.findByStatusAndCreatedAtBefore(status, cutoff)) { val expired = entity.createdAt.isBefore(deadline) attempt(entity.id, expired) { if (expired) { paymentService.markFailed(PaymentId(entity.id), DEADLINE_REASON) } else { - orchestrator.resume(adapter.toDomain(entity)) + recover(entity) } } } diff --git a/services/payments/src/test/kotlin/com/fincore/payments/application/retry/PaymentRetryServiceTest.kt b/services/payments/src/test/kotlin/com/fincore/payments/application/retry/PaymentRetryServiceTest.kt index 431110a..9ee8bb6 100644 --- a/services/payments/src/test/kotlin/com/fincore/payments/application/retry/PaymentRetryServiceTest.kt +++ b/services/payments/src/test/kotlin/com/fincore/payments/application/retry/PaymentRetryServiceTest.kt @@ -35,9 +35,13 @@ class PaymentRetryServiceTest { PaymentRetryProperties(enabled = true, stuckAfter = Duration.ofMinutes(5), maxAge = Duration.ofHours(1)), ) + init { + every { paymentRepository.findByStatusAndCreatedAtBefore(any(), any()) } returns emptyList() + } + @Test - fun `should resume a payment stuck within the retry window`() { - every { paymentRepository.findByStatusAndCreatedAtBefore(any(), any()) } returns listOf(stuck(Duration.ofMinutes(10))) + fun `should resume a payment stuck in screening within the retry window`() { + screening(listOf(stuck(PaymentStatus.SCREENING, Duration.ofMinutes(10)))) service.retryStuck() @@ -46,8 +50,8 @@ class PaymentRetryServiceTest { } @Test - fun `should fail a payment past the retry deadline without resuming`() { - every { paymentRepository.findByStatusAndCreatedAtBefore(any(), any()) } returns listOf(stuck(Duration.ofHours(2))) + fun `should fail a screening payment past the retry deadline without resuming`() { + screening(listOf(stuck(PaymentStatus.SCREENING, Duration.ofHours(2)))) service.retryStuck() @@ -55,10 +59,36 @@ class PaymentRetryServiceTest { verify(exactly = 0) { orchestrator.resume(any()) } } + @Test + fun `should process a payment stuck in initiated within the retry window`() { + initiated(listOf(stuck(PaymentStatus.INITIATED, Duration.ofMinutes(10)))) + + service.retryStuck() + + verify { orchestrator.process(any()) } + verify(exactly = 0) { paymentService.markFailed(any(), any()) } + verify(exactly = 0) { orchestrator.resume(any()) } + } + + @Test + fun `should fail an initiated payment past the retry deadline without processing`() { + initiated(listOf(stuck(PaymentStatus.INITIATED, Duration.ofHours(2)))) + + service.retryStuck() + + verify { paymentService.markFailed(any(), "retry deadline exceeded") } + verify(exactly = 0) { orchestrator.process(any()) } + verify(exactly = 0) { orchestrator.resume(any()) } + } + @Test fun `should continue the batch when resuming one payment throws`() { - every { paymentRepository.findByStatusAndCreatedAtBefore(any(), any()) } returns - listOf(stuck(Duration.ofMinutes(10)), stuck(Duration.ofMinutes(20))) + screening( + listOf( + stuck(PaymentStatus.SCREENING, Duration.ofMinutes(10)), + stuck(PaymentStatus.SCREENING, Duration.ofMinutes(20)), + ), + ) every { orchestrator.resume(any()) } throws RuntimeException("bank down") andThen Payment(PaymentId.generate(), Money(BigDecimal("100.00"), Currency.USD), "order-1", PaymentStatus.SUBMITTED) @@ -67,13 +97,24 @@ class PaymentRetryServiceTest { verify(exactly = 2) { orchestrator.resume(any()) } } - private fun stuck(age: Duration): PaymentEntity = + private fun screening(entities: List) { + every { paymentRepository.findByStatusAndCreatedAtBefore(PaymentStatus.SCREENING, any()) } returns entities + } + + private fun initiated(entities: List) { + every { paymentRepository.findByStatusAndCreatedAtBefore(PaymentStatus.INITIATED, any()) } returns entities + } + + private fun stuck( + status: PaymentStatus, + age: Duration, + ): PaymentEntity = PaymentEntity( UUID.randomUUID(), "order-1", BigDecimal("100.00"), "USD", - PaymentStatus.SCREENING, + status, Instant.now().minus(age), 0L, )