From 113bafdd2f19042341bc0f59ef987a4beb0b420b Mon Sep 17 00:00:00 2001 From: Erawat Chamanont Date: Mon, 9 Mar 2026 22:57:15 +0000 Subject: [PATCH] CIVIMM-480: Add webhook stuck recovery with attempt tracking --- .../BAO/PaymentWebhook.php | 138 ++++++++-- .../Service/WebhookQueueRunnerService.php | 148 ++++++++++- api/v3/PaymentWebhookRunner/Run.php | 2 +- phpstan-baseline.neon | 43 ++- .../BAO/PaymentWebhookTest.php | 246 +++++++++++++++--- 5 files changed, 509 insertions(+), 68 deletions(-) diff --git a/CRM/Paymentprocessingcore/BAO/PaymentWebhook.php b/CRM/Paymentprocessingcore/BAO/PaymentWebhook.php index d0f74dd..6757d02 100644 --- a/CRM/Paymentprocessingcore/BAO/PaymentWebhook.php +++ b/CRM/Paymentprocessingcore/BAO/PaymentWebhook.php @@ -58,8 +58,8 @@ public static function findByEventId($eventId) { * being processed by another worker, callers should skip it. * * Stuck webhooks (in 'processing' too long) are handled separately by - * resetStuckWebhooks() which resets their status to 'new', after which - * this method will return FALSE. + * the stuck recovery process in WebhookQueueRunnerService which resets + * their status to 'new', after which this method will return FALSE. * * @param string $eventId Processor event ID * @return bool TRUE if event is processed or being processed, FALSE otherwise @@ -368,65 +368,147 @@ public static function batchUpdateStatus(array $ids, string $status): void { } /** - * Maximum number of stuck webhooks to reset per run. + * Maximum number of stuck webhooks to process per run. * * Prevents unbounded loop when many webhooks are stuck. */ public const MAX_STUCK_RESET_LIMIT = 100; /** - * Reset stuck webhooks that have been processing for too long. + * Default timeout for stuck webhook detection (1 day in minutes). + */ + public const DEFAULT_STUCK_TIMEOUT_MINUTES = 1440; + + /** + * Find webhooks stuck in 'processing' state beyond the timeout. * - * Webhooks stuck in 'processing' for more than 30 minutes are reset to 'new'. - * This handles cases where the processor crashed mid-processing. + * Returns webhook records for the service layer to decide + * whether to retry or mark as permanent error. * * Limited to MAX_STUCK_RESET_LIMIT per run to prevent unbounded loops. * * @param int $timeoutMinutes Minutes after which to consider a webhook stuck * - * @return int Number of webhooks reset + * @return array List of stuck webhook records with id, attempts, processor_type + * @phpstan-return array */ - public static function resetStuckWebhooks(int $timeoutMinutes = 30): int { + public static function getStuckWebhooks(int $timeoutMinutes = self::DEFAULT_STUCK_TIMEOUT_MINUTES): array { $cutoff = date('Y-m-d H:i:s', strtotime("-{$timeoutMinutes} minutes")); - $stuckWebhooks = \Civi\Api4\PaymentWebhook::get(FALSE) - ->addSelect('id') + $webhooks = []; + foreach (\Civi\Api4\PaymentWebhook::get(FALSE) + ->addSelect('id', 'attempts', 'processor_type') ->addWhere('status', '=', 'processing') ->addWhere('processing_started_at', 'IS NOT NULL') ->addWhere('processing_started_at', '<', $cutoff) ->setLimit(self::MAX_STUCK_RESET_LIMIT) - ->execute(); + ->execute() as $webhook) { + $webhooks[] = $webhook; + } + return $webhooks; + } - $webhookIds = array_column($stuckWebhooks->getArrayCopy(), 'id'); + /** + * Find orphaned webhooks in 'new' status from previous recovery. + * + * When stuck recovery resets a webhook to 'new' but the process crashes + * before re-queuing, the webhook is left in 'new' with attempts > 0. + * This method finds those orphaned webhooks so they can be re-queued. + * + * Distinguishes orphans from retry-flow webhooks by requiring: + * - next_retry_at IS NULL (retry-flow sets next_retry_at) + * - processing_started_at IS NOT NULL (stuck recovery resets from + * 'processing' which always has processing_started_at set) + * + * @param string $processorType Processor type filter + * @param int $limit Maximum number of webhooks to return + * + * @return array List of orphaned webhook records + * @phpstan-return array + */ + public static function getOrphanedNewWebhooks(string $processorType, int $limit = 50): array { + $webhooks = []; + foreach (\Civi\Api4\PaymentWebhook::get(FALSE) + ->addSelect('id', 'processor_type') + ->addWhere('processor_type', '=', $processorType) + ->addWhere('status', '=', 'new') + ->addWhere('attempts', '>', 0) + ->addWhere('next_retry_at', 'IS NULL') + ->addWhere('processing_started_at', 'IS NOT NULL') + ->setLimit($limit) + ->execute() as $webhook) { + $webhooks[] = $webhook; + } + return $webhooks; + } - if (empty($webhookIds)) { + /** + * Batch reset stuck webhooks to 'new' with incremented attempt counts. + * + * Uses a single SQL UPDATE for performance. Only updates webhooks + * that are still in 'processing' status (optimistic locking). + * + * @param array $ids Webhook IDs to reset + * @phpstan-param array $ids + * @param string $errorLog Error message to record + * + * @return int Number of rows actually updated + */ + public static function batchResetStuckToNew(array $ids, string $errorLog): int { + if (empty($ids)) { return 0; } - // Batch update all stuck webhooks at once (avoids N+1) - $idList = implode(',', array_map('intval', $webhookIds)); - $errorLog = 'Reset from stuck processing state after ' . $timeoutMinutes . ' minutes'; + $idList = implode(',', array_map('intval', $ids)); $sql = "UPDATE civicrm_payment_webhook - SET status = 'new', error_log = %1 - WHERE id IN ({$idList})"; + SET status = 'new', + attempts = attempts + 1, + error_log = %1 + WHERE id IN ({$idList}) + AND status = 'processing'"; - \CRM_Core_DAO::executeQuery($sql, [ + $dao = \CRM_Core_DAO::executeQuery($sql, [ 1 => [$errorLog, 'String'], ]); - $count = count($webhookIds); + return $dao->affectedRows(); + } - if ($count > 0) { - \Civi::log()->warning('Reset stuck webhooks', [ - 'count' => $count, - 'timeout_minutes' => $timeoutMinutes, - 'webhook_ids' => $webhookIds, - 'limit_applied' => $count >= self::MAX_STUCK_RESET_LIMIT, - ]); + /** + * Batch mark stuck webhooks as permanent error with incremented attempts. + * + * Uses a single SQL UPDATE for performance. Only updates webhooks + * that are still in 'processing' status (optimistic locking). + * + * @param array $ids Webhook IDs to mark as permanent error + * @phpstan-param array $ids + * @param string $errorLog Error message to record + * + * @return int Number of rows actually updated + */ + public static function batchMarkStuckAsPermanentError(array $ids, string $errorLog): int { + if (empty($ids)) { + return 0; } - return $count; + $idList = implode(',', array_map('intval', $ids)); + + $sql = "UPDATE civicrm_payment_webhook + SET status = 'permanent_error', + result = 'error', + attempts = attempts + 1, + error_log = %1, + processed_at = %2 + WHERE id IN ({$idList}) + AND status = 'processing'"; + + $dao = \CRM_Core_DAO::executeQuery($sql, [ + 1 => [$errorLog, 'String'], + 2 => [date('Y-m-d H:i:s'), 'String'], + ]); + + return $dao->affectedRows(); } } diff --git a/Civi/Paymentprocessingcore/Service/WebhookQueueRunnerService.php b/Civi/Paymentprocessingcore/Service/WebhookQueueRunnerService.php index feaab1a..4b408fb 100644 --- a/Civi/Paymentprocessingcore/Service/WebhookQueueRunnerService.php +++ b/Civi/Paymentprocessingcore/Service/WebhookQueueRunnerService.php @@ -35,6 +35,13 @@ class WebhookQueueRunnerService { */ private WebhookHandlerRegistry $registry; + /** + * Whether stuck recovery has already run in this invocation. + * + * @var bool + */ + private bool $stuckRecoveryDone = FALSE; + /** * WebhookQueueRunnerService constructor. * @@ -62,21 +69,18 @@ public function __construct(WebhookHandlerRegistry $registry) { public function runAllQueues(int $batchSize = self::DEFAULT_BATCH_SIZE): array { $processorTypes = $this->registry->getRegisteredProcessorTypes(); - // First, reset any stuck webhooks across all processors - $stuckReset = \CRM_Paymentprocessingcore_BAO_PaymentWebhook::resetStuckWebhooks(); + // Recover stuck webhooks (crashed workers) across all processors + $stuckResult = $this->recoverStuckWebhooks(); $results = [ '_meta' => [ - 'stuck_webhooks_reset' => $stuckReset, + 'stuck_webhooks_reset' => $stuckResult['reset_count'], + 'stuck_webhooks_permanent_error' => $stuckResult['permanent_error_count'], 'batch_size' => $batchSize, ], ]; foreach ($processorTypes as $processorType) { - // First, re-queue any webhooks ready for retry - $this->requeueRetryableWebhooks($processorType); - - // Then process the queue $results[$processorType] = $this->runQueue($processorType, $batchSize); } @@ -99,12 +103,15 @@ public function runAllQueues(int $batchSize = self::DEFAULT_BATCH_SIZE): array { * items_failed, items_remaining */ public function runQueue(string $processorType, int $batchSize = self::DEFAULT_BATCH_SIZE): array { - // Reset any stuck webhooks before processing - \CRM_Paymentprocessingcore_BAO_PaymentWebhook::resetStuckWebhooks(); + // Recover stuck webhooks once per invocation (skipped if already done) + $this->recoverStuckWebhooks(); // Re-queue any webhooks ready for retry $this->requeueRetryableWebhooks($processorType); + // Re-queue orphaned 'new' webhooks from previous stuck recovery + $this->requeueOrphanedNewWebhooks($processorType); + /** @var \Civi\Paymentprocessingcore\Service\WebhookQueueService $queueService */ $queueService = \Civi::service('paymentprocessingcore.webhook_queue'); $queue = $queueService->getQueue($processorType); @@ -198,6 +205,91 @@ public function runQueue(string $processorType, int $batchSize = self::DEFAULT_B ]; } + /** + * Recover webhooks stuck in 'processing' state due to worker crashes. + * + * Finds webhooks stuck for longer than the timeout (default: 1 day), + * increments their attempt count, and decides: + * - attempts < MAX_RETRY_ATTEMPTS: reset to 'new' and re-queue + * - attempts >= MAX_RETRY_ATTEMPTS: mark as permanent_error + * + * Uses batch SQL updates for performance (1 SELECT + at most 2 UPDATEs). + * + * @return array Summary with 'reset_count' and 'permanent_error_count' + * @phpstan-return array{reset_count: int, permanent_error_count: int} + */ + private function recoverStuckWebhooks(): array { + if ($this->stuckRecoveryDone) { + return ['reset_count' => 0, 'permanent_error_count' => 0]; + } + $this->stuckRecoveryDone = TRUE; + + $stuckWebhooks = \CRM_Paymentprocessingcore_BAO_PaymentWebhook::getStuckWebhooks(); + + if (empty($stuckWebhooks)) { + return ['reset_count' => 0, 'permanent_error_count' => 0]; + } + + $maxAttempts = \CRM_Paymentprocessingcore_BAO_PaymentWebhook::MAX_RETRY_ATTEMPTS; + $errorLog = sprintf( + 'Reset from stuck processing state after %d minutes', + \CRM_Paymentprocessingcore_BAO_PaymentWebhook::DEFAULT_STUCK_TIMEOUT_MINUTES + ); + + // Partition into retryable vs exceeded based on what attempt count will be + $retryableIds = []; + $exceededIds = []; + $retryableWebhooks = []; + + foreach ($stuckWebhooks as $webhook) { + $nextAttempts = $webhook['attempts'] + 1; + + if ($nextAttempts >= $maxAttempts) { + $exceededIds[] = (int) $webhook['id']; + } + else { + $retryableIds[] = (int) $webhook['id']; + $retryableWebhooks[] = $webhook; + } + } + + // Batch update: retryable → 'new' with attempts+1 + $resetCount = \CRM_Paymentprocessingcore_BAO_PaymentWebhook::batchResetStuckToNew( + $retryableIds, + $errorLog + ); + + // Batch update: exceeded → 'permanent_error' with attempts+1 + $permanentErrorCount = \CRM_Paymentprocessingcore_BAO_PaymentWebhook::batchMarkStuckAsPermanentError( + $exceededIds, + sprintf('Max retries exceeded after stuck recovery. %s', $errorLog) + ); + + // Re-queue recovered webhooks + if (!empty($retryableWebhooks)) { + /** @var \Civi\Paymentprocessingcore\Service\WebhookQueueService $queueService */ + $queueService = \Civi::service('paymentprocessingcore.webhook_queue'); + + foreach ($retryableWebhooks as $webhook) { + $queueService->addTask($webhook['processor_type'], (int) $webhook['id'], []); + } + } + + if ($resetCount > 0 || $permanentErrorCount > 0) { + \Civi::log()->warning('Recovered stuck webhooks', [ + 'reset_to_new' => $resetCount, + 'permanent_error' => $permanentErrorCount, + 'reset_ids' => $retryableIds, + 'permanent_error_ids' => $exceededIds, + ]); + } + + return [ + 'reset_count' => $resetCount, + 'permanent_error_count' => $permanentErrorCount, + ]; + } + /** * Re-queue webhooks that are ready for retry. * @@ -250,6 +342,44 @@ private function requeueRetryableWebhooks(string $processorType): int { return $count; } + /** + * Re-queue orphaned 'new' webhooks from previous stuck recovery. + * + * When stuck recovery resets a webhook to 'new' but the process crashes + * before re-queuing, webhooks are left orphaned. This method finds + * those webhooks (status='new' with attempts > 0) and adds them + * back to the queue. + * + * @param string $processorType The processor type + * + * @return int Number of webhooks re-queued + */ + private function requeueOrphanedNewWebhooks(string $processorType): int { + $webhooks = \CRM_Paymentprocessingcore_BAO_PaymentWebhook::getOrphanedNewWebhooks( + $processorType + ); + + if (empty($webhooks)) { + return 0; + } + + /** @var \Civi\Paymentprocessingcore\Service\WebhookQueueService $queueService */ + $queueService = \Civi::service('paymentprocessingcore.webhook_queue'); + + foreach ($webhooks as $webhook) { + $queueService->addTask($processorType, (int) $webhook['id'], []); + } + + $count = count($webhooks); + + \Civi::log()->info('Re-queued orphaned new webhooks', [ + 'processor_type' => $processorType, + 'count' => $count, + ]); + + return $count; + } + /** * Process a single webhook task. * diff --git a/api/v3/PaymentWebhookRunner/Run.php b/api/v3/PaymentWebhookRunner/Run.php index 7ca5a45..d74c567 100644 --- a/api/v3/PaymentWebhookRunner/Run.php +++ b/api/v3/PaymentWebhookRunner/Run.php @@ -16,7 +16,7 @@ * Features: * - Batch size limiting to prevent job timeouts (default: 250 per processor) * - Automatic retry of failed webhooks with exponential backoff - * - Recovery of stuck webhooks (processing > 30 minutes) + * - Recovery of stuck webhooks (processing > 1 day) with attempt tracking * * @param array $params * API parameters: diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 13ce0d8..de1f8b4 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -45,6 +45,11 @@ parameters: count: 1 path: CRM/Paymentprocessingcore/BAO/PaymentWebhook.php + - + message: "#^Call to an undefined method object\\:\\:affectedRows\\(\\)\\.$#" + count: 2 + path: CRM/Paymentprocessingcore/BAO/PaymentWebhook.php + - message: "#^Comparison operation \"\\>\" between int\\<1, max\\> and 0 is always true\\.$#" count: 1 @@ -75,6 +80,16 @@ parameters: count: 1 path: CRM/Paymentprocessingcore/BAO/PaymentWebhook.php + - + message: "#^Method CRM_Paymentprocessingcore_BAO_PaymentWebhook\\:\\:getOrphanedNewWebhooks\\(\\) should return array\\ but returns array\\, mixed\\>\\.$#" + count: 1 + path: CRM/Paymentprocessingcore/BAO/PaymentWebhook.php + + - + message: "#^Method CRM_Paymentprocessingcore_BAO_PaymentWebhook\\:\\:getStuckWebhooks\\(\\) should return array\\ but returns array\\, mixed\\>\\.$#" + count: 1 + path: CRM/Paymentprocessingcore/BAO/PaymentWebhook.php + - message: "#^Method CRM_Paymentprocessingcore_BAO_PaymentWebhook\\:\\:getWebhooksReadyForRetry\\(\\) return type has no value type specified in iterable type array\\.$#" count: 1 @@ -240,6 +255,7 @@ parameters: count: 1 path: Civi/Paymentprocessingcore/Service/WebhookQueueRunnerService.php + - message: "#^Method Civi\\\\Paymentprocessingcore\\\\Service\\\\WebhookQueueService\\:\\:addTask\\(\\) has parameter \\$params with no value type specified in iterable type array\\.$#" count: 1 @@ -382,7 +398,7 @@ parameters: - message: "#^Cannot access property \\$id on CRM_Paymentprocessingcore_DAO_PaymentWebhook\\|null\\.$#" - count: 4 + count: 26 path: tests/phpunit/CRM/Paymentprocessingcore/BAO/PaymentWebhookTest.php - @@ -480,11 +496,36 @@ parameters: count: 1 path: tests/phpunit/CRM/Paymentprocessingcore/BAO/PaymentWebhookTest.php + - + message: "#^Offset 'attempts' does not exist on array\\|null\\.$#" + count: 4 + path: tests/phpunit/CRM/Paymentprocessingcore/BAO/PaymentWebhookTest.php + + - + message: "#^Offset 'error_log' does not exist on array\\|null\\.$#" + count: 2 + path: tests/phpunit/CRM/Paymentprocessingcore/BAO/PaymentWebhookTest.php + - message: "#^Offset 'id' does not exist on array\\|null\\.$#" count: 2 path: tests/phpunit/CRM/Paymentprocessingcore/BAO/PaymentWebhookTest.php + - + message: "#^Offset 'processed_at' does not exist on array\\|null\\.$#" + count: 1 + path: tests/phpunit/CRM/Paymentprocessingcore/BAO/PaymentWebhookTest.php + + - + message: "#^Offset 'result' does not exist on array\\|null\\.$#" + count: 1 + path: tests/phpunit/CRM/Paymentprocessingcore/BAO/PaymentWebhookTest.php + + - + message: "#^Offset 'status' does not exist on array\\|null\\.$#" + count: 4 + path: tests/phpunit/CRM/Paymentprocessingcore/BAO/PaymentWebhookTest.php + - message: "#^Property CRM_Paymentprocessingcore_BAO_PaymentWebhookTest\\:\\:\\$attemptId \\(int\\) does not accept int\\|string\\|null\\.$#" count: 1 diff --git a/tests/phpunit/CRM/Paymentprocessingcore/BAO/PaymentWebhookTest.php b/tests/phpunit/CRM/Paymentprocessingcore/BAO/PaymentWebhookTest.php index f912b9a..8f6060c 100644 --- a/tests/phpunit/CRM/Paymentprocessingcore/BAO/PaymentWebhookTest.php +++ b/tests/phpunit/CRM/Paymentprocessingcore/BAO/PaymentWebhookTest.php @@ -424,9 +424,9 @@ public function testUpdateStatusAtomicDoesNotSetTimestampForOtherStatuses(): voi } /** - * Tests resetStuckWebhooks finds and resets webhooks based on processing_started_at. + * Tests getStuckWebhooks finds webhooks based on processing_started_at. */ - public function testResetStuckWebhooksUsesProcessingStartedAt(): void { + public function testGetStuckWebhooksFindsCorrectRecords(): void { // Create webhook stuck in processing (processing_started_at > 30 min ago) $stuckWebhook = PaymentWebhook::create([ 'event_id' => 'evt_test_stuck_1003', @@ -452,60 +452,248 @@ public function testResetStuckWebhooksUsesProcessingStartedAt(): void { $this->assertNotNull($notStuckWebhook); $this->assertNotNull($notStuckWebhook->id); - // Reset stuck webhooks - $resetCount = PaymentWebhook::resetStuckWebhooks(30); + // Get stuck webhooks with 30 min timeout + $stuckResults = PaymentWebhook::getStuckWebhooks(30); - $this->assertEquals(1, $resetCount, 'Should reset 1 stuck webhook'); + $this->assertCount(1, $stuckResults, 'Should find 1 stuck webhook'); + $this->assertEquals($stuckWebhook->id, $stuckResults[0]['id']); + $this->assertArrayHasKey('attempts', $stuckResults[0]); + $this->assertArrayHasKey('processor_type', $stuckResults[0]); + $this->assertEquals('stripe', $stuckResults[0]['processor_type']); + } + + /** + * Tests getStuckWebhooks does not return webhooks with NULL processing_started_at. + */ + public function testGetStuckWebhooksIgnoresNullProcessingStartedAt(): void { + // Create webhook in processing but with NULL processing_started_at + $webhook = PaymentWebhook::create([ + 'event_id' => 'evt_test_null_1005', + 'processor_type' => 'stripe', + 'event_type' => 'payment_intent.succeeded', + 'status' => 'processing', + 'attempts' => 0, + ]); + + $this->assertNotNull($webhook); + $this->assertNotNull($webhook->id); - // Verify stuck webhook was reset to 'new' - $stuckWebhookData = \Civi\Api4\PaymentWebhook::get(FALSE) - ->addWhere('id', '=', $stuckWebhook->id) + // Get stuck webhooks + $stuckResults = PaymentWebhook::getStuckWebhooks(30); + + $this->assertEmpty($stuckResults, 'Webhook with NULL timestamp should not be found'); + + // Verify webhook is still processing + $webhookData = \Civi\Api4\PaymentWebhook::get(FALSE) + ->addWhere('id', '=', $webhook->id) ->execute() ->first(); - $this->assertNotNull($stuckWebhookData); - $this->assertArrayHasKey('status', $stuckWebhookData); - $this->assertEquals('new', $stuckWebhookData['status'], 'Stuck webhook should be reset to new'); + $this->assertEquals('processing', $webhookData['status']); + } + + /** + * Tests getStuckWebhooks uses 1 day default timeout. + */ + public function testGetStuckWebhooksDefaultTimeoutIsOneDay(): void { + // Create webhook stuck for 2 days (should be found) + $oldStuck = PaymentWebhook::create([ + 'event_id' => 'evt_test_old_stuck_1006', + 'processor_type' => 'stripe', + 'event_type' => 'payment_intent.succeeded', + 'status' => 'processing', + 'attempts' => 0, + 'processing_started_at' => date('Y-m-d H:i:s', strtotime('-2 days')), + ]); - // Verify not-stuck webhook remains in processing - $notStuckWebhookData = \Civi\Api4\PaymentWebhook::get(FALSE) - ->addWhere('id', '=', $notStuckWebhook->id) + // Create webhook stuck for only 1 hour (should NOT be found with default) + $recentStuck = PaymentWebhook::create([ + 'event_id' => 'evt_test_recent_stuck_1007', + 'processor_type' => 'stripe', + 'event_type' => 'payment_intent.succeeded', + 'status' => 'processing', + 'attempts' => 0, + 'processing_started_at' => date('Y-m-d H:i:s', strtotime('-1 hour')), + ]); + + $this->assertNotNull($oldStuck->id); + $this->assertNotNull($recentStuck->id); + + // Use default timeout (1 day) + $stuckResults = PaymentWebhook::getStuckWebhooks(); + + $this->assertCount(1, $stuckResults, 'Only webhook stuck > 1 day should be found'); + $this->assertEquals($oldStuck->id, $stuckResults[0]['id']); + } + + /** + * Tests batchResetStuckToNew increments attempts and sets status to new. + */ + public function testBatchResetStuckToNewIncrementsAttempts(): void { + $webhook = PaymentWebhook::create([ + 'event_id' => 'evt_test_batch_reset_1008', + 'processor_type' => 'stripe', + 'event_type' => 'payment_intent.succeeded', + 'status' => 'processing', + 'attempts' => 1, + 'processing_started_at' => date('Y-m-d H:i:s', strtotime('-2 days')), + ]); + + $this->assertNotNull($webhook->id); + + PaymentWebhook::batchResetStuckToNew( + [(int) $webhook->id], + 'Test reset' + ); + + $webhookData = \Civi\Api4\PaymentWebhook::get(FALSE) + ->addWhere('id', '=', $webhook->id) ->execute() ->first(); - $this->assertNotNull($notStuckWebhookData); - $this->assertArrayHasKey('status', $notStuckWebhookData); - $this->assertEquals('processing', $notStuckWebhookData['status'], 'Not-stuck webhook should remain processing'); + $this->assertEquals('new', $webhookData['status']); + $this->assertEquals(2, $webhookData['attempts']); + $this->assertEquals('Test reset', $webhookData['error_log']); } /** - * Tests resetStuckWebhooks does not reset webhooks with NULL processing_started_at. + * Tests batchMarkStuckAsPermanentError sets correct status and fields. */ - public function testResetStuckWebhooksIgnoresNullProcessingStartedAt(): void { - // Create webhook in processing but with NULL processing_started_at + public function testBatchMarkStuckAsPermanentError(): void { $webhook = PaymentWebhook::create([ - 'event_id' => 'evt_test_null_1005', + 'event_id' => 'evt_test_batch_perm_1009', 'processor_type' => 'stripe', 'event_type' => 'payment_intent.succeeded', 'status' => 'processing', - 'attempts' => 0, + 'attempts' => 2, + 'processing_started_at' => date('Y-m-d H:i:s', strtotime('-2 days')), ]); - $this->assertNotNull($webhook); $this->assertNotNull($webhook->id); - // Reset stuck webhooks - $resetCount = PaymentWebhook::resetStuckWebhooks(30); + PaymentWebhook::batchMarkStuckAsPermanentError( + [(int) $webhook->id], + 'Max retries exceeded' + ); - // Verify webhook was NOT reset $webhookData = \Civi\Api4\PaymentWebhook::get(FALSE) ->addWhere('id', '=', $webhook->id) ->execute() ->first(); - $this->assertNotNull($webhookData); - $this->assertArrayHasKey('status', $webhookData); - $this->assertEquals('processing', $webhookData['status'], 'Webhook with NULL timestamp should not be reset'); + $this->assertEquals('permanent_error', $webhookData['status']); + $this->assertEquals('error', $webhookData['result']); + $this->assertEquals(3, $webhookData['attempts']); + $this->assertNotNull($webhookData['processed_at']); + $this->assertEquals('Max retries exceeded', $webhookData['error_log']); + } + + /** + * Tests batchResetStuckToNew only updates webhooks in processing status. + */ + public function testBatchResetStuckToNewOnlyUpdatesProcessingStatus(): void { + // Create a webhook already in 'new' status (should not be affected) + $newWebhook = PaymentWebhook::create([ + 'event_id' => 'evt_test_guard_1010', + 'processor_type' => 'stripe', + 'event_type' => 'payment_intent.succeeded', + 'status' => 'new', + 'attempts' => 0, + ]); + + $this->assertNotNull($newWebhook->id); + + PaymentWebhook::batchResetStuckToNew( + [(int) $newWebhook->id], + 'Should not update' + ); + + $webhookData = \Civi\Api4\PaymentWebhook::get(FALSE) + ->addWhere('id', '=', $newWebhook->id) + ->execute() + ->first(); + + // Attempts should NOT have been incremented + $this->assertEquals(0, $webhookData['attempts']); + $this->assertEquals('new', $webhookData['status']); + } + + /** + * Tests getOrphanedNewWebhooks excludes retry-flow webhooks. + */ + public function testGetOrphanedNewWebhooksExcludesRetryFlow(): void { + // Orphan from stuck recovery: new, attempts > 0, no next_retry_at, + // has processing_started_at (was previously in 'processing') + $orphan = PaymentWebhook::create([ + 'event_id' => 'evt_test_orphan_1011', + 'processor_type' => 'stripe', + 'event_type' => 'payment_intent.succeeded', + 'status' => 'new', + 'attempts' => 1, + 'processing_started_at' => date('Y-m-d H:i:s', strtotime('-2 hours')), + ]); + + // Retry-flow webhook: new, attempts > 0, HAS next_retry_at + $retryFlow = PaymentWebhook::create([ + 'event_id' => 'evt_test_retry_1012', + 'processor_type' => 'stripe', + 'event_type' => 'payment_intent.succeeded', + 'status' => 'new', + 'attempts' => 1, + 'next_retry_at' => date('Y-m-d H:i:s', strtotime('+10 minutes')), + ]); + + // Fresh webhook: new, attempts = 0 (should not match) + $fresh = PaymentWebhook::create([ + 'event_id' => 'evt_test_fresh_1013', + 'processor_type' => 'stripe', + 'event_type' => 'payment_intent.succeeded', + 'status' => 'new', + 'attempts' => 0, + ]); + + $this->assertNotNull($orphan->id); + $this->assertNotNull($retryFlow->id); + $this->assertNotNull($fresh->id); + + $results = PaymentWebhook::getOrphanedNewWebhooks('stripe'); + + $resultIds = array_column($results, 'id'); + $this->assertContains($orphan->id, $resultIds, 'Orphan should be found'); + $this->assertNotContains($retryFlow->id, $resultIds, 'Retry-flow webhook should be excluded'); + $this->assertNotContains($fresh->id, $resultIds, 'Fresh webhook should be excluded'); + } + + /** + * Tests batchResetStuckToNew returns actual affected row count. + */ + public function testBatchResetStuckToNewReturnsAffectedCount(): void { + $processing = PaymentWebhook::create([ + 'event_id' => 'evt_test_affected_1014', + 'processor_type' => 'stripe', + 'event_type' => 'payment_intent.succeeded', + 'status' => 'processing', + 'attempts' => 0, + 'processing_started_at' => date('Y-m-d H:i:s', strtotime('-2 days')), + ]); + + $notProcessing = PaymentWebhook::create([ + 'event_id' => 'evt_test_affected_1015', + 'processor_type' => 'stripe', + 'event_type' => 'payment_intent.succeeded', + 'status' => 'new', + 'attempts' => 0, + ]); + + $this->assertNotNull($processing->id); + $this->assertNotNull($notProcessing->id); + + $affected = PaymentWebhook::batchResetStuckToNew( + [(int) $processing->id, (int) $notProcessing->id], + 'Test reset' + ); + + $this->assertEquals(1, $affected, 'Only the processing webhook should be affected'); } }