diff --git a/phpunit.xml.dist b/phpunit.xml.dist index d9c16f1b..04403807 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -18,6 +18,7 @@ > + tests/Acceptance/Extra tests/Acceptance/Harness diff --git a/src/Internal/Declaration/Dispatcher/Dispatcher.php b/src/Internal/Declaration/Dispatcher/Dispatcher.php index b2c459f0..afb4525c 100644 --- a/src/Internal/Declaration/Dispatcher/Dispatcher.php +++ b/src/Internal/Declaration/Dispatcher/Dispatcher.php @@ -120,7 +120,10 @@ private function createExecutorFromFunction(\ReflectionFunction $fun): \Closure throw new \BadMethodCallException($message, $type, $error); }); - return $closure->call($ctx, ...$arguments); + + return $fun->isStatic() + ? $closure->bindTo(null, $ctx::class)?->__invoke(...$arguments) ?? $closure(...$arguments) + : $closure->call($ctx, ...$arguments); } finally { \restore_error_handler(); } diff --git a/src/Internal/Workflow/Process/DeferredGenerator.php b/src/Internal/Workflow/Process/DeferredGenerator.php index 17d4a40c..0b9c2e5f 100644 --- a/src/Internal/Workflow/Process/DeferredGenerator.php +++ b/src/Internal/Workflow/Process/DeferredGenerator.php @@ -89,7 +89,7 @@ public function send(mixed $value): mixed */ public function getReturn(): mixed { - // $this->start(); + $this->finished or throw new \LogicException('Cannot get return value of a generator that was not finished.'); try { return $this->generator->getReturn(); } catch (\Throwable $e) { @@ -149,7 +149,8 @@ public function valid(): bool { $this->start(); try { - return $this->generator->valid(); + $result = $this->generator->valid() or $this->finished = true; + return $result; } catch (\Throwable $e) { $this->handleException($e); } @@ -218,9 +219,9 @@ private function handleException(\Throwable $e): never { $this->finished and throw $e; $this->finished = true; - foreach ($this->catchers as $catch) { + foreach ($this->catchers as $catcher) { try { - $catch($e); + $catcher($e); } catch (\Throwable) { // Do nothing. } diff --git a/src/Internal/Workflow/Process/Scope.php b/src/Internal/Workflow/Process/Scope.php index 1782f94b..0385ea69 100644 --- a/src/Internal/Workflow/Process/Scope.php +++ b/src/Internal/Workflow/Process/Scope.php @@ -186,7 +186,8 @@ public function startSignal(callable $handler, ValuesInterface $values, string $ */ public function attach(\Generator $generator): self { - $this->coroutine = DeferredGenerator::fromGenerator($generator); + $this->coroutine = DeferredGenerator::fromGenerator($generator) + ->catch($this->onException(...)); $this->next(); return $this; @@ -386,13 +387,13 @@ protected function next(): void begin: $this->context->resolveConditions(); - if (!$this->coroutine->valid()) { - try { + try { + if (!$this->coroutine->valid()) { $this->onResult($this->coroutine->getReturn()); - } catch (\Throwable) { - $this->onResult(null); + return; } - + } catch (\Throwable) { + $this->onResult(null); return; } @@ -418,11 +419,7 @@ protected function next(): void break; case $current instanceof \Generator: - try { - $this->nextPromise($this->createScope(false)->attach($current)); - } catch (\Throwable $e) { - $this->coroutine->throw($e); - } + $this->nextPromise($this->createScope(false)->attach($current)); break; default: diff --git a/src/Workflow.php b/src/Workflow.php index 8f4172ee..028fde50 100644 --- a/src/Workflow.php +++ b/src/Workflow.php @@ -18,6 +18,7 @@ use Temporal\Client\WorkflowStubInterface; use Temporal\DataConverter\Type; use Temporal\DataConverter\ValuesInterface; +use Temporal\Exception\Failure\CanceledFailure; use Temporal\Exception\OutOfContextException; use Temporal\Internal\Support\Facade; use Temporal\Internal\Workflow\ScopeContext; @@ -958,6 +959,9 @@ public static function uuid7(?\DateTimeInterface $dateTime = null): PromiseInter * Run a function when the mutex is released. * The mutex is locked for the duration of the function. * + * Note that calling the method creates a non-detached asynchronous context {@see Workflow::async()}. + * Closing the context using the `cancel()` method will reject the returned promise with a {@see CanceledFailure}. + * * @template T * @param Mutex $mutex Mutex name or instance. * @param callable(): T $callable Function to run. diff --git a/tests/Acceptance/Extra/Schedule/ScheduleClientTest.php b/tests/Acceptance/Extra/Schedule/ScheduleClientTest.php index a11cbdb5..02f9533f 100644 --- a/tests/Acceptance/Extra/Schedule/ScheduleClientTest.php +++ b/tests/Acceptance/Extra/Schedule/ScheduleClientTest.php @@ -40,6 +40,17 @@ public function listSchedulesWithQuery( ); } + // Wait for schedules to be created + $deadline = \microtime(true) + 5; + check: + $paginator = $client->listSchedules( + pageSize: 10, + query: 'bar = 4242' + ); + if (\count($paginator->getPageItems()) < 6 && \microtime(true) < $deadline) { + goto check; + } + try { $paginator = $client->listSchedules( pageSize: 5, diff --git a/tests/Acceptance/Extra/Update/UntypedStubTest.php b/tests/Acceptance/Extra/Update/UntypedStubTest.php index ef0daee6..05ee3d21 100644 --- a/tests/Acceptance/Extra/Update/UntypedStubTest.php +++ b/tests/Acceptance/Extra/Update/UntypedStubTest.php @@ -9,7 +9,7 @@ use Temporal\Client\WorkflowClientInterface; use Temporal\Client\WorkflowStubInterface; use Temporal\Exception\Client\TimeoutException; -use Temporal\Exception\Client\UntypedStubException; +use Temporal\Exception\Client\WorkflowUpdateException; use Temporal\Internal\Support\DateInterval; use Temporal\Tests\Acceptance\App\Attribute\Stub; use Temporal\Tests\Acceptance\App\TestCase; @@ -37,7 +37,11 @@ public function fetchResolvedResultAfterWorkflowCompleted( $this->assertSame(['key' => 'resolved'], (array)$result, 'Workflow result contains resolved value'); $this->assertFalse($handle->hasResult()); - $this->assertFalse($resolver->hasResult(), 'Resolver should not have result because of wait policy'); + + // Since Temporal CLI 1.2.0, the result is available immediately after the operation + $this->assertTrue($resolver->hasResult()); + $this->assertSame('resolved', $resolver->getResult()); + // Fetch result $this->assertSame('resolved', $handle->getResult()); $this->assertTrue($handle->hasResult()); @@ -88,7 +92,7 @@ public function handleUnknownUpdate( try { $stub->startUpdate('unknownUpdateMethod', '42'); $this->fail('Should throw exception'); - } catch (UntypedStubException $e) { + } catch (WorkflowUpdateException $e) { $this->assertStringContainsString( 'unknown update method unknownUpdateMethod', $e->getPrevious()->getMessage(), diff --git a/tests/Acceptance/Extra/Update/UpdateWithStartTest.php b/tests/Acceptance/Extra/Update/UpdateWithStartTest.php index cf95d858..a72fbd2c 100644 --- a/tests/Acceptance/Extra/Update/UpdateWithStartTest.php +++ b/tests/Acceptance/Extra/Update/UpdateWithStartTest.php @@ -60,7 +60,7 @@ public function failWithBadUpdateName( try { $stub->getResult(timeout: 1); $this->fail('Workflow must fail'); - } catch (WorkflowFailedException $e) { + } catch (WorkflowFailedException) { $this->assertTrue(true); } } diff --git a/tests/Acceptance/Extra/Workflow/AllHandlersFinishedTest.php b/tests/Acceptance/Extra/Workflow/AllHandlersFinishedTest.php index 3238afa4..f3f5ead7 100644 --- a/tests/Acceptance/Extra/Workflow/AllHandlersFinishedTest.php +++ b/tests/Acceptance/Extra/Workflow/AllHandlersFinishedTest.php @@ -33,7 +33,11 @@ public function updateHandlersWithOneCall( $this->assertSame(['key' => 'resolved'], (array) $result, 'Workflow result contains resolved value'); $this->assertFalse($handle->hasResult()); - $this->assertFalse($resolver->hasResult(), 'Resolver should not have result because of wait policy'); + + // Since Temporal CLI 1.2.0, the result is available immediately after the operation + $this->assertTrue($resolver->hasResult()); + $this->assertSame('resolved', $resolver->getResult()); + // Fetch signal's result $this->assertSame('resolved', $handle->getResult()); $this->assertTrue($handle->hasResult()); diff --git a/tests/Acceptance/Extra/Workflow/MutexRunLockedTest.php b/tests/Acceptance/Extra/Workflow/MutexRunLockedTest.php index b62a3b55..57625948 100644 --- a/tests/Acceptance/Extra/Workflow/MutexRunLockedTest.php +++ b/tests/Acceptance/Extra/Workflow/MutexRunLockedTest.php @@ -8,6 +8,7 @@ use React\Promise\PromiseInterface; use Temporal\Client\WorkflowStubInterface; use Temporal\DataConverter\Type; +use Temporal\Exception\Failure\CanceledFailure; use Temporal\Tests\Acceptance\App\Attribute\Stub; use Temporal\Tests\Acceptance\App\TestCase; use Temporal\Workflow; @@ -28,6 +29,7 @@ public function runLockedWithGeneratorAndAwait( $this->assertTrue($result[0], 'Mutex must be unlocked after runLocked is finished'); $this->assertTrue($result[1], 'The function inside runLocked mist wait for signal'); $this->assertTrue($result[2], 'Mutex must be locked during runLocked'); + $this->assertNull($result[3], 'No exception must be thrown'); } #[Test] @@ -41,6 +43,7 @@ public function runLockedAndCancel( $this->assertTrue($result[0], 'Mutex must be unlocked after runLocked is cancelled'); $this->assertNull($result[2], 'Mutex must be locked during runLocked'); + $this->assertSame(CanceledFailure::class, $result[3], 'CanceledFailure must be thrown'); } } @@ -64,7 +67,12 @@ public function __construct() #[Workflow\ReturnType(Type::TYPE_ARRAY)] public function handle(): \Generator { - $result = yield $this->promise = Workflow::runLocked($this->mutex, $this->runLocked(...)); + $exception = null; + try { + $result = yield $this->promise = Workflow::runLocked($this->mutex, $this->runLocked(...)); + } catch (\Throwable $e) { + $exception = $e::class; + } $trailed = false; yield Workflow::await( @@ -78,7 +86,7 @@ public function handle(): \Generator // that was created inside the first runLocked $trailed and throw new \Exception('The trailed runLocked must not be executed.'); - return [$this->unlocked, $this->unblock, $result]; + return [$this->unlocked, $this->unblock, $result, $exception]; } #[Workflow\SignalMethod] diff --git a/tests/Acceptance/Harness/ChildWorkflow/ThrowOnExecuteTest.php b/tests/Acceptance/Harness/ChildWorkflow/ThrowOnExecuteTest.php index 6c26c552..c509baba 100644 --- a/tests/Acceptance/Harness/ChildWorkflow/ThrowOnExecuteTest.php +++ b/tests/Acceptance/Harness/ChildWorkflow/ThrowOnExecuteTest.php @@ -19,7 +19,7 @@ class ThrowOnExecuteTest extends TestCase { #[Test] - public static function check(#[Stub('Harness_ChildWorkflow_ThrowsOnExecute')]WorkflowStubInterface $stub): void + public static function throwExceptionOnInit(#[Stub('Harness_ChildWorkflow_ThrowsOnExecute')]WorkflowStubInterface $stub): void { try { $stub->getResult(); @@ -41,16 +41,42 @@ public static function check(#[Stub('Harness_ChildWorkflow_ThrowsOnExecute')]Wor self::assertSame(['foo' => 'bar'], $failure->getDetails()->getValue(0, 'array')); } } + + #[Test] + public static function throwExceptionAfterInit( + #[Stub('Harness_ChildWorkflow_ThrowsOnExecute', args: [true])] + WorkflowStubInterface $stub, + ): void { + try { + $stub->getResult(); + throw new \Exception('Expected exception'); + } catch (WorkflowFailedException $e) { + self::assertSame('Harness_ChildWorkflow_ThrowsOnExecute', $e->getWorkflowType()); + + /** @var ChildWorkflowFailure $previous */ + $previous = $e->getPrevious(); + self::assertInstanceOf(ChildWorkflowFailure::class, $previous); + self::assertSame('Harness_ChildWorkflow_ThrowsOnExecute_ChildThrowOnInit', $previous->getWorkflowType()); + + /** @var ApplicationFailure $failure */ + $failure = $previous->getPrevious(); + self::assertInstanceOf(ApplicationFailure::class, $failure); + self::assertStringContainsString('Test message', $failure->getOriginalMessage()); + self::assertSame('TestError', $failure->getType()); + self::assertTrue($failure->isNonRetryable()); + self::assertSame(['foo' => 'bar'], $failure->getDetails()->getValue(0, 'array')); + } + } } #[WorkflowInterface] class MainWorkflow { #[WorkflowMethod('Harness_ChildWorkflow_ThrowsOnExecute')] - public function run() + public function run(bool $onInit = false) { return yield Workflow::newChildWorkflowStub( - ChildWorkflow::class, + $onInit ? ChildWorkflowThrowOnInit::class : ChildWorkflow::class, )->run(); } } @@ -65,3 +91,14 @@ public function run() throw new ApplicationFailure('Test message', 'TestError', true, EncodedValues::fromValues([['foo' => 'bar']])); } } + + +#[WorkflowInterface] +class ChildWorkflowThrowOnInit +{ + #[WorkflowMethod('Harness_ChildWorkflow_ThrowsOnExecute_ChildThrowOnInit')] + public function run() + { + throw new ApplicationFailure('Test message', 'TestError', true, EncodedValues::fromValues([['foo' => 'bar']])); + } +} diff --git a/tests/Unit/Worker/AutowiringTestCase.php b/tests/Unit/Worker/AutowiringTestCase.php index 83a25865..b5a2ec24 100644 --- a/tests/Unit/Worker/AutowiringTestCase.php +++ b/tests/Unit/Worker/AutowiringTestCase.php @@ -38,12 +38,6 @@ public static function reflectionDataProvider(): array $instance = (new \ReflectionClass(static::class))->newInstanceWithoutConstructor(); return [ - // Closure - 'closure' => [new \ReflectionFunction($instance->instanceMethod(...))], - - // Static Closure - 'static closure' => [new \ReflectionFunction(static fn() => global_function())], - // Instance Method static::class . '->instanceMethod' => [new \ReflectionMethod($instance, 'instanceMethod')], diff --git a/tests/Unit/Workflow/DeferredGeneratorTestCase.php b/tests/Unit/Workflow/DeferredGeneratorTestCase.php index 691c9b37..b76811d7 100644 --- a/tests/Unit/Workflow/DeferredGeneratorTestCase.php +++ b/tests/Unit/Workflow/DeferredGeneratorTestCase.php @@ -81,7 +81,6 @@ public function testCompareReturn(): void [ 'current', 'key', 'current', 'key', 'valid', 'next', - 'getReturn', ], ); } @@ -137,17 +136,6 @@ public function testCompareEmptyThrowValid(): void ); } - public function testCompareEmptyThrowGetReturn(): void - { - $this->compare( - fn() => (function () { - throw new \Exception('foo'); - yield; - })(), - ['getReturn', 'getReturn'], - ); - } - public function testCompareEmptyThrowGetKey(): void { $this->compare( @@ -184,7 +172,6 @@ public function testLazyNotGeneratorWithException(): void $lazy->current(); } - public function testLazyNotGeneratorWithException2(): void { $lazy = DeferredGenerator::fromHandler(fn() => throw new \Exception('foo'), EncodedValues::empty()); @@ -198,6 +185,35 @@ public function testLazyNotGeneratorWithException2(): void $this->assertNull($lazy->current()); } + public function testLazyOnGeneratorHandler(): void + { + $lazy = DeferredGenerator::fromHandler(static function () { + throw new \LogicException('foo'); + yield; + }, EncodedValues::empty()); + + try { + $lazy->current(); + $this->fail('Exception was not thrown'); + } catch (\LogicException) { + // ignore + } + + $this->assertNull($lazy->current()); + } + + public function testGetResultFromNotStartedGenerator(): void + { + $closure = fn() => (function () { + yield 1; + }); + + $handler = DeferredGenerator::fromHandler($closure, EncodedValues::empty()); + + $this->expectException(\LogicException::class); + $handler->getReturn(); + } + /** * @param callable(): \Generator $generatorFactory * @param iterable $actions