Fatal error when I upgrade a 4.7 dev site with plugins

jpdupont · 2017-12-05 22:46:36

I just upgrade a site 4.7 dev (24 oct 2017) to the last version from 5 dec 2017.

Error on first login to the admin :

Fatal error: Uncaught Error: Unsupported operand types in /home/rouvroy/webapps/commune/textpattern/lib/txplib_misc.php(1802) : eval()'d code:93 Stack trace: #0 /home/rouvroy/webapps/commune/textpattern/lib/txplib_misc.php(1802) : eval()'d code(48): adi_recent_tab_init() #1 /home/rouvroy/webapps/commune/textpattern/lib/txplib_misc.php(1802): eval() #2 /home/rouvroy/webapps/commune/textpattern/index.php(209): load_plugins(1) #3 {main} thrown in /home/rouvroy/webapps/commune/textpattern/lib/txplib_misc.php(1802) : eval()'d code on line 93

If I go to the plugins screen, and try to delete or click on the activated “Yes” link, I get this errors :

Fatal error: Uncaught Error: Unsupported operand types in /home/rouvroy/webapps/commune/textpattern/lib/txplib_misc.php(1374) : eval()'d code:93 Stack trace: #0 /home/rouvroy/webapps/commune/textpattern/lib/txplib_misc.php(1374) : eval()'d code(48): adi_recent_tab_init() #1 /home/rouvroy/webapps/commune/textpattern/lib/txplib_misc.php(1374): eval() #2 /home/rouvroy/webapps/commune/textpattern/vendors/Textpattern/Plugin/Plugin.php(211): load_plugin('adi_recent_tab', true) #3 /home/rouvroy/webapps/commune/textpattern/include/txp_plugin.php(269): Textpattern\Plugin\Plugin->changeStatus('adi_recent_tab', 0) #4 /home/rouvroy/webapps/commune/textpattern/include/txp_plugin.php(50): switch_status() #5 /home/rouvroy/webapps/commune/textpattern/index.php(227): include('/home/rouvroy/w...') #6 {main} thrown in /home/rouvroy/webapps/commune/textpattern/lib/txplib_misc.php(1374) : eval()'d code on line 93

Or I get this error message on a white screen :

Fatal error: Uncaught Error: Unsupported operand types in /home/rouvroy/webapps/commune/textpattern/lib/txplib_misc.php(1374) : eval()'d code:93 Stack trace: #0 /home/rouvroy/webapps/commune/textpattern/lib/txplib_misc.php(1374) : eval()'d code(48): adi_recent_tab_init() #1 /home/rouvroy/webapps/commune/textpattern/lib/txplib_misc.php(1374): eval() #2 /home/rouvroy/webapps/commune/textpattern/vendors/Textpattern/Plugin/Plugin.php(183): load_plugin('adi_recent_tab', true) #3 /home/rouvroy/webapps/commune/textpattern/include/txp_plugin.php(514): Textpattern\Plugin\Plugin->delete('adi_recent_tab') #4 /home/rouvroy/webapps/commune/textpattern/include/txp_plugin.php(50): plugin_multi_edit() #5 /home/rouvroy/webapps/commune/textpattern/index.php(227): include('/home/rouvroy/w...') #6 {main} thrown in /home/rouvroy/webapps/commune/textpattern/lib/txplib_misc.php(1374) : eval()'d code on line 93

Bloke · 2017-12-05 23:22:07

I axed our global variable $textarray last week so any plugins that use it directly are likely to fail.

Not entirely sure what to do about this. We could reinstate $textarray as an empty global array for the time being, so at least errors like this will go away, but then any plugin that uses it (as adi does) won’t get the desired results, since there’ll be no strings for it to use and core won’t recognise them, so it’s kind of masking the issue.

Any plugins (re)installed now that contain Textpacks will automatically have their strings installed and, crucially, retained at install time, ready for when another language is installed/updated. So the hoops that adi in particular jumps through to ensure that his strings are available to the system were a by-product of the core’s poor attitude towards strings in the past.

The only unknown right now is what happens to plugins that are run from file. No installation = no textpack storage in the DB = no language hooks when langs are installed.

For now, if you comment out or remove the line near the top of any of adi’s plugins that looks like this:

$textarray += $adi_recent_tab_gtxt;

then it should at least get you past the error message.

jpdupont · 2017-12-06 08:39:42

Thanks Stef, the action corrects the errors.
So upgrading to 4.7 will cause these fatal errors if some plugins are installed?
Should not we choose to make this error silent, and give a warning message in diagnostics like “the xxx plugin needs to be upgraded”? Or mark the plugins in error in the list of plugins?

Last edited by jpdupont (2017-12-06 08:42:32)

Bloke · 2017-12-06 11:00:54

jpdupont wrote #308114:

So upgrading to 4.7 will cause these fatal errors if some plugins are installed?

Only ones that do something with some feature we’ve removed. Like use $textarray in the manner in which adi did… which is, umm, most of his admin-side plugins. In general, relying on a global variable being present without testing its existence first is poor coding practice, but the core does it all over the place so it’s not adi’s fault for doing the same. He was working around our shoddy working practices to fix the problem with core incorrectly using a global variable in the first place!

Should not we choose to make this error silent, and give a warning message… Or mark the plugins in error in the list of plugins?

That’d be wonderful. Unfortunately, when Textpattern loads plugins – on every page refresh – it does so by “running” the plugin. Any syntax, logic or incompatibility errors get thrown at this point by our error handler. We could suppress warnings (we do in Live mode) but then nobody would be able to find out why a plugin is broken.

Unfortunately, the problem in this case is that PHP is checking the two variables ($textarray and adi’s own language strings array) and determining that the two types of data are incompatible and throwing a fatal error. That’s a show-stopper as far as PHP is concerned. The result of this error will largely depend on the version of PHP that’s installed. Earlier versions might let this kind of thing slip by, but PHP 7+ for example is a lot more strict.

One thing we might be able to do – and I’ll need advice on this from Oleg / makss – is if we actually try ... catch in the plugin error handler and if any fatal errors are thrown when the plugin is attempted to load, we skip it. Not loading it is better than breaking your admin side. Not sure what would happen about public plugins though, as the tags would then not be available and might break your site.

But doing something like this might mean, on the High Diagnostics page, we could perhaps load all the plugins just to test which ones run fine and show alongside each an indicator if any are completely broken. Leave it with us.

jpdupont · 2017-12-06 11:17:08

I just anticipated the bug on a second site to update, commenting on the line in question, but in a future update (beta or 4.7 final) by all, this problem will arise, unless Adi and the others adapt their codes beforehand.

On the other hand, what code should be used in these plugins to still load textpacks?

Bloke · 2017-12-06 11:46:45

Adi’s plugins – as far as I recall – all bundle Textpacks inside the plugin in a different manner to the way core handles them. That’s the crux of the issue. When you install the plugin, he uses the internal strings to give you a working UI and then encourages you to install the strings “properly” from his site via the plugin Options.

That works (worked) well when our Textpack string support in core wasn’t very mature. In fact, it was a brilliant solution.

Now (from 4.7.0) core support has evolved somewhat. So in future, Adi won’t – at least, shouldn’t – need to do the self-install Textpack dance, because core will load all known (at the time) Textpacks into the database and keep them there, just in case. It’ll auto-install them for the current language and if you install any other Txp language in future that has a pack in the plugin, the strings will be auto-installed then.

Alternatively, you can manually install strings from the Admin->Languages panel if there isn’t a pack available, and send the translations to the plugin author to bundle in the next plugin release.

I intend to beef up the Languages panel in a future release, either in core or (more likely) via a string manager plugin to help translators and admins manage language strings and patch gaps. Would be nice if it synced with Crowdin… but that’s for another day.

EDIT: and to actually answer your question :-) … the plugin needs to be bundled with the Textpack embedded in the plugin data’s textpack block.

Last edited by Bloke (2017-12-06 11:48:10)

etc · 2017-12-06 11:49:24

I guess we’d better leave global $textarray there for one more dev cycle, the time to test possible solutions.

Testing public plugins in Diagnostic can be problematic since the context is not the same. A try...catch in Live mode seems ok to me, we just need a way to log the warning and send it to admins.

Bloke · 2017-12-06 12:01:11

etc wrote #308123:

I guess we’d better leave global $textarray there for one more dev cycle

Yeah, we can do that no problem and just not use it. Some plugins might be “surprised” to find it empty. Or do you mean populate it even though core doesn’t use it?

Testing public plugins in Diagnostic can be problematic since the context is not the same.

Ah, true. Some will only load on certain panels. That means it won’t be possible to do this.

A try...catch in Live mode seems ok to me, we just need a way to log the warning and send it to admins.

Yes, hmmm. Not sure. Any cool ideas come to mind?

etc · 2017-12-06 12:13:09

Hmm, is it worth populating now? Not sure, just an empty array to avoid crashes imo.

Bloke · 2017-12-06 14:39:07

Done.

gomedia · 2017-12-08 02:54:09

I thought my ears were burning!

etc wrote #308123:

I guess we’d better leave global $textarray there for one more dev cycle, the time to test possible solutions.

Thank you throwing me a lifebelt.

Bloke wrote #308121:

Adi’s plugins – as far as I recall – all bundle Textpacks inside the plugin in a different manner to the way core handles them. That’s the crux of the issue. When you install the plugin, he uses the internal strings to give you a working UI and then encourages you to install the strings “properly” from his site via the plugin Options.

The pertinent bit being “he uses the internal strings to give you a working UI” – and most importantly both for cached and “installed” plugins.

Now (from 4.7.0) core support has evolved somewhat. So in future, Adi won’t – at least, shouldn’t – need to do the self-install Textpack dance, because core will load all known (at the time) Textpacks into the database and keep them there, just in case.

Could you clarify this bit please? Does it mean that text strings in cached plugins will now be loaded automatically?

Bloke · 2017-12-08 10:56:33

gomedia wrote #308180:

The pertinent bit being “he uses the internal strings to give you a working UI” – and most importantly both for cached and “installed” plugins.

Yes, that’s something we need to test more. Installed plugins might need a bit of extra thought here, but if they contain the Textpack translation strings in the header area of the plugin, we should be able to extract those and inject them into the local language table on an as-needed basis when the plugin runs.

Does it mean that text strings in cached plugins will now be loaded automatically?

Here’s what happens now:

Plugin author writes a plugin and bundles all known Textpacks into the plugin at that time, in $plugin['textpack']. Each lang block must be prefixed by a valid ‘@language’ code — all of which have changed in this release to “super-short” codes where possible, e.g. ‘da-dk’ => ‘da’. But variants still retain their full designation (e.g. ‘en-gb’ is distinct from ‘en-us’).
People install the plugin into the database.
The entire Textpack array – for all languages that the plugin has packs for – is stored internally in the txp_plugin table (a new column). For safekeeping.
Any language that is currently installed in Txp – for front of house, or for admin side – will be automatically updated at plugin install time. So strings matching those languages from the plugin Textpacks are installed right out of the gate automatically.
If at any point in future an admin installs a new language or updates an existing language, all installed plugins are checked at that point. If there are any strings that were bundled with the plugin in that language, they will be automatically installed too.
If you install a language that does not have a representation inside the plugin, its strings obviously can’t be brought into the database. BUT we now have the notion of fallback strings so if the language isn’t installed, the strings from the default language (as defined in config.php – or en if not specified) will be used instead for that language. So if you installed Norwegian, any strings that weren’t defined would be in English. This may apply to plugins too, but I’m not sure yet if that bit’s taken care of. Limited test data. It’s certainly the plan.

I want the above to apply to ‘installed’ plugin cache files too, so when it loads the plugin it’ll see the lang strings in $plugin['textpack'], but instead of ‘installing’ them in the database it just adds the ones from the current language to the Lang object on-the-fly, so the system knows about them and can therefore render the UI properly. It might do that already, I haven’t checked recently.

Howzat?

Textpattern CMS

Textpattern CMS support forum

#1 2017-12-05 22:46:36