FOUR-21497 - Fix script retry error handling for script microservice

ProcessMaker/Jobs/CheckScriptTimeout.php

@@ -0,0 +1,48 @@
+<?php
+
+namespace ProcessMaker\Jobs;
+
+use Illuminate\Bus\Queueable;
+use Illuminate\Contracts\Queue\ShouldQueue;
+use Illuminate\Foundation\Bus\Dispatchable;
+use Illuminate\Queue\InteractsWithQueue;
+use Illuminate\Queue\SerializesModels;
+use Illuminate\Support\Facades\Cache;
+use ProcessMaker\Exception\ScriptTimeoutException;
+
+/**
+ * The microservice handles script timeouts but this is an additional check
+ * in case the microservice does not respond.
+ */
+class CheckScriptTimeout implements ShouldQueue
+{
+    use Dispatchable, InteractsWithQueue, Queueable, SerializesModels;
+
+    protected string $executionId;
+
+    const ADDITIONAL_TIME = 5;
+
+    public function __construct(string $executionId)
+    {
+        $this->executionId = $executionId;
+    }
+
+    public function handle()
+    {
+        if (Cache::has($this->executionId)) {
+            throw new ScriptTimeoutException('Script execution timed out for execution ID: ' . $this->executionId);
+        }
+    }
+
+    public static function start(string $executionId, int $timeout)
+    {
+        Cache::put($executionId, 'timeout: ' . $timeout, 86400); // hold in cache for a maximum of 24 hours
+        self::dispatch($executionId)
+            ->delay(now()->addSeconds($timeout + self::ADDITIONAL_TIME));
+    }
+
+    public static function stop(string $executionId)
+    {
+        Cache::forget($executionId);
+    }
+}

ProcessMaker/Jobs/ErrorHandling.php

@@ -2,15 +2,21 @@
 
 namespace ProcessMaker\Jobs;
 
+use Illuminate\Contracts\Queue\ShouldQueue;
 use Illuminate\Support\Arr;
 use Illuminate\Support\Facades\Log;
 use Illuminate\Support\Facades\Notification;
+use ProcessMaker\Exception\ScriptException;
+use ProcessMaker\Exception\ScriptTimeoutException;
 use ProcessMaker\Models\Process;
 use ProcessMaker\Models\ProcessRequest;
 use ProcessMaker\Models\ProcessRequestToken;
 use ProcessMaker\Models\Script;
 use ProcessMaker\Models\ScriptVersion;
+use ProcessMaker\Nayra\Contracts\Bpmn\ActivityInterface;
+use ProcessMaker\Nayra\Contracts\Bpmn\ScriptTaskInterface;
 use ProcessMaker\Notifications\ErrorExecutionNotification;
+use Throwable;
 
 class ErrorHandling
 {
@@ -33,29 +39,52 @@ class ErrorHandling
      */
     public $defaultErrorHandling = [];
 
+    public $job;
+
+    public $exception;
+
+    public $metadata;
+
     public function __construct(
-        public $element,
-        public $processRequestToken,
+        public ActivityInterface $element,
+        public ProcessRequestToken $processRequestToken,
     ) {
         $this->bpmnErrorHandling = json_decode($element->getProperty('errorHandling'), true) ?? [];
     }
 
-    public function handleRetries($job, $exception)
+    public function handleRetries(ShouldQueue $job, Throwable $exception)
+    {
+        $this->job = $job;
+        $this->exception = $exception;
+
+        return $this->handldRetryAttempts();
+    }
+
+    public function handleRetriesForScriptMicroservice(Throwable $exception, array $metadata)
+    {
+        $this->metadata = $metadata;
+        $this->exception = $exception;
+
+        return $this->handldRetryAttempts();
+    }
+
+    private function handldRetryAttempts()
     {
-        $message = $exception->getMessage();
         $finalAttempt = true;
+        $attemptNumber = $this->getAttemptNumber();
+        $message = $this->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);
+            if ($attemptNumber <= $this->retryAttempts()) {
+                Log::info('Retry the job process. Attempt ' . $attemptNumber . ' of ' . $this->retryAttempts() . ', Wait time: ' . $this->retryWaitTime());
+                $this->requeue();
 
                 $finalAttempt = false;
 
                 return [$message, $finalAttempt];
             }
 
-            $message = __('Job failed after :attempts total attempts', ['attempts' => $job->attemptNum]) . "\n" . $message;
+            $message = __('Job failed after :attempts total attempts', ['attempts' => $attemptNumber]) . "\n" . $message;
 
             $this->sendExecutionErrorNotification($message);
         } else {
@@ -65,26 +94,46 @@ public function handleRetries($job, $exception)
         return [$message, $finalAttempt];
     }
 
-    private function requeue($job)
+    private function requeue()
     {
-        $class = get_class($job);
-        if ($job instanceof RunNayraServiceTask) {
+        if ($this->job instanceof RunNayraServiceTask) {
             $newJob = new RunNayraServiceTask($this->processRequestToken);
-            $newJob->attemptNum = $job->attemptNum + 1;
+            $newJob->attemptNum = $this->attemptNumber + 1;
         } else {
-            $newJob = new $class(
-                Process::findOrFail($job->definitionsId),
-                ProcessRequest::findOrFail($job->instanceId),
-                ProcessRequestToken::findOrFail($job->tokenId),
-                $job->data,
-                $job->attemptNum + 1
+            $jobClass = $this->getJobClass();
+            $newJob = new $jobClass(
+                $this->processRequestToken->process,
+                $this->processRequestToken->processRequest,
+                $this->processRequestToken,
+                $this->getData(),
+                $this->getAttemptNumber() + 1
             );
         }
         $newJob->delay($this->retryWaitTime());
         $newJob->onQueue('bpmn');
         dispatch($newJob);
     }
 
+    private function getData()
+    {
+        return $this->job ? $this->job->data : $this->metadata['script_task']['data'];
+    }
+
+    private function getAttemptNumber()
+    {
+        return $this->job ? $this->job->attemptNum : $this->metadata['script_task']['attempt_num'];
+    }
+
+    private function getMessage()
+    {
+        return $this->exception->getMessage();
+    }
+
+    private function getJobClass()
+    {
+        return $this->job ? get_class($this->job) : $this->metadata['script_task']['job_class'];
+    }
+
     /**
      * Send execution error notification.
      */
@@ -204,4 +253,14 @@ public function setDefaultsFromDataSourceConfig(array $config)
             'retry_wait_time' => Arr::get($endpointConfig, 'retry_wait_time', 5),
         ];
     }
+
+    public static function convertErrorResponseToException($response)
+    {
+        if ($response['status'] === 'error') {
+            if (str_starts_with($response['message'], 'Command exceeded timeout of')) {
+                throw new ScriptTimeoutException($response['message']);
+            }
+            throw new ScriptException($response['message']);
+        }
+    }
 }

ProcessMaker/Jobs/RunScriptTask.php

@@ -105,7 +105,8 @@ public function action(ProcessRequestToken $token = null, ScriptTaskInterface $e
                     'instance_id' => $this->instanceId,
                     'token_id' => $this->tokenId,
                     'data' => $data,
-                    'attempts' => $this->attemptNum,
+                    'attempt_num' => $this->attemptNum,
+                    'job_class' => get_class($this),
                 ],
             ];
             $response = $script->runScript($data, $configuration, $token->getId(), $errorHandling->timeout(), 0, $metadata);
@@ -126,20 +127,38 @@ public function action(ProcessRequestToken $token = null, ScriptTaskInterface $e
                 $message = $exception->getMessage();
             }
 
-            if ($finalAttempt) {
-                $token->setStatus(ScriptTaskInterface::TOKEN_STATE_FAILING);
-            }
+            self::setErrorStatus($finalAttempt, $token, $element, $message, $scriptRef, $exception, false);
+        }
+    }
 
-            $error = $element->getRepository()->createError();
-            $error->setName($message);
+    public static function setErrorStatus(
+        bool $finalAttempt,
+        ProcessRequestToken $token,
+        $element,
+        $message,
+        $scriptRef,
+        Throwable $exception,
+        bool $isMicroservice
+    ) {
+        if ($finalAttempt) {
+            $token->setStatus(ScriptTaskInterface::TOKEN_STATE_FAILING);
+        }
 
-            $token->setProperty('error', $error);
-            $exceptionClass = get_class($exception);
-            $modifiedException = new $exceptionClass($message);
-            $token->logError($modifiedException, $element);
+        $error = $element->getRepository()->createError();
+        $error->setName($message);
 
-            Log::error('Script failed: ' . $scriptRef . ' - ' . $message);
-            Log::error($exception->getTraceAsString());
+        $token->setProperty('error', $error);
+        $exceptionClass = get_class($exception);
+        $modifiedException = new $exceptionClass($message);
+        $token->logError($modifiedException, $element);
+
+        Log::error('Script failed: ' . $scriptRef . ' - ' . $message);
+        Log::error($exception->getTraceAsString());
+
+        // If it's from the microservice, we need to call handleFailed()
+        // manually here because failed() is only called when it's a job.
+        if ($isMicroservice) {
+            self::handleFailed($exception, $token->id);
         }
     }
 
@@ -169,7 +188,12 @@ private function updateData($response)
      */
     public function failed(Throwable $exception)
     {
-        if (!$this->tokenId) {
+        self::handleFailed($exception, $this->tokenId);
+    }
+
+    private static function handleFailed(Throwable $exception, $tokenId)
+    {
+        if (!$tokenId) {
             Log::error('Script failed: ' . $exception->getMessage());
 
             return;

ProcessMaker/ScriptRunners/ScriptMicroserviceRunner.php

@@ -6,9 +6,11 @@
 use Illuminate\Support\Facades\Cache;
 use Illuminate\Support\Facades\Http;
 use Illuminate\Support\Facades\Log;
+use Illuminate\Support\Str;
 use ProcessMaker\Exception\ConfigurationException;
-use ProcessMaker\Exception\ScriptException;
 use ProcessMaker\GenerateAccessToken;
+use ProcessMaker\Jobs\CheckScriptTimeout;
+use ProcessMaker\Jobs\ErrorHandling;
 use ProcessMaker\Models\EnvironmentVariable;
 use ProcessMaker\Models\Script;
 use ProcessMaker\Models\User;
@@ -92,7 +94,15 @@ public function run($code, array $data, array $config, $timeout, $user, $sync, $
 
         $response->throw();
 
-        return $response->json();
+        $result = $response->json();
+
+        if ($sync) {
+            ErrorHandling::convertErrorResponseToException($result);
+        } else {
+            CheckScriptTimeout::start($metadata['execution_id'], $timeout);
+        }
+
+        return $result;
     }
 
     private function getEnvironmentVariables(User $user)
@@ -148,6 +158,7 @@ public function getMetadata($user)
             'instance' => config('app.url'),
             'user_id' => $user->id,
             'user_email' => $user->email,
+            'execution_id' => 'script-execution-' . Str::uuid(),
         ];
     }
 

ProcessMaker/Services/ScriptMicroserviceService.php

@@ -5,7 +5,12 @@
 use Illuminate\Http\Request;
 use Illuminate\Support\Facades\Log;
 use ProcessMaker\Events\ScriptResponseEvent;
+use ProcessMaker\Exception\ScriptException;
+use ProcessMaker\Exception\ScriptTimeoutException;
+use ProcessMaker\Jobs\CheckScriptTimeout;
 use ProcessMaker\Jobs\CompleteActivity;
+use ProcessMaker\Jobs\ErrorHandling;
+use ProcessMaker\Jobs\RunScriptTask;
 use ProcessMaker\Models\Process as Definitions;
 use ProcessMaker\Models\ProcessRequest;
 use ProcessMaker\Models\ProcessRequestToken;
@@ -32,6 +37,11 @@ public function handle(Request $request)
                 null,
                 $response['metadata']['nonce']));
         }
+
+        if ($response['metadata']['execution_id']) {
+            CheckScriptTimeout::stop($response['metadata']['execution_id']);
+        }
+
         if (!empty($response['metadata']['script_task'])) {
             $script = Script::find($response['metadata']['script_task']['script_id']);
             $definitions = Definitions::find($response['metadata']['script_task']['definition_id']);
@@ -41,5 +51,15 @@ public function handle(Request $request)
                 CompleteActivity::dispatch($definitions, $instance, $token, $response['output'])->onQueue('bpmn');
             }
         }
+
+        try {
+            ErrorHandling::convertErrorResponseToException($response);
+        } catch (ScriptException|ScriptTimeoutException $e) {
+            $element = $definitions->getDefinitions(true)->getElementInstanceById($token->element_id);
+            $errorHandling = new ErrorHandling($element, $token);
+            $errorHandling->setDefaultsFromScript($script->versionFor($instance));
+            [$message, $finalAttempt] = $errorHandling->handleRetriesForScriptMicroservice($e, $response['metadata']);
+            RunScriptTask::setErrorStatus($finalAttempt, $token, $element, $message, $script->id, $e, true);
+        }
     }
 }