Fixed Event listeners running on install

[Jake B.]

Affected version

2.0.0 Beta 2

In our Reactions add-on we have a couple event listeners which appear to be running while the add-on is being installed and was causing some errors (The event listener is templater_global_data and we're manually suppressing this) I thought I had seen a bug report for this previously but it's still happening and I can't find that other bug report anymore so this may be a duplicate of something that exists already

 

[DragonByte Tech]

In Beta 1, they were also running on uninstall, I've not tested it on Beta 2 yet.


Fillip

 

[Jake B.]

I haven't tested uninstall without the errors from this listener being suppressed so I'm not sure, will give it a try later. That might've been the bug report that I'd seen

 

[Chris D]

Could you please also disclose what the errors/symptoms of the issue are? It could be relevant in terms of identifying whether there's a specific cause or a reproduction case.

 

[Jake B.]

Yeah, these are the current contents of that listener:

public static function templaterGlobalData(\XF\App $app, array &$data) {
    try {
        $data['reactions'] = $app->container('reactions');
        $data['reactionTypes'] = $app->container('reactionTypes');
    } catch (\Exception $e) {}
}

The try/catch was added to suppress the error on install, which is:

An exception occurred: [InvalidArgumentException] Container key 'reactions' was not found in /Users/jakebooher/Sites/xf2/src/XF/Container.php on line 43

 

[Mike]

Can you still reproduce this on beta 3? There are some potentially relevant changes I believe.

 

[Jake B.]

Yep, still getting the same error:

An exception occurred: [InvalidArgumentException] Container key 'reactions' was not found in /Users/jakebooher/Sites/xf2/src/XF/Container.php on line 43

XF\Container->offsetGet() in /Users/jakebooher/Sites/xf2/src/XF/App.php at line 2614
XF\App->container() in /Users/jakebooher/Sites/xf2/src/addons/ThemeHouse/Reactions/Listener/CodeEvent.php at line 22
ThemeHouse\Reactions\Listener\CodeEvent::templaterGlobalData()
call_user_func_array() in /Users/jakebooher/Sites/xf2/src/XF/Extension.php at line 67
XF\Extension->fire() in /Users/jakebooher/Sites/xf2/src/XF/App.php at line 2248
XF\App->fire() in /Users/jakebooher/Sites/xf2/src/XF/App.php at line 1558
XF\App->getGlobalTemplateData() in /Users/jakebooher/Sites/xf2/src/XF/App.php at line 1476
XF\App->preRender() in /Users/jakebooher/Sites/xf2/src/XF/Admin/App.php at line 97
XF\Admin\App->preRender() in /Users/jakebooher/Sites/xf2/src/XF/Mvc/Dispatcher.php at line 280
XF\Mvc\Dispatcher->render() in /Users/jakebooher/Sites/xf2/src/XF/Mvc/Dispatcher.php at line 44
XF\Mvc\Dispatcher->run() in /Users/jakebooher/Sites/xf2/src/XF/App.php at line 1787
XF\App->run() in /Users/jakebooher/Sites/xf2/src/XF.php at line 326
XF::runApp() in /Users/jakebooher/Sites/xf2/admin.php at line 13

 

[Mike]

Can you include the request state data as well (assuming it's in the error log)?

 

[Jake B.]

array(4) {
  ["url"] => string(24) "/admin.php?tools/run-job"
  ["referrer"] => string(38) "http://xf2.dev/admin.php?tools/run-job"
  ["_GET"] => array(1) {
    ["tools/run-job"] => string(0) ""
  }
  ["_POST"] => array(3) {
    ["_xfRedirect"] => string(120) "http://xf2.dev/admin.php?add-ons/ThemeHouse-Reactions/finalize&t=1505914142%2C629cb76849d12e343ce90a9b3648656e&a=install"
    ["_xfToken"] => string(8) "********"
    ["only_ids"] => string(1) "9"
  }
}

 

[Mike]

I've made a change here for beta 5 that will resolve this, though it's a pretty difficult issue to generically fix. It has to do with us disabling listeners/extensions for an add-on while it's being installed/upgraded. Previously, upon this entering or leaving this state, we immediately reloaded the listeners/extensions to ensure they either didn't run or started running. The problem is that listeners will often (reasonably) assume that if they run, a previous event will have run as well, which this violates. Therefore, in most cases, we aren't triggering these changes immediately and just letting them apply to the next request. In most cases, I believe this should be fine.

The one case where it isn't is when doing add-on CLI actions as, unlike the web interaction, we aren't doing a bunch of PHP "refreshes" so we don't get the expected behavior. Equally, the CLI won't trigger many of the "basic" events that are likely to trigger this (templater related stuff, for example). The full fix for the CLI would really require a model where we start forking child PHP processes so that the internal PHP state gets reloaded as necessary. That's quite a significant change to say the least, so we'll cross that bridge if needed in the future.