sdk-php icon indicating copy to clipboard operation
sdk-php copied to clipboard

react/promise v3 doesn't work fine

Open roxblnfk opened this issue 2 years ago • 8 comments

In order to ensure asynchronous operation of user workflows, coroutines are implemented in the SDK, which use promises. It is a standard situation when we reject them with internal exceptions, which in some branches may not be handled. However, in the paradigm of react/promise v3, this is considered undesirable practice. Therefore, when a rejected promise is completed, in which there is no exception handler, the error_log() function is called with the error text.

The side effects of such behavior are unpredictable, and it is currently impossible to disable it.

The issue about unhandled exceptions may be fixed once react/promise has the ability to set a permanent handler, not a one-time one.


Thanks to the tests in roadrunner-temporal, other deviations from the normal operation of the SDK were detected. These tests have been partially transferred to the SDK (see #359).

Therefore, I suggest temporarily abandoning react/promise v3 since version 2.6.1 until a solution is found.

roxblnfk avatar Oct 03 '23 20:10 roxblnfk

Is there an update on this? I noticed that I had to downgrade react/promise on my project in order to install this SDK. And now I see all kind of Implicitly marking parameter $x as nullable is deprecated errors on PHP 8.4 that are fixed in v3 with https://github.com/reactphp/promise/pull/260.

ruudk avatar May 18 '25 16:05 ruudk

React v3 still has several problems that prevent us from using it.

roxblnfk avatar May 18 '25 21:05 roxblnfk

I read the issue https://github.com/temporalio/sdk-php/pull/359 but saw some follow up PR's, one that introduced a global rejection handler: https://github.com/reactphp/promise?tab=readme-ov-file#set_rejection_handler

Isn't this what was missing?

ruudk avatar May 19 '25 06:05 ruudk

It's still one-time handler

roxblnfk avatar May 19 '25 07:05 roxblnfk

Should there be a new issue to track this? I'm a bit surprised that the Temporal SDK has deprecations on PHP 8.4, which was released 6 months ago.

ruudk avatar May 19 '25 10:05 ruudk

Regarding deprecations in the SDK, definitely create an issue 👍 . However, deprecations in dependencies should be tracked by the dependencies themselves.

roxblnfk avatar May 19 '25 12:05 roxblnfk

Sure, but in this case, this SDK prevents installation of the latest major, so I think it would have been good if this project created that issue upstream. How else is it going to be fixed?

ruudk avatar May 19 '25 13:05 ruudk

Yes, it would be great to push the development of React/Promise. However, we still won't achieve the set goals, including those related to the @yield annotation.

Therefore, the most promising direction is to create a fork and maintain our own version of promises compatible with React. However, this is not a priority right now.

roxblnfk avatar May 19 '25 13:05 roxblnfk