Skip to content

Commit ba02a61

Browse files
nolanpronolanpro
committed
WIP
1 parent d27abb2 commit ba02a61

File tree

6 files changed

+148
-36
lines changed

6 files changed

+148
-36
lines changed

‎ProcessMaker/Jobs/CheckScriptTimeout.php

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
<?php
2+
3+
namespace ProcessMaker\Jobs;
4+
5+
use Illuminate\Bus\Queueable;
6+
use Illuminate\Contracts\Queue\ShouldQueue;
7+
use Illuminate\Foundation\Bus\Dispatchable;
8+
use Illuminate\Queue\InteractsWithQueue;
9+
use Illuminate\Queue\SerializesModels;
10+
use Illuminate\Support\Facades\Cache;
11+
use ProcessMaker\Exception\ScriptTimeoutException;
12+
13+
/**
14+
* The microservice handles script timeouts but this is an additional check
15+
* in case the microservice does not respond.
16+
*/
17+
class CheckScriptTimeout implements ShouldQueue
18+
{
19+
use Dispatchable, InteractsWithQueue, Queueable, SerializesModels;
20+
21+
protected string $executionId;
22+
23+
const ADDITIONAL_TIME = 5;
24+
25+
public function __construct(string $executionId)
26+
{
27+
$this->executionId = $executionId;
28+
}
29+
30+
public function handle()
31+
{
32+
if (Cache::has($this->executionId)) {
33+
throw new ScriptTimeoutException('Script execution timed out for execution ID: ' . $this->executionId);
34+
}
35+
}
36+
37+
public static function start(string $executionId, int $timeout)
38+
{
39+
Cache::put($executionId, 'timeout: ' . $timeout, 86400); // hold in cache for a maximum of 24 hours
40+
self::dispatch($executionId)
41+
->delay(now()->addSeconds($timeout + self::ADDITIONAL_TIME));
42+
}
43+
44+
public static function stop(string $executionId)
45+
{
46+
Cache::forget($executionId);
47+
}
48+
}

‎ProcessMaker/Jobs/ErrorHandling.php

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -116,22 +116,22 @@ private function requeue()
116116

117117
private function getData()
118118
{
119-
return $this->job ? $this->job->data : $this->metadata['data'];
119+
return $this->job ? $this->job->data : $this->metadata['script_task']['data'];
120120
}
121121

122122
private function getAttemptNumber()
123123
{
124-
return $this->job ? $this->job->attemptNum : $this->metadata['attemptNum'];
124+
return $this->job ? $this->job->attemptNum : $this->metadata['script_task']['attempt_num'];
125125
}
126126

127127
private function getMessage()
128128
{
129-
$this->exception->getMessage();
129+
return $this->exception->getMessage();
130130
}
131131

132132
private function getJobClass()
133133
{
134-
return $this->job ? get_class($this->job) : $this->metadata['job_class'];
134+
return $this->job ? get_class($this->job) : $this->metadata['script_task']['job_class'];
135135
}
136136

137137
/**
@@ -253,4 +253,14 @@ public function setDefaultsFromDataSourceConfig(array $config)
253253
'retry_wait_time' => Arr::get($endpointConfig, 'retry_wait_time', 5),
254254
];
255255
}
256+
257+
public static function convertErrorResponseToException($response)
258+
{
259+
if ($response['status'] === 'error') {
260+
if (str_starts_with($response['message'], 'Command exceeded timeout of')) {
261+
throw new ScriptTimeoutException($response['message']);
262+
}
263+
throw new ScriptException($response['message']);
264+
}
265+
}
256266
}

‎ProcessMaker/Jobs/RunScriptTask.php

Lines changed: 37 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -105,11 +105,11 @@ public function action(ProcessRequestToken $token = null, ScriptTaskInterface $e
105105
'instance_id' => $this->instanceId,
106106
'token_id' => $this->tokenId,
107107
'data' => $data,
108-
'attempts' => $this->attemptNum,
108+
'attempt_num' => $this->attemptNum,
109109
'job_class' => get_class($this),
110110
],
111111
];
112-
$response = $script->runScript($data, $configuration, $token->getId(), $errorHandling->timeout(), 1, $metadata);
112+
$response = $script->runScript($data, $configuration, $token->getId(), $errorHandling->timeout(), 0, $metadata);
113113

114114
if (!config('script-runner-microservice.enabled') ||
115115
($scriptExecutor && $scriptExecutor->type === ScriptExecutorType::Custom)) {
@@ -127,20 +127,38 @@ public function action(ProcessRequestToken $token = null, ScriptTaskInterface $e
127127
$message = $exception->getMessage();
128128
}
129129

130-
if ($finalAttempt) {
131-
$token->setStatus(ScriptTaskInterface::TOKEN_STATE_FAILING);
132-
}
130+
self::setErrorStatus($finalAttempt, $token, $element, $message, $scriptRef, $exception, false);
131+
}
132+
}
133133

134-
$error = $element->getRepository()->createError();
135-
$error->setName($message);
134+
public static function setErrorStatus(
135+
bool $finalAttempt,
136+
ProcessRequestToken $token,
137+
$element,
138+
$message,
139+
$scriptRef,
140+
Throwable $exception,
141+
bool $isMicroservice
142+
) {
143+
if ($finalAttempt) {
144+
$token->setStatus(ScriptTaskInterface::TOKEN_STATE_FAILING);
145+
}
136146

137-
$token->setProperty('error', $error);
138-
$exceptionClass = get_class($exception);
139-
$modifiedException = new $exceptionClass($message);
140-
$token->logError($modifiedException, $element);
147+
$error = $element->getRepository()->createError();
148+
$error->setName($message);
141149

142-
Log::error('Script failed: ' . $scriptRef . ' - ' . $message);
143-
Log::error($exception->getTraceAsString());
150+
$token->setProperty('error', $error);
151+
$exceptionClass = get_class($exception);
152+
$modifiedException = new $exceptionClass($message);
153+
$token->logError($modifiedException, $element);
154+
155+
Log::error('Script failed: ' . $scriptRef . ' - ' . $message);
156+
Log::error($exception->getTraceAsString());
157+
158+
// If it's from the microservice, we need to call handleFailed()
159+
// manually here because failed() is only called when it's a job.
160+
if ($isMicroservice) {
161+
self::handleFailed($exception, $token->id);
144162
}
145163
}
146164

@@ -170,7 +188,12 @@ private function updateData($response)
170188
*/
171189
public function failed(Throwable $exception)
172190
{
173-
if (!$this->tokenId) {
191+
self::handleFailed($exception, $this->tokenId);
192+
}
193+
194+
private static function handleFailed(Throwable $exception, $tokenId)
195+
{
196+
if (!$tokenId) {
174197
Log::error('Script failed: ' . $exception->getMessage());
175198

176199
return;

‎ProcessMaker/ScriptRunners/ScriptMicroserviceRunner.php

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,10 @@
66
use Illuminate\Support\Facades\Cache;
77
use Illuminate\Support\Facades\Http;
88
use Illuminate\Support\Facades\Log;
9+
use Illuminate\Support\Str;
910
use ProcessMaker\Exception\ConfigurationException;
1011
use ProcessMaker\GenerateAccessToken;
12+
use ProcessMaker\Jobs\CheckScriptTimeout;
1113
use ProcessMaker\Jobs\ErrorHandling;
1214
use ProcessMaker\Models\EnvironmentVariable;
1315
use ProcessMaker\Models\Script;
@@ -95,7 +97,9 @@ public function run($code, array $data, array $config, $timeout, $user, $sync, $
9597
$result = $response->json();
9698

9799
if ($sync) {
98-
ErrorHandling::convertResponseToException($result);
100+
ErrorHandling::convertErrorResponseToException($result);
101+
} else {
102+
CheckScriptTimeout::start($metadata['execution_id'], $timeout);
99103
}
100104

101105
return $result;
@@ -154,6 +158,7 @@ public function getMetadata($user)
154158
'instance' => config('app.url'),
155159
'user_id' => $user->id,
156160
'user_email' => $user->email,
161+
'execution_id' => 'script-execution-' . Str::uuid(),
157162
];
158163
}
159164

‎ProcessMaker/Services/ScriptMicroserviceService.php

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use ProcessMaker\Events\ScriptResponseEvent;
88
use ProcessMaker\Exception\ScriptException;
99
use ProcessMaker\Exception\ScriptTimeoutException;
10+
use ProcessMaker\Jobs\CheckScriptTimeout;
1011
use ProcessMaker\Jobs\CompleteActivity;
1112
use ProcessMaker\Jobs\ErrorHandling;
1213
use ProcessMaker\Jobs\RunScriptTask;
@@ -36,6 +37,11 @@ public function handle(Request $request)
3637
null,
3738
$response['metadata']['nonce']));
3839
}
40+
41+
if ($response['metadata']['execution_id']) {
42+
CheckScriptTimeout::stop($response['metadata']['execution_id']);
43+
}
44+
3945
if (!empty($response['metadata']['script_task'])) {
4046
$script = Script::find($response['metadata']['script_task']['script_id']);
4147
$definitions = Definitions::find($response['metadata']['script_task']['definition_id']);
@@ -47,21 +53,13 @@ public function handle(Request $request)
4753
}
4854

4955
try {
50-
$this->convertResponseToException($response);
56+
ErrorHandling::convertErrorResponseToException($response);
5157
} catch (ScriptException|ScriptTimeoutException $e) {
52-
$element = $definitions->getElementInstanceById($token->element_id);
58+
$element = $definitions->getDefinitions(true)->getElementInstanceById($token->element_id);
5359
$errorHandling = new ErrorHandling($element, $token);
54-
$errorHandling->handleRetriesForScriptMicroservice($e, $response['metadata']);
55-
}
56-
}
57-
58-
public function convertResponseToException($response)
59-
{
60-
if ($response['status'] === 'error') {
61-
if (str_starts_with($response['message'], 'Command exceeded timeout of')) {
62-
throw new ScriptTimeoutException($response['message']);
63-
}
64-
throw new ScriptException($response['message']);
60+
$errorHandling->setDefaultsFromScript($script->versionFor($instance));
61+
[$message, $finalAttempt] = $errorHandling->handleRetriesForScriptMicroservice($e, $response['metadata']);
62+
RunScriptTask::setErrorStatus($finalAttempt, $token, $element, $message, $script->id, $e, true);
6563
}
6664
}
6765
}

‎tests/Jobs/ErrorHandlingTest.php

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ private function runAssertions($settings)
2020

2121
extract($settings);
2222
$errorHandling = ['timeout' => $bpmnTimeout, 'retry_attempts' => $bpmnRetryAttempts, 'retry_wait_time' => $bpmnRetryWaitTime];
23-
$element = Mockery::mock();
23+
$element = Mockery::mock(\ProcessMaker\Nayra\Contracts\Bpmn\ActivityInterface::class);
2424
$element->shouldReceive('getProperty')->andReturn(json_encode($errorHandling));
2525

2626
$script = Script::factory()->create(['timeout' => $modelTimeout, 'retry_attempts' => $modelRetryAttempts, 'retry_wait_time' => $modelRetryWaitTime]);
@@ -33,16 +33,44 @@ private function runAssertions($settings)
3333
'process_request_id' => $processRequest->id,
3434
]);
3535

36-
$job = new RunScriptTask($process, $processRequest, $token, [], $attempt);
36+
$job = new RunScriptTask($process, $processRequest, $token, ['foo' => 'baz'], $attempt);
3737
$errorHandling = new ErrorHandling($element, $token);
3838
$errorHandling->setDefaultsFromScript($script->getLatestVersion());
3939
$this->assertEquals($expectedTimeout, $errorHandling->timeout());
4040
$errorHandling->handleRetries($job, new \RuntimeException('error'));
4141

42+
$expectedData = ['foo' => 'baz'];
43+
44+
$this->assertQueue($expectedNextAttempt, $expectedWaitTime, $expectedData);
45+
46+
// Test handleRetriesForScriptMicroservice
47+
48+
$metadata = [
49+
'script_task' => [
50+
'job_class' => RunScriptTask::class,
51+
'data' => ['foo' => 'bar'],
52+
'attempt_num' => $attempt,
53+
'message' => 'error',
54+
],
55+
];
56+
57+
Queue::fake();
58+
59+
$expectedData = ['foo' => 'bar'];
60+
$errorHandling = new ErrorHandling($element, $token);
61+
$errorHandling->setDefaultsFromScript($script->getLatestVersion());
62+
$errorHandling->handleRetriesForScriptMicroservice(new \RuntimeException('error'), $metadata);
63+
64+
$this->assertQueue($expectedNextAttempt, $expectedWaitTime, $expectedData);
65+
}
66+
67+
private function assertQueue($expectedNextAttempt, $expectedWaitTime, $expectedData)
68+
{
4269
if ($expectedNextAttempt !== false) {
43-
Queue::assertPushed(RunScriptTask::class, function ($job) use ($expectedNextAttempt, $expectedWaitTime) {
70+
Queue::assertPushed(RunScriptTask::class, function ($job) use ($expectedNextAttempt, $expectedWaitTime, $expectedData) {
4471
return $job->attemptNum === $expectedNextAttempt &&
45-
$job->delay === $expectedWaitTime;
72+
$job->delay === $expectedWaitTime &&
73+
$job->data === $expectedData;
4674
});
4775
} else {
4876
Queue::assertNotPushed(RunScriptTask::class);

0 commit comments

Comments
 (0)