diff --git a/ProcessMaker/Jobs/CheckScriptTimeout.php b/ProcessMaker/Jobs/CheckScriptTimeout.php new file mode 100644 index 0000000000..b9d0691db6 --- /dev/null +++ b/ProcessMaker/Jobs/CheckScriptTimeout.php @@ -0,0 +1,48 @@ +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); + } +} diff --git a/ProcessMaker/Jobs/ErrorHandling.php b/ProcessMaker/Jobs/ErrorHandling.php index f33d994c32..9bf31cd1f1 100644 --- a/ProcessMaker/Jobs/ErrorHandling.php +++ b/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,19 +94,19 @@ 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()); @@ -85,6 +114,26 @@ private function requeue($job) 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']); + } + } } diff --git a/ProcessMaker/Jobs/RunScriptTask.php b/ProcessMaker/Jobs/RunScriptTask.php index 5558c23fa5..875abc6c13 100644 --- a/ProcessMaker/Jobs/RunScriptTask.php +++ b/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; diff --git a/ProcessMaker/ScriptRunners/ScriptMicroserviceRunner.php b/ProcessMaker/ScriptRunners/ScriptMicroserviceRunner.php index 420405652b..74674867fa 100644 --- a/ProcessMaker/ScriptRunners/ScriptMicroserviceRunner.php +++ b/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(), ]; } diff --git a/ProcessMaker/Services/ScriptMicroserviceService.php b/ProcessMaker/Services/ScriptMicroserviceService.php index a7f07309be..3e46cd0a31 100644 --- a/ProcessMaker/Services/ScriptMicroserviceService.php +++ b/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); + } } } diff --git a/tests/Jobs/ErrorHandlingTest.php b/tests/Jobs/ErrorHandlingTest.php index bfe8f2d2e1..af8dfd9463 100644 --- a/tests/Jobs/ErrorHandlingTest.php +++ b/tests/Jobs/ErrorHandlingTest.php @@ -20,7 +20,7 @@ private function runAssertions($settings) extract($settings); $errorHandling = ['timeout' => $bpmnTimeout, 'retry_attempts' => $bpmnRetryAttempts, 'retry_wait_time' => $bpmnRetryWaitTime]; - $element = Mockery::mock(); + $element = Mockery::mock(\ProcessMaker\Nayra\Contracts\Bpmn\ActivityInterface::class); $element->shouldReceive('getProperty')->andReturn(json_encode($errorHandling)); $script = Script::factory()->create(['timeout' => $modelTimeout, 'retry_attempts' => $modelRetryAttempts, 'retry_wait_time' => $modelRetryWaitTime]); @@ -33,16 +33,44 @@ private function runAssertions($settings) 'process_request_id' => $processRequest->id, ]); - $job = new RunScriptTask($process, $processRequest, $token, [], $attempt); + $job = new RunScriptTask($process, $processRequest, $token, ['foo' => 'baz'], $attempt); $errorHandling = new ErrorHandling($element, $token); $errorHandling->setDefaultsFromScript($script->getLatestVersion()); $this->assertEquals($expectedTimeout, $errorHandling->timeout()); $errorHandling->handleRetries($job, new \RuntimeException('error')); + $expectedData = ['foo' => 'baz']; + + $this->assertQueue($expectedNextAttempt, $expectedWaitTime, $expectedData); + + // Test handleRetriesForScriptMicroservice + + $metadata = [ + 'script_task' => [ + 'job_class' => RunScriptTask::class, + 'data' => ['foo' => 'bar'], + 'attempt_num' => $attempt, + 'message' => 'error', + ], + ]; + + Queue::fake(); + + $expectedData = ['foo' => 'bar']; + $errorHandling = new ErrorHandling($element, $token); + $errorHandling->setDefaultsFromScript($script->getLatestVersion()); + $errorHandling->handleRetriesForScriptMicroservice(new \RuntimeException('error'), $metadata); + + $this->assertQueue($expectedNextAttempt, $expectedWaitTime, $expectedData); + } + + private function assertQueue($expectedNextAttempt, $expectedWaitTime, $expectedData) + { if ($expectedNextAttempt !== false) { - Queue::assertPushed(RunScriptTask::class, function ($job) use ($expectedNextAttempt, $expectedWaitTime) { + Queue::assertPushed(RunScriptTask::class, function ($job) use ($expectedNextAttempt, $expectedWaitTime, $expectedData) { return $job->attemptNum === $expectedNextAttempt && - $job->delay === $expectedWaitTime; + $job->delay === $expectedWaitTime && + $job->data === $expectedData; }); } else { Queue::assertNotPushed(RunScriptTask::class);