Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
3805813
Add error handling to inspector
nolanpro May 22, 2023
29d7675
Merge branch 'develop' into feature/FOUR-8061
nolanpro May 22, 2023
881b961
Remove optional errorHandling from the interface
nolanpro May 22, 2023
053189b
Add errorHandler to scripts and data sources
nolanpro May 24, 2023
f9291d4
Add shared error handling settings
nolanpro May 25, 2023
d8078a1
Allow null errorHandling
nolanpro May 25, 2023
2a966cf
FIx timeout from bpmn
nolanpro May 25, 2023
c0e75c9
Remove old timeout control
nolanpro May 25, 2023
f51fc67
Improve error messaging
nolanpro May 26, 2023
2d8e9cb
Set min and max values for timeouts
nolanpro May 30, 2023
ef1f0a6
FOUR-8064 Automated Retries
gproly Jun 2, 2023
ebc9106
Merge branch 'feature/FOUR-8059' into feature/FOUR-8061
nolanpro Jun 2, 2023
779472b
Add test for script retry setting
nolanpro Jun 1, 2023
ba4c65a
Add support for retries
nolanpro Jun 1, 2023
c184839
Update error handling to include note in body
nolanpro Jun 1, 2023
57c437a
Add min and max to timeout setting
nolanpro Jun 6, 2023
5bc38ac
FOUR-8064 Automated Retries, correction observations.
gproly Jun 7, 2023
3d843fa
Merge pull request #4803 from ProcessMaker/feature/FOUR-8061
nolanpro Jun 7, 2023
9fe77bd
Merge branch 'feature/FOUR-8059' into feature/FOUR-8064
nolanpro Jun 7, 2023
14619bc
Update error handling settings
nolanpro Jun 8, 2023
3d508da
FOUR-8064 fixed observations
gproly Jun 9, 2023
e7726b4
Merge pull request #4837 from ProcessMaker/feature/FOUR-8064
nolanpro Jun 9, 2023
c4a1417
Throw original error if not retries are configured
nolanpro Jun 12, 2023
9358dea
Merge branch 'develop' into feature/FOUR-8059
ryancooley Jun 13, 2023
f26df97
FIx tests
nolanpro Jun 13, 2023
1ee41d5
Fix missing quote
nolanpro Jun 14, 2023
0f72723
Merge branch 'develop' into feature/FOUR-8059
nolanpro Jun 15, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions ProcessMaker/Exception/HttpResponseException.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,18 @@ class HttpResponseException extends Exception implements HttpExceptionInterface
/**
* @param Response $response
*/
public function __construct(Response $response)
public function __construct(Response $response, $note = null)
{
$this->status = $response->getStatusCode();
$this->body = $response->getBody()->getContents();
$this->headers = $response->getHeaders();
if ($note) {
$this->body = $note . "\n" . $this->body;
}
$body = Str::limit($this->body, 100);
parent::__construct(__("Unexpected response (status=:status)\n:body", [
'status' => $this->status,
'body' => Str::limit($this->body, 100),
'body' => $body,
]), 0);
}

Expand Down
3 changes: 2 additions & 1 deletion ProcessMaker/Jobs/RunScriptTask.php
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ 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);

// Check to see if we've failed parsing. If so, let's convert to empty array.
if ($configuration === null) {
Expand All @@ -85,7 +86,7 @@ public function action(ProcessRequestToken $token = null, ScriptTaskInterface $e
$this->unlock();
$dataManager = new DataManager();
$data = $dataManager->getData($token);
$response = $script->runScript($data, $configuration, $token->getId());
$response = $script->runScript($data, $configuration, $token->getId(), $errorHandling);

$this->withUpdatedContext(function ($engine, $instance, $element, $processModel, $token) use ($response) {
// Exit if the task was completed or closed
Expand Down
6 changes: 4 additions & 2 deletions ProcessMaker/Jobs/RunServiceTask.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ 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) {
$configuration = [];
Expand All @@ -88,10 +90,10 @@ public function action(ProcessRequestToken $token = null, ServiceTaskInterface $

if ($existsImpl) {
$response = [
'output' => WorkflowManager::runServiceImplementation($implementation, $data, $configuration, $token->getId()),
'output' => WorkflowManager::runServiceImplementation($implementation, $data, $configuration, $token->getId(), $errorHandling),
];
} else {
$response = $script->runScript($data, $configuration, $token->getId());
$response = $script->runScript($data, $configuration, $token->getId(), $errorHandling);
}
$this->withUpdatedContext(function ($engine, $instance, $element, $processModel, $token) use ($response) {
// Exit if the task was completed or closed
Expand Down
89 changes: 87 additions & 2 deletions ProcessMaker/Models/Script.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,14 @@

namespace ProcessMaker\Models;

use Hamcrest\Type\IsNumeric;
use Illuminate\Support\Arr;
use Illuminate\Support\Facades\Log;
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;
Expand Down Expand Up @@ -77,8 +82,14 @@ class Script extends ProcessMakerModel implements ScriptInterface

protected $casts = [
'timeout' => 'integer',
'retry_attempts' => 'integer',
'retry_wait_time' => 'integer',
];

private $errorHandling = [];

private $attemptedRetries = 1;

/**
* Override the default boot method to allow access to lifecycle hooks
*
Expand Down Expand Up @@ -116,6 +127,8 @@ 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)],
];
}
Expand All @@ -126,19 +139,47 @@ public static function rules($existing = null)
* @param array $data
* @param array $config
*/
public function runScript(array $data, array $config, $tokenId = '')
public function runScript(array $data, array $config, $tokenId = '', $errorHandling = [])
{
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);
if (!$user) {
throw new \RuntimeException('A user is required to run scripts');
}

return $runner->run($this->code, $data, $config, $this->timeout, $user);
try {
return $runner->run($this->code, $data, $config, $this->timeout(), $user);
} catch (ScriptException | \RuntimeException $e) {
if ($this->retryAttempts() !== null && $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);
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 {
throw $e;
}
}
}

/**
Expand Down Expand Up @@ -312,4 +353,48 @@ 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;
}
}
4 changes: 3 additions & 1 deletion ProcessMaker/Models/ScriptDockerBindingFilesTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,9 @@ private function runContainer($image, $command, $parameters, $bindings, $timeout
Log::error('Script timed out');
throw new ScriptTimeoutException(
__('Script took too long to complete. Consider increasing the timeout.')
. ' '
. "\n"
. __('Timeout: :timeout seconds', ['timeout' => $timeout])
. "\n"
. implode("\n", $output)
);
}
Expand Down
4 changes: 2 additions & 2 deletions ProcessMaker/Models/ScriptVersion.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,15 @@ public function parent()
* @param array $data
* @param array $config
*/
public function runScript(array $data, array $config, $tokenId = '')
public function runScript(array $data, array $config, $tokenId = '', $errorHandling = [])
{
$script = $this->parent->replicate();
$except = $script->getGuarded();
foreach (collect($script->getAttributes())->except($except)->keys() as $prop) {
$script->$prop = $this->$prop;
}

return $script->runScript($data, $config, $tokenId);
return $script->runScript($data, $config, $tokenId, $errorHandling);
}

/**
Expand Down
4 changes: 2 additions & 2 deletions ProcessMaker/Nayra/Managers/WorkflowManagerDefault.php
Original file line number Diff line number Diff line change
Expand Up @@ -357,11 +357,11 @@ class_exists($this->serviceTaskImplementations[$implementation]);
*
* @return mixed
*/
public function runServiceImplementation($implementation, array $data, array $config, $tokenId = '')
public function runServiceImplementation($implementation, array $data, array $config, $tokenId = '', $errorHandling = [])
{
$class = $this->serviceTaskImplementations[$implementation];
$service = new $class();

return $service->run($data, $config, $tokenId);
return $service->run($data, $config, $tokenId, $errorHandling);
}
}
2 changes: 1 addition & 1 deletion ProcessMaker/ScriptRunners/MockRunner.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

class MockRunner
{
public function __construct($_executor)
public function __construct($scriptExecutor)
{
}

Expand Down
2 changes: 1 addition & 1 deletion ProcessMaker/ScriptRunners/ScriptRunner.php
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ private function getScriptRunner(ScriptExecutor $executor)
} else {
$class = "ProcessMaker\\ScriptRunners\\{$runner}";

return new $class($executor);
return app()->make($class, ['scriptExecutor' => $executor]);
}
}

Expand Down
96 changes: 93 additions & 3 deletions ProcessMaker/Traits/MakeHttpRequests.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
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;
Expand Down Expand Up @@ -34,6 +35,16 @@ trait MakeHttpRequests

private $mustache = null;

private $timeout = 0;

private $retryAttempts = 0;

private $retryWaitTime = 0;

private $attemptedRetries = 0;

private $retryMessage = null;

private function getMustache()
{
if ($this->mustache === null) {
Expand Down Expand Up @@ -61,9 +72,45 @@ public function request(array $data = [], array $config = [])
$request = $this->prepareRequestWithOutboundConfig($data, $config);

return $this->responseWithHeaderData($this->call(...$request), $data, $config);
} catch (ClientException $exception) {
throw new HttpResponseException($exception->getResponse());
} catch (TransferException $exception) {
$result = $this->handleRetries($data, $config);
if ($result) {
// Retry was successful
return $result;
}

if ($exception instanceof ClientException) {
$response = $exception->getResponse();
throw new HttpResponseException($response, $this->retryMessage);
} else {
$message = $exception->getMessage();
throw new \Exception($this->retryMessage ? $this->retryMessage . "\n" . $message : $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);
}

return $this->request($data, $config);
}

/**
Expand Down Expand Up @@ -98,6 +145,41 @@ 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;
}
}
}

/**
* Return the value of $this->timeout
*
* @return int
*/
public function getTimeout()
{
return $this->timeout;
}

/**
* Prepare data to be used in body (mustache)
*
Expand Down Expand Up @@ -202,6 +284,8 @@ private function prepareRequestWithOutboundConfig(array $requestData, array &$co
$request = [$method, $url, $headers, $body, $bodyType];
$request = $this->addAuthorizationHeaders(...$request);

$this->setErrorHandlingProperties($endpoint, $config);

return $request;
}

Expand Down Expand Up @@ -427,7 +511,13 @@ private function responseWithHeaderData($response, array $data = [], array $conf
*/
private function call($method, $url, array $headers, $body, $bodyType)
{
$client = $this->client ?? new Client(['verify' => $this->verifySsl]);
$client = $this->client ?? app()->make(Client::class, [
'config' => [
'verify' => $this->verifySsl,
'timeout' => $this->getTimeout(),
],
]);

$options = [];
if ($bodyType === 'form-data') {
$options['form_params'] = json_decode($body, true);
Expand Down
Loading