Fixed resolveExtendedClassToRoot can fail badly

[Xon]

Affected version

2.2.13

Getting flooded with these errors after updating to 2.2.13 (even after disabling the addon):

Error: Class 'SV\UserMentionsImprovements\XF\Service\Post\XFCP_Notifier' not found src/addons/SV/UserMentionsImprovements/XF/Service/Post/Notifier.php:8

#0 src/vendor/composer/ClassLoader.php(571): include()
#1 src/vendor/composer/ClassLoader.php(428): Composer\Autoload\includeFile('/home/***/publi...')
#2 [internal function]: Composer\Autoload\ClassLoader->loadClass('SV\\UserMentions...')
#3 [internal function]: spl_autoload_call('SV\\UserMentions...')
#4 src/XF/Job/Notifier.php(22): class_exists('SV\\UserMentions...')
#5 src/XF/Job/Manager.php(260): XF\Job\Notifier->run(8)
#6 src/XF/Job/Manager.php(202): XF\Job\Manager->runJobInternal(Array, 8)
#7 src/XF/Job/Manager.php(86): XF\Job\Manager->runJobEntry(Array, 8)
#8 job.php(43): XF\Job\Manager->runQueue(false, 8)
#9 {main}

Edit: Even getting these after uninstalling the addon. Finally resolved after deleting all files related to the addon.

XF\Job\Notifier and XF\Service\AbstractNotifier\getJobData requires that resolveExtendedClassToRoot reliably resolves to the root.

Somehow resolveExtendedClassToRoot was returning the calling class when the mapping didn't exist, and instead of hard failing caused the above job to relentlessly spam the logs with error messages.

 

[Xon]

\XF\Service\Post\Notifier extends XF\Service\AbstractNotifier, and then when a post notification job is queued, XF\Service\AbstractNotifier::getJobData calls resolveExtendedClassToRoot($this).

resolveExtendedClassToRoot($this) should be returning \XF\Service\Post\Notifier.

Essentially, SV\UserMentionsImprovements\XF\Service\Post\Notifier absolutely should not be being stuffed into the job to fail horribly like this.

 

[Xon]

This is definitely the fault of the add-on;

[AL] Core Package

The package contains libraries we share between several our add-ons. Until now the files were included in packages of the add-ons themselves, however, having multiple add-ons to using the same files can sometimes cause a conflict between them, if...

xenforo.com

I think instead of a conditional throwing on if debugMode enable, it should always throw.

 

[PaulB]

I think instead of a conditional throwing on if debugMode enable, it should always throw.

Chances are this is going to expose a number of add-on bugs that previously went unnoticed, and most will be inconsequential, so it makes sense to give people some time to sort those out. I'm definitely in favor of making it throw even in production mode eventually, but that'd be rough on point release.

 

[Xon]

Chances are this is going to expose a number of add-on bugs that previously went unnoticed, and most will be inconsequential, so it makes sense to give people some time to sort those out. I'm definitely in favor of making it throw even in production mode eventually, but that'd be rough on point release.

Even just log unconditionally would be helpful for exposing these bugs

 

[PaulB]

It would be, but I’m a little worried it could result in infinite recursion.

 

[Xon]

Related, it would be nice if XF\Job\Notifier was a little more robust when loading the class to be used. As at the moment purging the broken job requires multiple steps

 

[XF Bug Bot]

Thank you for reporting this issue, it has now been resolved. We are aiming to include any changes that have been made in a future XF release (2.2.14).

Change log:

Improve error handling when loading notifier services

There may be a delay before changes are rolled out to the XenForo Community.

 

[Jeremy P]

\XF\Job\Notifier will now handle this situation more gracefully, logging the exception but otherwise marking the job as completed. I've also made changes in 2.3 to throw from \XF\Extension::resolveExtendedClassToRoot unconditionally as hopefully add-ons will have sorted out any issues by then.