diff --git a/ProcessMaker/Jobs/ErrorHandling.php b/ProcessMaker/Jobs/ErrorHandling.php new file mode 100644 index 0000000000..7343c6452f --- /dev/null +++ b/ProcessMaker/Jobs/ErrorHandling.php @@ -0,0 +1,198 @@ +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), + ]; + } +} diff --git a/ProcessMaker/Jobs/RunScriptTask.php b/ProcessMaker/Jobs/RunScriptTask.php index 57c622e674..130c69cf95 100644 --- a/ProcessMaker/Jobs/RunScriptTask.php +++ b/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,7 +35,7 @@ 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(); @@ -45,6 +43,7 @@ public function __construct(Definitions $definitions, ProcessRequest $instance, $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(); } diff --git a/ProcessMaker/Jobs/RunServiceTask.php b/ProcessMaker/Jobs/RunServiceTask.php index 3071adc163..72ce729918 100644 --- a/ProcessMaker/Jobs/RunServiceTask.php +++ b/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()); } } diff --git a/ProcessMaker/Models/Script.php b/ProcessMaker/Models/Script.php index 5dd39e8c8d..3ca23655ab 100644 --- a/ProcessMaker/Models/Script.php +++ b/ProcessMaker/Models/Script.php @@ -2,19 +2,11 @@ namespace ProcessMaker\Models; -use Hamcrest\Type\IsNumeric; -use Illuminate\Support\Arr; -use Illuminate\Support\Facades\Log; -use Illuminate\Support\Facades\Notification; use Illuminate\Validation\Rule; use ProcessMaker\Contracts\ScriptInterface; -use ProcessMaker\Exception\ScriptException; use ProcessMaker\Exception\ScriptLanguageNotSupported; -use ProcessMaker\Exception\ScriptTimeoutException; -use ProcessMaker\GenerateAccessToken; use ProcessMaker\Models\ScriptCategory; use ProcessMaker\Models\User; -use ProcessMaker\Notifications\ErrorExecutionNotification; use ProcessMaker\ScriptRunners\ScriptRunner; use ProcessMaker\Traits\Exportable; use ProcessMaker\Traits\HasCategories; @@ -85,13 +77,9 @@ class Script extends ProcessMakerModel implements ScriptInterface protected $casts = [ 'timeout' => 'integer', 'retry_attempts' => 'integer', - 'retry_wait_time' => 'integer', + 'retry_wait_time' => 'integer' ]; - private $errorHandling = []; - - private $attemptedRetries = 1; - /** * Override the default boot method to allow access to lifecycle hooks * @@ -129,8 +117,6 @@ public static function rules($existing = null) 'description' => 'required', 'run_as_user_id' => 'required', 'timeout' => 'integer|min:0|max:65535', - 'retry_attempts' => 'integer|min:0|max:65535', - 'retry_wait_time' => 'integer|min:0|max:65535', 'script_category_id' => [new CategoryRule($existing)], ]; } @@ -141,12 +127,11 @@ public static function rules($existing = null) * @param array $data * @param array $config */ - public function runScript(array $data, array $config, $tokenId = '', $errorHandling = []) + public function runScript(array $data, array $config, $tokenId = '', $timeout = 60) { if (!$this->scriptExecutor) { throw new ScriptLanguageNotSupported($this->language); } - $this->errorHandling = $errorHandling; $runner = new ScriptRunner($this->scriptExecutor); $runner->setTokenId($tokenId); $user = User::find($this->run_as_user_id); @@ -154,52 +139,7 @@ public function runScript(array $data, array $config, $tokenId = '', $errorHandl throw new \RuntimeException('A user is required to run scripts'); } - try { - return $runner->run($this->code, $data, $config, $this->timeout(), $user); - } catch (ScriptException | \RuntimeException $e) { - if ($this->retryAttempts() && $this->retryWaitTime() !== null) { - Log::info('Retry the runScript process. Attempt ' . $this->attemptedRetries . ' of ' . $this->retryAttempts()); - if ($this->attemptedRetries > $this->retryAttempts()) { - $message = __('Script failed after :attempts total attempts', ['attempts' => $this->attemptedRetries]); - $message = $message . "\n" . $e->getMessage(); - Log::error($message); - - $this->sendExecutionErrorNotification($message, $tokenId, $errorHandling); - - if ($e instanceof ScriptTimeoutException) { - throw new ScriptTimeoutException($message); - } - throw new ScriptException($message); - } - - Log::info("Waiting {$this->retryWaitTime()} seconds before retrying."); - sleep($this->retryWaitTime()); - $this->attemptedRetries++; - - Log::info('Re-running the script'); - $result = $this->runScript($data, $config, $tokenId, $errorHandling); - Log::info('The script completed successfully'); - - return $result; - } else { - $this->sendExecutionErrorNotification($e->getMessage(), $tokenId, $errorHandling); - throw $e; - } - } - } - - /** - * Send execution error notification. - */ - public function sendExecutionErrorNotification(string $message, string $tokenId, array $errorHandling) - { - $processRequestToken = ProcessRequestToken::find($tokenId); - if ($processRequestToken) { - $user = $processRequestToken->processRequest->processVersion->manager; - if ($user !== null) { - Notification::send($user, new ErrorExecutionNotification($processRequestToken, $message, $errorHandling)); - } - } + return $runner->run($this->code, $data, $config, $timeout, $user); } /** @@ -373,48 +313,4 @@ private function setDefaultExecutor() $this->language = $this->scriptExecutor->language; } } - - /** - * This method retrieves the error number by type. - * In case of not finding it, -1 is returned. - */ - private function getErrorHandlingNumberByType($type) - { - $result = Arr::get($this->errorHandling, $type, null); - if (is_numeric($result)) { - return (int) $result; - } - - return -1; - } - - /** - * Get the value of the timeout property if there hasn't been a defined error in the errorHandling array. - */ - private function timeout() - { - $result = $this->getErrorHandlingNumberByType('timeout'); - - return $result === -1 ? $this->timeout : $result; - } - - /** - * Get the value of the retryAttempts property if there hasn't been a defined error in the errorHandling array. - */ - private function retryAttempts() - { - $result = $this->getErrorHandlingNumberByType('retry_attempts'); - - return $result === -1 ? $this->retry_attempts : $result; - } - - /** - * Get the value of the retryWaitTime property if there hasn't been a defined error in the errorHandling array. - */ - private function retryWaitTime() - { - $result = $this->getErrorHandlingNumberByType('retry_wait_time'); - - return $result === -1 ? $this->retry_wait_time : $result; - } } diff --git a/ProcessMaker/Models/ScriptDockerBindingFilesTrait.php b/ProcessMaker/Models/ScriptDockerBindingFilesTrait.php index 201061bc1e..2e84c92548 100644 --- a/ProcessMaker/Models/ScriptDockerBindingFilesTrait.php +++ b/ProcessMaker/Models/ScriptDockerBindingFilesTrait.php @@ -83,7 +83,7 @@ private function runContainer($image, $command, $parameters, $bindings, $timeout . implode("\n", $output) ); } - Log::error('Script threw return code ' . $returnCode . 'Message: ' . implode("\n", $output)); + Log::error('Script threw return code ' . $returnCode . ' Message: ' . implode("\n", $output)); $message = implode("\n", $output); $message .= "\n\nProcessMaker Stack:\n"; diff --git a/ProcessMaker/Models/ScriptVersion.php b/ProcessMaker/Models/ScriptVersion.php index 1b79b9def3..1fb3674f19 100644 --- a/ProcessMaker/Models/ScriptVersion.php +++ b/ProcessMaker/Models/ScriptVersion.php @@ -50,7 +50,7 @@ public function parent() * @param array $data * @param array $config */ - public function runScript(array $data, array $config, $tokenId = '', $errorHandling = []) + public function runScript(array $data, array $config, $tokenId = '') { $script = $this->parent->replicate(); $except = $script->getGuarded(); @@ -58,7 +58,7 @@ public function runScript(array $data, array $config, $tokenId = '', $errorHandl $script->$prop = $this->$prop; } - return $script->runScript($data, $config, $tokenId, $errorHandling); + return $script->runScript($data, $config, $tokenId); } /** diff --git a/ProcessMaker/Nayra/Managers/WorkflowManagerDefault.php b/ProcessMaker/Nayra/Managers/WorkflowManagerDefault.php index e82a230fd3..b97c351546 100644 --- a/ProcessMaker/Nayra/Managers/WorkflowManagerDefault.php +++ b/ProcessMaker/Nayra/Managers/WorkflowManagerDefault.php @@ -357,11 +357,11 @@ class_exists($this->serviceTaskImplementations[$implementation]); * * @return mixed */ - public function runServiceImplementation($implementation, array $data, array $config, $tokenId = '', $errorHandling = []) + public function runServiceImplementation($implementation, array $data, array $config, $tokenId = '', $timeout = 0) { $class = $this->serviceTaskImplementations[$implementation]; $service = new $class(); - return $service->run($data, $config, $tokenId, $errorHandling); + return $service->run($data, $config, $tokenId, $timeout); } } diff --git a/ProcessMaker/Notifications/ErrorExecutionNotification.php b/ProcessMaker/Notifications/ErrorExecutionNotification.php index d5873299a3..60775b5718 100644 --- a/ProcessMaker/Notifications/ErrorExecutionNotification.php +++ b/ProcessMaker/Notifications/ErrorExecutionNotification.php @@ -3,7 +3,6 @@ namespace ProcessMaker\Notifications; use Illuminate\Bus\Queueable; -use Illuminate\Contracts\Queue\ShouldQueue; use Illuminate\Notifications\Messages\BroadcastMessage; use Illuminate\Notifications\Messages\MailMessage; use Illuminate\Notifications\Notification; diff --git a/ProcessMaker/Traits/MakeHttpRequests.php b/ProcessMaker/Traits/MakeHttpRequests.php index 4427853f10..dec3c38140 100644 --- a/ProcessMaker/Traits/MakeHttpRequests.php +++ b/ProcessMaker/Traits/MakeHttpRequests.php @@ -6,20 +6,16 @@ use GuzzleHttp\Client; use GuzzleHttp\Exception\ClientException; use GuzzleHttp\Exception\GuzzleException; -use GuzzleHttp\Exception\TransferException; use GuzzleHttp\Psr7\Request; use GuzzleHttp\Psr7\Response; use Illuminate\Support\Arr; use Illuminate\Support\Facades\Log; -use Illuminate\Support\Facades\Notification; use Mustache_Engine; use ProcessMaker\Exception\HttpInvalidArgumentException; use ProcessMaker\Exception\HttpResponseException; use ProcessMaker\Helpers\StringHelper; +use ProcessMaker\Jobs\ErrorHandling; use ProcessMaker\Models\FormalExpression; -use ProcessMaker\Models\ProcessRequest; -use ProcessMaker\Models\ProcessRequestToken; -use ProcessMaker\Notifications\ErrorExecutionNotification; use Psr\Http\Message\ResponseInterface; trait MakeHttpRequests @@ -41,20 +37,6 @@ trait MakeHttpRequests private $timeout = 0; - private $retryAttempts = 0; - - private $retryWaitTime = 0; - - private $attemptedRetries = 0; - - private $retryMessage = null; - - private $inappNotification = false; - - private $emailNotification = false; - - private $tokenId = null; - private function getMustache() { if ($this->mustache === null) { @@ -82,64 +64,8 @@ public function request(array $data = [], array $config = []) $request = $this->prepareRequestWithOutboundConfig($data, $config); return $this->responseWithHeaderData($this->call(...$request), $data, $config); - } catch (TransferException $exception) { - $result = $this->handleRetries($data, $config); - if ($result) { - // Retry was successful - return $result; - } - - $message = $exception->getMessage(); - $message = $this->retryMessage ? $this->retryMessage . "\n" . $message : $message; - - $this->sendErrorNotification($data, $message); - - if ($exception instanceof ClientException) { - $response = $exception->getResponse(); - throw new HttpResponseException($response, $message); - } else { - throw new \Exception($message); - } - } - } - - /** - * Handle retry attempts if configured - */ - private function handleRetries(array $data, array $config) - { - if ($this->retryAttempts === 0) { - return false; - } - - if ($this->attemptedRetries >= $this->retryAttempts) { - $this->retryMessage = __('Failed after :num total attempts', ['num' => $this->attemptedRetries + 1]); - - return false; - } - - $this->attemptedRetries++; - - if ($this->retryWaitTime > 0) { - sleep($this->retryWaitTime); - } - Log::info('Retry the runScript process. Attempt ' . ($this->attemptedRetries) . ' of ' . $this->retryAttempts); - - return $this->request($data, $config); - } - - private function sendErrorNotification(array $data, string $message) - { - if ($this->tokenId) { - $processRequestToken = ProcessRequestToken::findOrFail($this->tokenId); - $user = $processRequestToken->processRequest->processVersion->manager; - if ($user !== null) { - $errorHandling = [ - 'inapp_notification' => $this->inappNotification, - 'email_notification' => $this->emailNotification, - ]; - Notification::send($user, new ErrorExecutionNotification($processRequestToken, $message, $errorHandling)); - } + } catch (ClientException $exception) { + throw new HttpResponseException($exception->getResponse()); } } @@ -175,53 +101,6 @@ private function prepareRequest(array &$data, array &$config) return $request; } - /** - * Set error handling properties from endpoint config or from BPMN errorHandling - * - * @param array $endpoint - * @param array $config - * - * @return void - */ - private function setErrorHandlingProperties(array $endpoint, array $config) - { - $properties = [ - 'timeout' => 'timeout', - 'retryAttempts' => 'retry_attempts', - 'retryWaitTime' => 'retry_wait_time', - ]; - - foreach ($properties as $classProperty => $settingProperty) { - $this->$classProperty = (int) Arr::get($endpoint, $settingProperty, 0); - $bpmnSetting = Arr::get($config, "errorHandling.{$settingProperty}", null); - if (is_numeric($bpmnSetting)) { - $this->$classProperty = (int) $bpmnSetting; - } - } - - $this->inappNotification = Arr::get($endpoint, 'inapp_notification', false); - $this->emailNotification = Arr::get($endpoint, 'email_notification', false); - - $this->tokenId = Arr::get($config, 'tokenId', null); - - if (Arr::get($config, 'errorHandling.inapp_notification', false) === true) { - $this->inappNotification = true; - } - if (Arr::get($config, 'errorHandling.email_notification', false) === true) { - $this->emailNotification = true; - } - } - - /** - * Return the value of $this->timeout - * - * @return int - */ - public function getTimeout() - { - return $this->timeout; - } - /** * Prepare data to be used in body (mustache) * @@ -326,8 +205,6 @@ private function prepareRequestWithOutboundConfig(array $requestData, array &$co $request = [$method, $url, $headers, $body, $bodyType]; $request = $this->addAuthorizationHeaders(...$request); - $this->setErrorHandlingProperties($endpoint, $config); - return $request; } @@ -556,10 +433,9 @@ private function call($method, $url, array $headers, $body, $bodyType) $client = $this->client ?? app()->make(Client::class, [ 'config' => [ 'verify' => $this->verifySsl, - 'timeout' => $this->getTimeout(), + 'timeout' => $this->timeout, ], ]); - $options = []; if ($bodyType === 'form-data') { $options['form_params'] = json_decode($body, true); diff --git a/resources/lang/en.json b/resources/lang/en.json index dcc2470bdd..a970aa0246 100644 --- a/resources/lang/en.json +++ b/resources/lang/en.json @@ -1768,8 +1768,8 @@ "Set maximum run retry wait time in seconds. Leave empty to use data connector default. Set to 0 for no retry wait time.": "Set maximum run retry wait time in seconds. Leave empty to use data connector default. Set to 0 for no retry wait time.", "Script failed after :attempts total attempts": "Script failed after :attempts total attempts", "Failed after :num total attempts": "Failed after :num total attempts", - "Seconds to wait before retrying. Leave empty to use script default. Set to 0 for no retry wait time.": "Seconds to wait before retrying. Leave empty to use script default. Set to 0 for no retry wait time.", - "Number of times to retry. Leave empty to use script default. Set to 0 for no retry attempts.": "Number of times to retry. Leave empty to use script default. Set to 0 for no retry attempts.", + "Seconds to wait before retrying. Leave empty to use script default. Set to 0 for no retry wait time. This setting is only used when running a script task in a process.": "Seconds to wait before retrying. Leave empty to use script default. Set to 0 for no retry wait time. This setting is only used when running a script task in a process.", + "Number of times to retry. Leave empty to use script default. Set to 0 for no retry attempts. This setting is only used when running a script task in a process.": "Number of times to retry. Leave empty to use script default. Set to 0 for no retry attempts. This setting is only used when running a script task in a process.", "View Request": "View Request", "Execution Error": "Execution Error" } diff --git a/resources/views/processes/scripts/edit.blade.php b/resources/views/processes/scripts/edit.blade.php index ea1fa7ed9b..199ae3eaa6 100644 --- a/resources/views/processes/scripts/edit.blade.php +++ b/resources/views/processes/scripts/edit.blade.php @@ -85,7 +85,7 @@ $bpmnTimeout, 'retry_attempts' => $bpmnRetryAttempts, 'retry_wait_time' => $bpmnRetryWaitTime]; + $element = Mockery::mock(); + $element->shouldReceive('getProperty')->andReturn(json_encode($errorHandling)); + + $script = Script::factory()->create(['timeout' => $modelTimeout, 'retry_attempts' => $modelRetryAttempts, 'retry_wait_time' => $modelRetryWaitTime]); + + $process = Process::factory()->create(); + $processRequest = ProcessRequest::factory()->create([ + 'process_id' => $process->id, + ]); + $token = ProcessRequestToken::factory()->create([ + 'process_request_id' => $processRequest->id, + ]); + + $job = new RunScriptTask($process, $processRequest, $token, [], $attempt); + $errorHandling = new ErrorHandling($element, $token); + $errorHandling->setDefaultsFromScript($script->getLatestVersion()); + $this->assertEquals($expectedTimeout, $errorHandling->timeout()); + $errorHandling->handleRetries($job, new \RuntimeException('error')); + + if ($expectedNextAttempt !== false) { + Queue::assertPushed(RunScriptTask::class, function ($job) use ($expectedNextAttempt, $expectedWaitTime) { + return $job->attemptNum === $expectedNextAttempt && + $job->delay === $expectedWaitTime; + }); + } else { + Queue::assertNotPushed(RunScriptTask::class); + } + } + + public function testRetry() + { + $this->runAssertions([ + 'attempt' => 3, + 'expectedTimeout' => 15, + 'expectedNextAttempt' => 4, // retry the job + 'expectedWaitTime' => 5, + + 'modelTimeout' => 8, + 'modelRetryAttempts' => 2, + 'modelRetryWaitTime' => 6, + 'bpmnTimeout' => 15, + 'bpmnRetryAttempts' => 3, + 'bpmnRetryWaitTime' => 5, + ]); + } + + public function testRetryUseModelSettings() + { + $this->runAssertions([ + 'attempt' => 3, + 'expectedTimeout' => 15, + 'expectedNextAttempt' => 4, // retry the job + 'expectedWaitTime' => 5, + + 'modelTimeout' => 8, + 'modelRetryAttempts' => 99, + 'modelRetryWaitTime' => 6, + 'bpmnTimeout' => 15, + 'bpmnRetryAttempts' => '', // use modelRetryAttempts + 'bpmnRetryWaitTime' => 5, + ]); + } + + public function testDoNotRetry() + { + $this->runAssertions([ + 'attempt' => 4, + 'expectedTimeout' => 15, + 'expectedNextAttempt' => false, // do not retry the job because attempt is 4 and bpmnRetryAttempts is 3 + 'expectedWaitTime' => 5, + + 'modelTimeout' => 8, + 'modelRetryAttempts' => 2, + 'modelRetryWaitTime' => 6, + 'bpmnTimeout' => 15, + 'bpmnRetryAttempts' => 3, + 'bpmnRetryWaitTime' => 5, + ]); + } + + public function testRetryWaitFromModel() + { + $this->runAssertions([ + 'attempt' => 3, + 'expectedTimeout' => 15, + 'expectedNextAttempt' => 4, + 'expectedWaitTime' => 6, // use modelRetryWaitTime because bpmnWaitTime is empty + + 'modelTimeout' => 8, + 'modelRetryAttempts' => 2, + 'modelRetryWaitTime' => 6, + 'bpmnTimeout' => 15, + 'bpmnRetryAttempts' => 3, + 'bpmnRetryWaitTime' => '', + ]); + } + + public function testTimeoutFromModel() + { + $this->runAssertions([ + 'attempt' => 3, + 'expectedTimeout' => 8, // use modelTimeout because bpmnTimeout is empty + 'expectedNextAttempt' => 4, + 'expectedWaitTime' => 5, + + 'modelTimeout' => 8, + 'modelRetryAttempts' => 2, + 'modelRetryWaitTime' => 6, + 'bpmnTimeout' => '', + 'bpmnRetryAttempts' => 3, + 'bpmnRetryWaitTime' => 5, + ]); + } +} diff --git a/tests/Model/ScriptTest.php b/tests/Model/ScriptTest.php deleted file mode 100644 index 9af426c314..0000000000 --- a/tests/Model/ScriptTest.php +++ /dev/null @@ -1,86 +0,0 @@ -mockWithExpectedTimeout(123); - - $script = Script::factory()->create([ - 'code' => 'foo', - 'timeout' => 5, - 'retry_attempts' => 3, - 'retry_wait_time' => 1, - ]); - $script->runScript([], [], '', ['timeout' => 123]); - } - - public function testUseTimeoutFromSetting() - { - $this->mockWithExpectedTimeout(5); - - $script = Script::factory()->create([ - 'code' => 'foo', - 'timeout' => 5, - 'retry_attempts' => 3, - 'retry_wait_time' => 1, - ]); - $script->runScript([], [], '', ['timeout' => '']); - } - - private function mockWithExpectedTimeout($timeout) - { - $mock = Mockery::mock(MockRunner::class); - $mock->shouldReceive('run')->once()->with('foo', [], [], $timeout, Mockery::any()); - $mock->shouldReceive('setTokenId')->once(); - app()->bind(MockRunner::class, function () use ($mock) { - return $mock; - }); - } - - /** - * This test performs a test of retry_attempts and the retry_wait_time for script execution. - */ - public function testUseRetryAttemptsAndRetryWaitTime() - { - //Mockery needs to know about this retry. - $times = 3; - $this->mockWithExpectedTimeoutWithException(2, $times + 1); // Add 1 for original call - - $this->expectException(ScriptException::class); - - $script = Script::factory()->create([ - 'code' => 'foo', - 'timeout' => 2, - 'retry_attempts' => $times, - 'retry_wait_time' => 1, - ]); - $script->runScript([], [], '', ['timeout' => '']); - } - - /** - * It is necessary to pass this value to Mockery because we are attempting retries. Mockery needs - * to know about this retry. - */ - private function mockWithExpectedTimeoutWithException($timeout, $times) - { - $mock = Mockery::mock(MockRunner::class); - $mock->shouldReceive('setTokenId')->times($times); - $mock->shouldReceive('run')->times($times)->with('foo', [], [], $timeout, Mockery::any())->andReturnUsing(function () { - throw new \RuntimeException('Error occurred'); - }); - - app()->bind(MockRunner::class, function () use ($mock) { - return $mock; - }); - } -}