Should not use set_exception_handler

jarbey commented 2 years ago

https://github.com/denpamusic/php-bitcoinrpc/blob/172c184144cb219c29d692875a9a17791ba6d36c/src/functions.php#L121

Using set_exception_handler can override existings handler and can be a big problem for application that already use it. For it has shutdown the NewRelic error reporting !

I suggest either to not use it or, to use it right before a call and call restore_exception_handler after the call (a successfull call or an error one).

I will have to find a way to change that. If i can will propose you a pullrequest

denpamusic commented 2 years ago

Thank you for the feedback!

I'll see what I can do to change exception handling on the weekends. PR would be awesome too!

Sorry for inconvenience that it caused.

jarbey commented 2 years ago

Don't be worry for the inconvenience ! I should thank you for the job you have done and to share it with everyone !

I was asking myself "Why an handler is needed ?"

As for sync call you already have a try / catch : https://github.com/denpamusic/php-bitcoinrpc/blob/172c184144cb219c29d692875a9a17791ba6d36c/src/Client.php#L152-L164
For Async call you manage error on the promise : https://github.com/denpamusic/php-bitcoinrpc/blob/172c184144cb219c29d692875a9a17791ba6d36c/src/Client.php#L191-L195

Maybe it's not usefull to use a global PHP Exception handler, and we can locally trap exception, if there is any missing ?

jarbey commented 2 years ago

Any update on that please ?

denpamusic / php-bitcoinrpc

Should not use set_exception_handler #66