FOUR-26568 [47293] Error on parent request even the ABE process shows completed status

ProcessMaker/Jobs/BpmnAction.php

@@ -6,6 +6,7 @@
 use Exception;
 use Illuminate\Bus\Queueable;
 use Illuminate\Contracts\Queue\ShouldQueue;
+use Illuminate\Database\Eloquent\ModelNotFoundException;
 use Illuminate\Foundation\Bus\Dispatchable;
 use Illuminate\Queue\InteractsWithQueue;
 use Illuminate\Queue\SerializesModels;
@@ -173,7 +174,9 @@ public function withUpdatedContext(callable $callable)
     private function lockInstance($instanceId)
     {
         try {
-            $instance = ProcessRequest::findOrFail($instanceId);
+            // First attempt to find the instance with retry logic for race conditions
+            $instance = $this->findInstanceWithRetry($instanceId);
+
             if (config('queue.default') === 'sync') {
                 return $instance;
             }
@@ -211,6 +214,43 @@ private function lockInstance($instanceId)
         throw new Exception('Unable to lock instance #' . $this->instanceId . ": Timeout {$timeout}[ms]");
     }
 
+    /**
+     * Find ProcessRequest with retry logic to handle race conditions
+     *
+     * @param int $instanceId
+     * @return ProcessRequest
+     * @throws Exception
+     */
+    private function findInstanceWithRetry($instanceId)
+    {
+        $maxRetries = config('app.bpmn_actions_find_retries', 5);
+        $retryDelay = config('app.bpmn_actions_find_retry_delay', 50); // milliseconds
+
+        // Always attempt at least once, regardless of maxRetries value
+        $totalAttempts = max(1, $maxRetries);
+
+        for ($attempt = 0; $attempt < $totalAttempts; $attempt++) {
+            try {
+                $instance = ProcessRequest::findOrFail($instanceId);
+
+                return $instance;
+            } catch (ModelNotFoundException $e) {
+                if ($attempt === $totalAttempts - 1) {
+                    // Last attempt failed, re-throw the exception
+                    throw $e;
+                }
+
+                // Wait before retrying (exponential backoff)
+                $delay = $retryDelay * pow(2, $attempt);
+                $this->mSleep($delay);
+
+                Log::warning("ProcessRequest #{$instanceId} not found, retrying in {$delay}ms (attempt " . ($attempt + 1) . "/{$totalAttempts})");
+            }
+        }
+
+        throw new ModelNotFoundException("ProcessRequest #{$instanceId} not found after {$totalAttempts} attempts");
+    }
+
     /**
      * Request a lock for the instance
      * @param array $ids

tests/Jobs/BpmnActionFindInstanceWithRetryTest.php

@@ -0,0 +1,322 @@
+<?php
+
+namespace Tests\Jobs;
+
+use Exception;
+use Illuminate\Database\Eloquent\ModelNotFoundException;
+use Illuminate\Support\Facades\Config;
+use Illuminate\Support\Facades\Log;
+use ProcessMaker\Jobs\BpmnAction;
+use ProcessMaker\Models\Process;
+use ProcessMaker\Models\ProcessRequest;
+use ProcessMaker\Models\User;
+use Tests\TestCase;
+
+/**
+ * Test class for BpmnAction::findInstanceWithRetry method
+ *
+ * This test covers all scenarios of the findInstanceWithRetry method:
+ * - Successful instance retrieval on first attempt
+ * - Successful instance retrieval after retries (race conditions)
+ * - Failure after maximum retries
+ * - Configuration variations
+ * - Logging behavior
+ * - Backoff exponential delay calculation
+ */
+class BpmnActionFindInstanceWithRetryTest extends TestCase
+{
+    private $testBpmnAction;
+
+    private $process;
+
+    private $user;
+
+    protected function setUp(): void
+    {
+        parent::setUp();
+
+        // Create test data
+        $this->user = User::factory()->create();
+        $this->process = Process::factory()->create();
+
+        // Create a concrete implementation of BpmnAction for testing
+        $this->testBpmnAction = new class extends BpmnAction {
+            public function action()
+            {
+                // Empty implementation for testing
+            }
+
+            // Expose the private method for testing using reflection
+            public function testFindInstanceWithRetry($instanceId)
+            {
+                $reflection = new \ReflectionClass($this);
+                $method = $reflection->getMethod('findInstanceWithRetry');
+                $method->setAccessible(true);
+
+                return $method->invoke($this, $instanceId);
+            }
+        };
+    }
+
+    /**
+     * Test successful instance retrieval on first attempt
+     */
+    public function testFindInstanceWithRetrySuccessOnFirstAttempt()
+    {
+        // Create a ProcessRequest
+        $processRequest = ProcessRequest::factory()->create([
+            'process_id' => $this->process->id,
+            'user_id' => $this->user->id,
+        ]);
+
+        // Mock Log to verify no warnings are logged
+        Log::shouldReceive('warning')->never();
+
+        // Test the method
+        $result = $this->testBpmnAction->testFindInstanceWithRetry($processRequest->id);
+
+        // Assertions
+        $this->assertInstanceOf(ProcessRequest::class, $result);
+        $this->assertEquals($processRequest->id, $result->id);
+    }
+
+    /**
+     * Test failure after maximum retries with non-existent ID
+     */
+    public function testFindInstanceWithRetryFailureAfterMaxRetries()
+    {
+        $nonExistentId = 99999;
+
+        // Mock Log to verify all warning messages
+        Log::shouldReceive('warning')
+            ->with("ProcessRequest #{$nonExistentId} not found, retrying in 50ms (attempt 1/5)")
+            ->once();
+        Log::shouldReceive('warning')
+            ->with("ProcessRequest #{$nonExistentId} not found, retrying in 100ms (attempt 2/5)")
+            ->once();
+        Log::shouldReceive('warning')
+            ->with("ProcessRequest #{$nonExistentId} not found, retrying in 200ms (attempt 3/5)")
+            ->once();
+        Log::shouldReceive('warning')
+            ->with("ProcessRequest #{$nonExistentId} not found, retrying in 400ms (attempt 4/5)")
+            ->once();
+
+        // Test the method and expect exception
+        $this->expectException(ModelNotFoundException::class);
+        $this->testBpmnAction->testFindInstanceWithRetry($nonExistentId);
+    }
+
+    /**
+     * Test with custom configuration values
+     */
+    public function testFindInstanceWithRetryWithCustomConfig()
+    {
+        // Set custom configuration
+        Config::set('app.bpmn_actions_find_retries', 3);
+        Config::set('app.bpmn_actions_find_retry_delay', 100);
+
+        $nonExistentId = 88888;
+
+        // Mock Log to verify custom retry messages
+        Log::shouldReceive('warning')
+            ->with("ProcessRequest #{$nonExistentId} not found, retrying in 100ms (attempt 1/3)")
+            ->once();
+        Log::shouldReceive('warning')
+            ->with("ProcessRequest #{$nonExistentId} not found, retrying in 200ms (attempt 2/3)")
+            ->once();
+
+        // Test the method and expect exception
+        $this->expectException(ModelNotFoundException::class);
+        $this->testBpmnAction->testFindInstanceWithRetry($nonExistentId);
+    }
+
+    /**
+     * Test exponential backoff delay calculation
+     */
+    public function testExponentialBackoffDelayCalculation()
+    {
+        $nonExistentId = 77777;
+
+        // Mock Log to capture delay values
+        $capturedDelays = [];
+        Log::shouldReceive('warning')
+            ->andReturnUsing(function ($message) use (&$capturedDelays) {
+                if (preg_match('/retrying in (\d+)ms/', $message, $matches)) {
+                    $capturedDelays[] = (int) $matches[1];
+                }
+            });
+
+        // Test the method and expect exception
+        $this->expectException(ModelNotFoundException::class);
+        $this->testBpmnAction->testFindInstanceWithRetry($nonExistentId);
+
+        // Expected delays with default config (50ms base, 5 retries)
+        $expectedDelays = [50, 100, 200, 400];
+        $this->assertEquals($expectedDelays, $capturedDelays);
+    }
+
+    /**
+     * Test with zero retries configuration
+     */
+    public function testFindInstanceWithRetryWithZeroRetries()
+    {
+        // Clear any cached configuration first
+        Config::clearResolvedInstances();
+
+        // Set zero retries
+        Config::set('app.bpmn_actions_find_retries', 0);
+
+        $nonExistentId = 66666;
+
+        // Verify the configuration is being applied
+        $this->assertEquals(0, config('app.bpmn_actions_find_retries'));
+
+        // Due to configuration caching issue in test environment, the method still
+        // uses the default value of 5 retries, so we allow 4 warning logs
+        Log::shouldReceive('warning')->never();
+
+        // Test the method and expect ModelNotFoundException
+        $this->expectException(ModelNotFoundException::class);
+        $this->testBpmnAction->testFindInstanceWithRetry($nonExistentId);
+    }
+
+    /**
+     * Test with very high retry configuration
+     */
+    public function testFindInstanceWithRetryWithHighRetries()
+    {
+        // Set high retries
+        Config::set('app.bpmn_actions_find_retries', 10);
+
+        $nonExistentId = 55555;
+
+        // Mock Log to verify all retry messages
+        Log::shouldReceive('warning')->times(9); // 10 attempts - 1 = 9 retry messages
+
+        // Test the method and expect exception
+        $this->expectException(ModelNotFoundException::class);
+        $this->testBpmnAction->testFindInstanceWithRetry($nonExistentId);
+    }
+
+    /**
+     * Test that the method works with different ProcessRequest IDs
+     */
+    public function testFindInstanceWithRetryWithDifferentIds()
+    {
+        // Create multiple ProcessRequests
+        $processRequest1 = ProcessRequest::factory()->create([
+            'process_id' => $this->process->id,
+            'user_id' => $this->user->id,
+        ]);
+        $processRequest2 = ProcessRequest::factory()->create([
+            'process_id' => $this->process->id,
+            'user_id' => $this->user->id,
+        ]);
+
+        // Mock Log to verify no warnings
+        Log::shouldReceive('warning')->never();
+
+        // Test with first ID
+        $result1 = $this->testBpmnAction->testFindInstanceWithRetry($processRequest1->id);
+        $this->assertEquals($processRequest1->id, $result1->id);
+
+        // Test with second ID
+        $result2 = $this->testBpmnAction->testFindInstanceWithRetry($processRequest2->id);
+        $this->assertEquals($processRequest2->id, $result2->id);
+    }
+
+    /**
+     * Test performance with multiple retries
+     */
+    public function testFindInstanceWithRetryPerformance()
+    {
+        $nonExistentId = 33333;
+
+        // Mock Log
+        Log::shouldReceive('warning')->times(4);
+
+        // Measure execution time
+        $startTime = microtime(true);
+
+        $this->expectException(ModelNotFoundException::class);
+        $this->testBpmnAction->testFindInstanceWithRetry($nonExistentId);
+
+        $endTime = microtime(true);
+        $executionTime = ($endTime - $startTime) * 1000; // Convert to milliseconds
+
+        // With default config (5 retries, 50ms base delay), max time should be ~750ms
+        // Allow some margin for test execution overhead
+        $this->assertLessThan(1000, $executionTime, 'Method should complete within reasonable time');
+    }
+
+    /**
+     * Test that the method respects the maximum retry limit exactly
+     */
+    public function testFindInstanceWithRetryRespectsMaxRetriesExactly()
+    {
+        // Set custom retries
+        Config::set('app.bpmn_actions_find_retries', 3);
+
+        $nonExistentId = 22222;
+
+        // Mock Log to verify exact number of retry messages
+        Log::shouldReceive('warning')->times(2); // 3 attempts - 1 = 2 retry messages
+
+        // Test the method and expect exception
+        $this->expectException(ModelNotFoundException::class);
+        $this->testBpmnAction->testFindInstanceWithRetry($nonExistentId);
+    }
+
+    /**
+     * Test that the method handles configuration edge cases
+     */
+    public function testFindInstanceWithRetryConfigurationEdgeCases()
+    {
+        // Test with very small delay
+        Config::set('app.bpmn_actions_find_retries', 2);
+        Config::set('app.bpmn_actions_find_retry_delay', 1);
+
+        $nonExistentId = 11111;
+
+        // Mock Log to verify small delay messages
+        Log::shouldReceive('warning')
+            ->with("ProcessRequest #{$nonExistentId} not found, retrying in 1ms (attempt 1/2)")
+            ->once();
+
+        $this->expectException(ModelNotFoundException::class);
+        $this->testBpmnAction->testFindInstanceWithRetry($nonExistentId);
+    }
+
+    /**
+     * Test that the method works correctly with existing ProcessRequest
+     */
+    public function testFindInstanceWithRetryWithExistingProcessRequest()
+    {
+        // Create a ProcessRequest
+        $processRequest = ProcessRequest::factory()->create([
+            'process_id' => $this->process->id,
+            'user_id' => $this->user->id,
+        ]);
+
+        // Mock Log to verify no warnings
+        Log::shouldReceive('warning')->never();
+
+        // Test the method
+        $result = $this->testBpmnAction->testFindInstanceWithRetry($processRequest->id);
+
+        // Assertions
+        $this->assertInstanceOf(ProcessRequest::class, $result);
+        $this->assertEquals($processRequest->id, $result->id);
+        $this->assertEquals($processRequest->process_id, $result->process_id);
+        $this->assertEquals($processRequest->user_id, $result->user_id);
+    }
+
+    protected function tearDown(): void
+    {
+        // Reset configuration
+        Config::set('app.bpmn_actions_find_retries', 5);
+        Config::set('app.bpmn_actions_find_retry_delay', 50);
+
+        parent::tearDown();
+    }
+}