Refactor failed task retries to use the queue

ProcessMaker/Jobs/ErrorHandling.php

@@ -0,0 +1,198 @@
+<?php
+
+namespace ProcessMaker\Jobs;
+
+use Illuminate\Support\Arr;
+use Illuminate\Support\Facades\Log;
+use Illuminate\Support\Facades\Notification;
+use ProcessMaker\Models\Process;
+use ProcessMaker\Models\ProcessRequest;
+use ProcessMaker\Models\ProcessRequestToken;
+use ProcessMaker\Models\Script;
+use ProcessMaker\Models\ScriptVersion;
+use ProcessMaker\Notifications\ErrorExecutionNotification;
+
+class ErrorHandling
+{
+    /**
+     * Error handling settings that are set in modeler.
+     *
+     * These settings take precedence over those set in script or
+     * data source configuration.
+     *
+     * @var array
+     */
+    public $bpmnErrorHandling = [];
+
+    /**
+     * Default settings that are set in script or data source configuration.
+     *
+     * $bpmnErrorHandling takes precedence over these if set.
+     *
+     * @var array
+     */
+    public $defaultErrorHandling = [];
+
+    public function __construct(
+        public $element,
+        public $processRequestToken,
+    ) {
+        $this->bpmnErrorHandling = json_decode($element->getProperty('errorHandling'), true) ?? [];
+    }
+
+    public function handleRetries($job, $exception)
+    {
+        $message = $exception->getMessage();
+
+        if ($this->retryAttempts() > 0) {
+            if ($job->attemptNum <= $this->retryAttempts()) {
+                Log::info('Retry the job process. Attempt ' . $job->attemptNum . ' of ' . $this->retryAttempts() . ', Wait time: ' . $this->retryWaitTime());
+                $this->requeue($job);
+
+                return $message;
+            }
+
+            $message = __('Job failed after :attempts total attempts', ['attempts' => $job->attemptNum]) . "\n" . $message;
+
+            $this->sendExecutionErrorNotification($message);
+        } else {
+            $this->sendExecutionErrorNotification($message);
+        }
+
+        return $message;
+    }
+
+    private function requeue($job)
+    {
+        $class = get_class($job);
+        $newJob = new $class(
+            Process::findOrFail($job->definitionsId),
+            ProcessRequest::findOrFail($job->instanceId),
+            ProcessRequestToken::findOrFail($job->tokenId),
+            $job->data,
+            $job->attemptNum + 1
+        );
+        $newJob->delay($this->retryWaitTime());
+        dispatch($newJob);
+    }
+
+    /**
+     * Send execution error notification.
+     */
+    public function sendExecutionErrorNotification(string $message)
+    {
+        if ($this->processRequestToken) {
+            $user = $this->processRequestToken->processRequest->processVersion->manager;
+            if ($user !== null) {
+                Log::info('Send Execution Error Notification: ' . $message);
+                Notification::send($user, new ErrorExecutionNotification($this->processRequestToken, $message, $this->bpmnErrorHandling));
+            }
+        }
+    }
+
+    /**
+     * Get the effective timeout
+     *
+     * @return int
+     */
+    public function timeout()
+    {
+        return $this->get('timeout');
+    }
+
+    /**
+     * Get the effective retry attempts
+     *
+     * @return int
+     */
+    public function retryAttempts()
+    {
+        return $this->get('retry_attempts');
+    }
+
+    /**
+     * Get the effective retry wait time.
+     *
+     * @return int
+     */
+    public function retryWaitTime()
+    {
+        return $this->get('retry_wait_time');
+    }
+
+    /**
+     * Get the attribute from the bpmnErrorHandling array but if it's not set
+     * return the defaultErrorHandling value
+     *
+     * @param string $attribute
+     * @return void
+     */
+    public function get($attribute)
+    {
+        $value = Arr::get(
+            $this->bpmnErrorHandling,
+            $attribute,
+            null
+        );
+
+        if ($value === null || $value === '') {
+            $value = Arr::get($this->defaultErrorHandling, $attribute, 0);
+        }
+
+        return (int) $value;
+    }
+
+    /**
+     * Set defaults from script model
+     *
+     * @param ScriptVersion $script
+     * @return void
+     */
+    public function setDefaultsFromScript(Script|ScriptVersion $script)
+    {
+        $this->defaultErrorHandling = [
+            'timeout' => $script->timeout,
+            'retry_attempts' => $script->retry_attempts,
+            'retry_wait_time' => $script->retry_wait_time,
+        ];
+    }
+
+    /**
+     * Set defaults from data source model endpoint
+     *
+     * @param array $config
+     * @return void
+     */
+    public function setDefaultsFromDataSourceConfig(array $config)
+    {
+        // If this is not a Data Connecter, don't do any error handling
+        $id = Arr::get($config, 'dataSource', null);
+        if (!$id) {
+            return;
+        }
+
+        // Check if the data source package is installed
+        $class = "ProcessMaker\Packages\Connectors\DataSources\Models\DataSource";
+        if (!class_exists($class)) {
+            return;
+        }
+
+        // Check if the data source exists
+        $dataSource = $class::find($id);
+        if (!$dataSource) {
+            return;
+        }
+
+        // Check if the endpoint config exists in the data source
+        $endpointConfig = Arr::get($dataSource->endpoints, Arr::get($config, 'endpoint', null));
+        if (!$endpointConfig) {
+            return;
+        }
+
+        $this->defaultErrorHandling = [
+            'timeout' => Arr::get($endpointConfig, 'timeout', 0),
+            'retry_attempts' => Arr::get($endpointConfig, 'retry_attempts', 0),
+            'retry_wait_time' => Arr::get($endpointConfig, 'retry_wait_time', 5),
+        ];
+    }
+}

ProcessMaker/Jobs/RunScriptTask.php

@@ -25,9 +25,7 @@ class RunScriptTask extends BpmnAction implements ShouldQueue
 
     public $data;
 
-    public $tries = 3;
-
-    public $backoff = 60;
+    public $attemptNum;
 
     /**
      * Create a new job instance.
@@ -37,14 +35,15 @@ class RunScriptTask extends BpmnAction implements ShouldQueue
      * @param \ProcessMaker\Models\ProcessRequestToken $token
      * @param array $data
      */
-    public function __construct(Definitions $definitions, ProcessRequest $instance, ProcessRequestToken $token, array $data)
+    public function __construct(Definitions $definitions, ProcessRequest $instance, ProcessRequestToken $token, array $data, $attemptNum = 1)
     {
         $this->onQueue('bpmn');
         $this->definitionsId = $definitions->getKey();
         $this->instanceId = $instance->getKey();
         $this->tokenId = $token->getKey();
         $this->elementId = $token->getProperty('element_ref');
         $this->data = $data;
+        $this->attemptNum = $attemptNum;
     }
 
     /**
@@ -60,10 +59,6 @@ public function action(ProcessRequestToken $token = null, ScriptTaskInterface $e
         }
         $scriptRef = $element->getProperty('scriptRef');
         $configuration = json_decode($element->getProperty('config'), true);
-        $errorHandling = json_decode($element->getProperty('errorHandling'), true);
-        if ($errorHandling === null) {
-            $errorHandling = [];
-        }
 
         // Check to see if we've failed parsing.  If so, let's convert to empty array.
         if ($configuration === null) {
@@ -86,10 +81,13 @@ public function action(ProcessRequestToken $token = null, ScriptTaskInterface $e
                 $script = Script::findOrFail($scriptRef)->versionFor($instance);
             }
 
+            $errorHandling = new ErrorHandling($element, $token);
+            $errorHandling->setDefaultsFromScript($script);
+
             $this->unlock();
             $dataManager = new DataManager();
             $data = $dataManager->getData($token);
-            $response = $script->runScript($data, $configuration, $token->getId(), $errorHandling);
+            $response = $script->runScript($data, $configuration, $token->getId(), $errorHandling->timeout());
 
             $this->withUpdatedContext(function ($engine, $instance, $element, $processModel, $token) use ($response) {
                 // Exit if the task was completed or closed
@@ -111,13 +109,17 @@ public function action(ProcessRequestToken $token = null, ScriptTaskInterface $e
         } catch (Throwable $exception) {
             $token->setStatus(ScriptTaskInterface::TOKEN_STATE_FAILING);
 
+            $message = $errorHandling->handleRetries($this, $exception);
+
             $error = $element->getRepository()->createError();
-            $error->setName($exception->getMessage());
+            $error->setName($message);
 
             $token->setProperty('error', $error);
-            $token->logError($exception, $element);
+            $exceptionClass = get_class($exception);
+            $modifiedException = new $exceptionClass($message);
+            $token->logError($modifiedException, $element);
 
-            Log::error('Script failed: ' . $scriptRef . ' - ' . $exception->getMessage());
+            Log::error('Script failed: ' . $scriptRef . ' - ' . $message);
             Log::error($exception->getTraceAsString());
         }
     }
@@ -132,14 +134,22 @@ public function failed(Throwable $exception)
 
             return;
         }
+        if (get_class($exception) === 'Illuminate\\Queue\\MaxAttemptsExceededException') {
+            $message = 'This is a type MaxAttemptsExceededException exception, it appears '
+                . 'that the global value configured in config/horizon.php has been exceeded '
+                . 'in the retries. Please consult with your main administrator.';
+            Log::error($message);
+        }
         Log::error('Script (#' . $this->tokenId . ') failed: ' . $exception->getMessage());
         $token = ProcessRequestToken::find($this->tokenId);
         if ($token) {
             $element = $token->getBpmnDefinition();
             $token->setStatus(ScriptTaskInterface::TOKEN_STATE_FAILING);
-            $error = $element->getRepository()->createError();
-            $error->setName($exception->getMessage());
-            $token->setProperty('error', $error);
+            if (method_exists($element, 'getRepository')) {
+                $error = $element->getRepository()->createError();
+                $error->setName($exception->getMessage());
+                $token->setProperty('error', $error);
+            }
             Log::error($exception->getTraceAsString());
             $token->save();
         }

ProcessMaker/Jobs/RunServiceTask.php

@@ -29,6 +29,8 @@ class RunServiceTask extends BpmnAction implements ShouldQueue
 
     public $backoff = 60;
 
+    public $attemptNum;
+
     /**
      * Create a new job instance.
      *
@@ -37,7 +39,7 @@ class RunServiceTask extends BpmnAction implements ShouldQueue
      * @param \ProcessMaker\Models\ProcessRequestToken $token
      * @param array $data
      */
-    public function __construct(Definitions $definitions, ProcessRequest $instance, ProcessRequestToken $token, array $data)
+    public function __construct(Definitions $definitions, ProcessRequest $instance, ProcessRequestToken $token, array $data, $attemptNum = 1)
     {
         if ($token->getOwner()) {
             $pmConfig = json_decode($token->getOwnerElement()->getProperty('config', '{}'), true);
@@ -50,6 +52,7 @@ public function __construct(Definitions $definitions, ProcessRequest $instance,
         $this->tokenId = $token->getKey();
         $this->elementId = $token->getProperty('element_ref');
         $this->data = $data;
+        $this->attemptNum = $attemptNum;
     }
 
     /**
@@ -65,7 +68,6 @@ public function action(ProcessRequestToken $token = null, ServiceTaskInterface $
         }
         $implementation = $element->getImplementation();
         $configuration = json_decode($element->getProperty('config'), true);
-        $errorHandling = json_decode($element->getProperty('errorHandling'), true);
 
         // Check to see if we've failed parsing.  If so, let's convert to empty array.
         if ($configuration === null) {
@@ -84,16 +86,23 @@ public function action(ProcessRequestToken $token = null, ServiceTaskInterface $
                 throw new ScriptException('Service task not implemented: ' . $implementation);
             }
 
+            $errorHandling = new ErrorHandling($element, $token);
+            $errorHandling->setDefaultsFromDataSourceConfig($configuration);
+
             $this->unlock();
             $dataManager = new DataManager();
             $data = $dataManager->getData($token);
 
+            /**
+             * todo: Please review the "else" section as it appears unreachable since if `$existsImpl`
+             * is not true, an exception is thrown a few lines above.
+             */
             if ($existsImpl) {
                 $response = [
-                    'output' => WorkflowManager::runServiceImplementation($implementation, $data, $configuration, $token->getId(), $errorHandling),
+                    'output' => WorkflowManager::runServiceImplementation($implementation, $data, $configuration, $token->getId(), $errorHandling->timeout()),
                 ];
             } else {
-                $response = $script->runScript($data, $configuration, $token->getId(), $errorHandling);
+                $response = $script->runScript($data, $configuration, $token->getId(), $errorHandling->timeout());
             }
             $this->withUpdatedContext(function ($engine, $instance, $element, $processModel, $token) use ($response) {
                 // Exit if the task was completed or closed
@@ -115,10 +124,18 @@ public function action(ProcessRequestToken $token = null, ServiceTaskInterface $
         } catch (Throwable $exception) {
             // Change to error status
             $token->setStatus(ServiceTaskInterface::TOKEN_STATE_FAILING);
+
+            $message = $errorHandling->handleRetries($this, $exception);
+
             $error = $element->getRepository()->createError();
-            $error->setName($exception->getMessage());
+            $error->setName($message);
+
             $token->setProperty('error', $error);
-            Log::info('Service task failed: ' . $implementation . ' - ' . $exception->getMessage());
+            $exceptionClass = get_class($exception);
+            $modifiedException = new $exceptionClass($message);
+            $token->logError($modifiedException, $element);
+
+            Log::error('Service task failed: ' . $implementation . ' - ' . $message);
             Log::error($exception->getTraceAsString());
         }
     }