[Bug] Generators return null when exception is thrown

[IhorBerestPaybis]

What are you really trying to do?

I want to structure workflows in an easy-to-read and understandable manner, especially when the workflow's code is long enough. At my company, we have found that it is easier when we move the part of code that is related to the same action, or part of the flow (activity calls, timers, if statements) into a private function inside the workflow code. Thus we create private methods inside workflow classes that return Generators (simply because of the yield inside that method)

Describe the bug

When an exception is thrown in the generator it returns null instead of throwing that exception in the outer scope, which results in proceeding workflow execution instead of immediate cancel of workflow. See comparison below

Minimal Reproduction

Workflow code

<?php

declare(strict_types=1);

namespace App\Workflow\Test;

use Carbon\CarbonInterval;
use Temporal\Activity\ActivityOptions;
use Temporal\Workflow;
use Temporal\Workflow\WorkflowInterface;
use Temporal\Workflow\WorkflowMethod;

#[WorkflowInterface]
class TestWorkflow
{

    private Workflow\ActivityStubInterface $activity;

    public function __construct()
    {
        $defaultActivityOptions = ActivityOptions::new()
            ->withStartToCloseTimeout(CarbonInterval::seconds(45))
            ->withTaskQueue('default')
        ;

        $this->activity = Workflow::newUntypedActivityStub($defaultActivityOptions);
    }

    #[WorkflowMethod]
    public function execute(): \Generator
    {
        var_dump(yield $this->doSomething());
        echo "Execution is processed\n";
    }

    private function doSomething(): \Generator
    {
        yield $this->activity->execute('execute');
    }
}

Activity code

<?php

declare(strict_types=1);

namespace App\Workflow\Test;

use Temporal\Activity\ActivityInterface;
use Temporal\Activity\ActivityMethod;

#[ActivityInterface]
class TestActivity
{
    #[ActivityMethod]
    public function execute(): void
    {
        throw new \Exception("error");
    }
}

By running the example above we get the workflow in completed status, even though the activity has been canceled. And the doSomething method returns null

If we modify the workflow and move the activity execution from the private method into workflow method it works as expected and workflow finishes in canceled status

<?php

declare(strict_types=1);

namespace App\Workflow\Test;

use Carbon\CarbonInterval;
use Temporal\Activity\ActivityOptions;
use Temporal\Workflow;
use Temporal\Workflow\WorkflowInterface;
use Temporal\Workflow\WorkflowMethod;

#[WorkflowInterface]
class TestWorkflow
{

    private Workflow\ActivityStubInterface $activity;

    public function __construct()
    {
        $defaultActivityOptions = ActivityOptions::new()
            ->withStartToCloseTimeout(CarbonInterval::seconds(45))
            ->withTaskQueue('default')
        ;

        $this->activity = Workflow::newUntypedActivityStub($defaultActivityOptions);
    }

    #[WorkflowMethod]
    public function execute(): \Generator
    {
       yield $this->activity->execute('execute');
        echo "Execution is processed\n";
    }
}

Environment/Versions

• OS and processor: Linux, Mac
• Temporal Version: minimal SDK version the issues has been found is 2.11.0, before works as expected
• Using docker

Additional context

No additional context

[roxblnfk]

Hello. Thank you for creating this issue.

Let me point out that you can achieve the desired behavior using the yield from construct:

yield from $this->doSomething()

However, you won't be able to use this to obtain a result from the generator.

I also believe that we should receive an exception in the execution flow if it occurs in the yielded generator. I will discuss with the team whether the current behavior is intended.

[IhorBerestPaybis]

Hello, thank you for the tip.

As you said, we aren't able to obtain a result, and that's our problem, I am afraid.
However, we have found another solution.

We simply wrapped the generator code in try/catch clause and returned the Promise::reject object when catching a Throwable.

private function doSomething(): \Generator
{
    try {
        yield $this->activity->execute('execute');
    } catch (\Throwable $exception) {
        return Promise::reject($exception);
    }
}

But wrapping every generator method in try/catch clause isn't the best-looking long-term solution.