search

systopia / de.systopia.moregreetings

CiviCRM Extension for additional greetings
GNU Affero General Public License v3.0
4 stars 10 forks source link

Avoid calling original error handler with NULL reference #36

Closed vbrosch closed 1 year ago

vbrosch commented 1 year ago

Hi,

first of all, thanks for your helpful extension. We needed to make two small adjustments to get it running smoothly with PHP 8.1. I see that you already made $errContext an optional parameter after the last release.

Apart from that, in our development environment (based on the official wordpress docker image and caddy) set_error_handler returns NULL which is the reason why calling call_user_func throws an exception.

I just wanted to provide our small fix for this issue. I don't know if it has any value for you...

We are using PHP 8.1, WordPress 6.1.1 and CiviCRM 5.59 btw.

Greetings Valentin

vbrosch commented 1 year ago

@jensschuppe Thanks for your suggested fix, I agree that is_callable is more appropriate :-)

In our installation packages/Smarty/Smarty.class.php still contains trigger_error (line 1100), so I think that using the error handler is still required? I tried to remove it, but there is no exception thrown that could be catched.

jensschuppe commented 1 year ago

Thinking more about it, I think we should have a Core version guard for whether we set our own error handler or not, as with https://github.com/civicrm/civicrm-core/pull/25334 there should be no need for intercepting anymore and our error handler might not get reset. This needs more testing ...

jensschuppe commented 1 year ago

I included the change proposed here in #40, see 645383e0aeddcb95bb811f6be8b4999e83422da2. A core version switch would still be more robust.