Exceptions thrown in calls to Messenger::callRpc() from Rpc::handle() are not handled
Hi
I discovered that the call to callRpc in \WyriHaximus\React\ChildProcess\Messenger\Messages\Rpc::handle doesn't handle exceptions thrown in user code ( closures registerd with registerRpc )
That's because Promise:done() of a FulfilledPromise should not never throw an exception ( the $onFulfilled callable in done() method of a FulfilledPromise is immediately called )
$this->callRpc($target, $payload)->done(
function (array $payload) use ($uniqid,$target) {
$this->write($this->createLine(Factory::rpcSuccess($uniqid, $payload)));
},
function ($error) use ($uniqid) {
$this->write($this->createLine(Factory::rpcError($uniqid, $error)));
},
function ($payload) use ($uniqid) {
$this->write($this->createLine(Factory::rpcNotify($uniqid, $payload)));
}
);
A possible fix would be
try {
$this->callRpc($target, $payload)->then(
function (array $payload) use ($uniqid,$target) {
$this->write($this->createLine(Factory::rpcSuccess($uniqid, $payload)));
},
function ($error) use ($uniqid) {
$this->write($this->createLine(Factory::rpcError($uniqid, $error)));
},
function ($payload) use ($uniqid) {
$this->write($this->createLine(Factory::rpcNotify($uniqid, $payload)));
}
);
}
catch (\Throwable $throwable) {
$this->write($this->createLine(Factory::rpcError($uniqid, $throwable)));
}
To reproduce a similar error for example code in the ChildInterface an rpc with a closure throwing some \Error , for example
Argument 1 passed to WyriHaximus\React\ChildProcess\Messenger\Messages\Rpc::WyriHaximus\React\ChildProcess\Messenger\Messages{closure}() must be of the type array, string given, called in /usr/src/myapp/vendor/react/promise/src/FulfilledPromise.php on line 39
$messenger->registerRpc('example', function (Payload $payload) {
return \React\Promise\resolve('i am not an array');
});
these kind of errors will be totally skipped and parent process will not notify any problem
Hey @MatteoOreficeIT sorry for the late reply, been at PHPDay in Italy the past weekend and still recovering. Looks like you found a serious bug! Could you write up a PR with tests ensuring this doesn't happen again?
About the possible fix, it might be simpler to do ensuring we catch the errors in the then as well:
$this->callRpc($target, $payload)->done(
function (array $payload) use ($uniqid,$target) {
$this->write($this->createLine(Factory::rpcSuccess($uniqid, $payload)));
},
null,
function ($payload) use ($uniqid) {
$this->write($this->createLine(Factory::rpcNotify($uniqid, $payload)));
}
)->done(
null,
function ($error) use ($uniqid) {
$this->write($this->createLine(Factory::rpcError($uniqid, $error)));
}
);
Hi @WyriHaximus , sure I'll elaborate in the next days
Thanks
@MatteoOreficeIT :+1: