Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updating plugins throws errors #120

Closed
yellowled opened this issue Apr 17, 2014 · 36 comments
Closed

Updating plugins throws errors #120

yellowled opened this issue Apr 17, 2014 · 36 comments

Comments

@yellowled
Copy link
Member

Fatal error: Uncaught exception 'ErrorException' with message 'Assigning the return value of new by reference is deprecated' in /path/to//include/compat.inc.php:126

Full error messages

@yellowled yellowled added this to the RC milestone Apr 17, 2014
@mattsches
Copy link
Member

Would be interesting to see which plugins are loaded when the error occurs. Unfortunately, the name of the plugins is cut off in the error messages.

@onli
Copy link
Member

onli commented Apr 17, 2014

If you could improve the error messages in general, I'd be thankful. I tried that once, but I don't get what s9y is doing in that part.

@ddeimeke
Copy link

Maybe this helps:

https://gist.github.com/ddeimeke/11002987

@garvinhicking
Copy link
Member

@ddeimeke Could you edit include/compat.inc.php and replace:

debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 8);
and
debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS);

with only:

debug_backtrace();

?

This should reveal a full backtrace. You might need to check your output if it contains sensitive passwords and remove them. What would be required for us are the loadPlugin(...) full arguments, because currently as @onli mentioned it is stripped out.

@ddeimeke
Copy link

Did that: https://gist.github.com/ddeimeke/11277312

I do not see anymore.

@garvinhicking
Copy link
Member

@ddeimeke Hhm ok that's strange. Maybe you can edit your serendipity_config_local.inc.php and set

$serendipity['production'] = 'debug';

which should enable the more verbose debug error mode, I hope...

@ddeimeke
Copy link

I have a new "Smarty Debug Console", please find the output here:

https://gist.github.com/ddeimeke/11281622

@ddeimeke
Copy link

And the "FULL DEBUG ERROR MODE"

https://gist.github.com/ddeimeke/11281669

@garvinhicking
Copy link
Member

The smarty output sadly doesn'T help, and I'm stumped as to why the full debug error mode doesn'T show the extra arguments. You did removfe that DEBUG_BACKTRACE_IGNORE_ARGS right?

@ophian do you maybe have a clue?! :-(

@ddeimeke
Copy link

Yes, correct.

@garvinhicking
Copy link
Member

Hm, maybe we can get to it another way. Please edit your include/plugin_api.inc.php file and find the function load_plugin. There you can see several commented $serendipity['debug'] vars. Remove the // in front of all these.

Then in your iinclude/compat.inc.php file, just after:

        echo ' == FULL DEBUG ERROR MODE == ';

you could please put a:

        echo '<br />PLUGIN API:<pre>' . print_r($serendipity['debug'], true) . '</pre><br />';

and hopefully this will give us a better view on at which plugin it seems to fail.

@ddeimeke
Copy link

https://gist.github.com/ddeimeke/bf441d3573a3a4a91a12

"No valid path/filename found. Aborting." sounds interesting.

@garvinhicking
Copy link
Member

I think you patched the probePlugin() method and not load_plugin?! You might want to enable every instance of the $serendipity['debug']['pluginload'] commented variables...

@ddeimeke
Copy link

I missed include/plugin_api.inc.php completely (sorry!)

https://gist.github.com/ddeimeke/11283111

@ophian
Copy link
Member

ophian commented Apr 25, 2014

The smarty output sadly doesn'T help, and I'm stumped as to why the full debug error mode doesn'T show the extra arguments. You did removfe that DEBUG_BACKTRACE_IGNORE_ARGS right?

@ophian do you maybe have a clue?! :-(

I know this is very complicated to remove the PHP internal argument truncation, that it works everywhere. That is why I didn't try to. The cases where you need it are extremly rare, but this one seems to be one of it.
I suggest digging into plugin_api (debug outputs) would be more efficent to find the real issue for the reference error.

Else Dirk could try to enable this
https://github.com/s9y/Serendipity/blob/2.0/include/compat.inc.php#L108
or add

$e = new Exception();
var_dump($e->getTrace());

which should hold full arguments.

@ddeimeke
Copy link

@garvinhicking
Copy link
Member

The last plugin before the error happens seems to be serendipity_plugin_amazon - do you use that plugin and is it the most recent version (maybe manual upload as long as spartacus doesn't work)?

Also, you might want to try moving serendipity_plugin_amazon out of plugins/ (it may not be contained within plugins/ so do not move to a subdirectory of that one, but a parent directory!) and see if that changes anything?

Please check your gist file thoroughly for possible passwords, since the debug output tends to hold these!

@ophian
Copy link
Member

ophian commented Apr 25, 2014

Its not amazon only ...

Array
(
[0] => 8192
[1] => Assigning the return value of new by reference is deprecated
[2] => /srv/www/deimeke.net/dirk/blog/plugins/serendipity_plugin_amazon/serendipity_plugin_amazon.php
[3] => 171
[4] => Array

but also

Array
(
[0] => 8192
[1] => Assigning the return value of new by reference is deprecated
[2] => /srv/www/deimeke.net/dirk/blog/plugins/serendipity_event_aggregator/serendipity_event_aggregator.php
[3] => 767
[4] => Array

@ddeimeke
Please remove you db credentials!!! (This is one of the reasons we never use full args.)

@ddeimeke
Copy link

Gists deleted.

Plugins deleted.

DB password changed. Personal Password changed.

I still get the same error.

https://gist.github.com/ddeimeke/b5518a88981bb4ce88bd
https://gist.github.com/ddeimeke/649f56ff4263dc2fae56

@garvinhicking
Copy link
Member

Man, that is really annoying to debug.

Could you try to move all plugins out of /plugins that are not part of the core distribution and see if that makes a difference? (Moving them will not remote them completely, once you move them back, their config would be restored).

Apparently, one of the plugins seems to contain a "$var = &new XXX" code, but I can't see the actual plugin which does that. It could be an outdated plugin in your installation, or a very rarely used one in the spartacus repo that has not been patched yet. We'd need to find the actual culprit.

I don't currently have an environment to test spartacus with. @ophian or @yellowled do you have an install where you could i.e. try to patch one plugin to report as an outdated/older version and see if you also get those PHP fatal errors when using spartacus?

@ddeimeke
Copy link

http://board.s9y.org/viewtopic.php?f=10&t=16149 - Point 1 ...

My blog is nine years old and I think, I tried nearly every plugin in this time.

I will try to find out the plugins that are in the core distribution.

@yellowled
Copy link
Member Author

I would need more detailed info as to what to debug where and how.

I edit the version number of any plugin installed using Spartacus to a lower version that the one on Spartacus and update it?

@ddeimeke
Copy link

So, I removed all non-used plugins and it works now.!

Question: If a plugin is not used, why is it checked?

Question 2: Once I delete a plugin, why is the directory not deleted?

Question 3: How do I cleanup the database and remove all configuration not used anymore?

@garvinhicking
Copy link
Member

1: Even if a plugin is not used, it needs to be parsed so that it can be offered to be installed, and also maintained/updated.

2: Serendipity never delets a plugin, but only sets it as "unused". This is to make sure that you can always re-install a plugin, and if you maybe did manual changes, those shall not be simply deleted.

3: There is no specific task for that. The config values are quite low-space so it shouldn't really matter. If you'd need you could use SQL like DELETE FROM serendipity_config WHERE name LIKE '%serendipity_event_XXX%' and do that for all plugins. Of course one could write a script that gathers all installed plugins, and then remove every config var that doesn't match. Could be a nice idea for the maintenance plugin that we have.

Now my question would be if you're willing to move the old plugins back one by one to see which one(s) actually caused the error so that we could fix that plugin. ;) However, it might even not be fixable, if the plugin is locally in that state, parsing the plugin would always create the error and spartacus could never upgrade that. Don't really know how we can circumvent that, except maybe when $serendipity['production'] is true we try to suppress the error. That should happen automagically once $serendipity['version'] no longer contains a "-beta" suffix...

@ddeimeke
Copy link

Hmpf, unfortunately I removed the tarfile ... sorry for that.

But I really think that the plugin was outdated a very long time. So maybe it was already fixed.

Maybe we should head back to the forum. The issue is solved.

  1. If it is not used why should it be updated? In my opinion this does not make any sense. In case it is (re-)activated, Spartacus should be asked, if a more recent version is available.
  2. I get the point. What about asking for confirmation?
  3. As long as the database gets updated, once a plugin is reininstalled, I don't care.

@ophian
Copy link
Member

ophian commented Apr 25, 2014

Dirk, can't you grep for '= &new' in your plugins/ dir?
The only occurrence I could find in additional_plugins is here

additional_plugins/serendipity_plugin_flickr/phpFlickr/PEAR/HTTP/Request.php (1 hit)
    Line 110:  * $a = &new HTTP_Request('http://www.yahoo.com/');

@ddeimeke
Copy link

13:43:35 [root:/srv/www/deimeke.net/dirk/blog/plugins] ⌀ grep -lir "&new" *
serendipity_event_geshi/geshi.bak/php-brief.php
serendipity_event_geshi/geshi.bak/php.php
serendipity_event_geshi/geshi/php-brief.php
serendipity_event_geshi/geshi/rebol.php
serendipity_event_geshi/geshi/php.php
serendipity_event_spamblock/.svn/text-base/serendipity_event_spamblock.php.svn-base
serendipity_event_spartacus/.svn/text-base/serendipity_event_spartacus.php.svn-base
serendipity_plugin_remoterss/.svn/text-base/serendipity_plugin_remoterss.php.svn-base

@ophian
Copy link
Member

ophian commented Apr 25, 2014

Does that include the removed non-used plugins? :)

@ddeimeke
Copy link

Unfortunately I do not have them anymore (I deleted the backup tar-file).

@ddeimeke
Copy link

Took a look into my backup, these are the plugins I deleted:

15:35:32 [root:/srv/www/deimeke.net/dirk/blog/plugins] 3s ⌀ git status | awk '/deleted:/ && /serendipity_/ {print $3}' | sed 'sx/.*$xx' | sort -u
serendipity_event_aggregator
serendipity_event_autotitle
serendipity_event_autoupdate
serendipity_event_blogpdf
serendipity_event_browsercompatibility
serendipity_event_cal
serendipity_event_categorytemplates
serendipity_event_commentspice
serendipity_event_commentspice.zip
serendipity_event_dashboard
serendipity_event_downloadmanager
serendipity_event_dpsyntaxhighlighter
serendipity_event_findmore
serendipity_event_htmlvalidator
serendipity_event_jquery
serendipity_event_kubrickheader
serendipity_event_layout_linkmarkup
serendipity_event_ljupdate
serendipity_event_mobile_output
serendipity_event_mycalendar
serendipity_event_openid
serendipity_event_podcast
serendipity_event_popfetcher
serendipity_event_prettify
serendipity_event_realtimecomments
serendipity_event_recaptcha
serendipity_event_spamblock_bee.zip
serendipity_event_template_editor
serendipity_event_todolist
serendipity_event_trackback
serendipity_event_typesetbuttons.tar
serendipity_event_unstrip_tags
serendipity_event_versioning
serendipity_event_youtube
serendipity_plugin_amazon
serendipity_plugin_currently
serendipity_plugin_pagerank
serendipity_plugin_twitter
serendipity_plugin_uberwach
serendipity_plugin_weather

Checked out additional_plugins.git

15:37:27 [dirk@rico:~/workspace/additional_plugins.git] master ± git grep '&new'
serendipity_event_geshi/geshi/php-brief.php: *  -  Fixed &new problem
serendipity_event_geshi/geshi/php.php: *  -  Fixed &new problem
serendipity_plugin_flickr/phpFlickr/PEAR/HTTP/Request.php: * $a = &new HTTP_Request('http://www.yahoo.com/');

@mattsches
Copy link
Member

I've just finished catching up on all comments on this issue. I agree that the culprit probably was a very old version of one of the plugins listed.

Which of the following things should we do now?

  • Close this issue as fixed.
  • Implement more meaningful debug output once an execption is thrown.
  • Write a method that cleans up outdated and unused plugins (and templates) as requested by Dirk (or even downloads the most recent version of every plugin that is not installed; this could be done in the maintenance plugin, or in Metatron; or in Spartacus).
  • Keep on trying to find the plugins that triggered the execption - OK, this doesn't actually make much sense, right? ;)

If we decide that some of these points are too much work for now - we're trying to get a release ready after all - let's move them to our todo list so we won't forget them later.

@ophian
Copy link
Member

ophian commented Apr 25, 2014

Implement more meaningful debug output once an execption is thrown.

I think we already have that in our errorHandler, which is extremly finetuned to catch all possibilities with different hosts error settings and the Serendipity 'testing/beta/stable(admin)' debug output fallback chain. To catch the rare ones (like this), we have the linked //print_r($args); in testing, which I think is enough.

And, to say this loud, this (new) errorHandler with 1.7 and up, did give us a hell lot of things to repair and notice about since then, which made Serendipity improve and much better, IMHO!

So, I for myself would close this issue as fixed, since I am quite sure it were some old plugin occurrences of serendipity_event_aggregator and serendipity_plugin_amazon.

And yes, writing an unused plugin directory purger eg. via recursive_directory_iterator() by user request in maintenance sounds like a good idea for the future.

@ophian
Copy link
Member

ophian commented Apr 26, 2014

@garvinhicking The plugin serendipity_event_aggregator uses its own SimplePie lib, which is older than the one in bundled libs. Maybe that is (a hidden/another) breakpoint in this issue....

@ddeimeke
Copy link

Please help me, I have one problem in understanding the update mechanism.

Before I updated to beta1 (and afterwards beta2 via GitHub) I updated all plugins.

Why does the updater iterates through all plugins lying in the plugin directory and does not do anything? I would think that the updater finds outdated plugins and offers the update. In case it does not, there is no need to iterate through every plugin in the directory.

Please give me the missing link.

@ophian
Copy link
Member

ophian commented Apr 26, 2014

I would say Spartacus (Plugin Updater Check) iterates over all activated or hidden set plugins only.
If not activated or set hidden, a only physically stored plugin is not in the checked plugin array, which is (IMO) build by the database serendipity_plugins table, for plugins being in hide, left, right, event or eventh.
So when you updated your Plugins before the Core upgrade, only active and hidden set plugins were checked for an update via the xml file.

The other (independent) thing is the Serendipity Plugin class behaviour (Garvins answer to 1), which checks/parses all plugins currently stored in /plugins, to get an idea of what could happen next. That is one of the reasons why you cannot have two of the same name, eg. serendipity_class_xyz, or some with breaking errors in the workflow.

@yellowled yellowled modified the milestones: Future, RC May 11, 2014
@yellowled yellowled modified the milestones: 2.0.x, Future Jan 22, 2015
@garvinhicking
Copy link
Member

I think this issue is either unclear to me or fixed; please elaborate what the problem is, if there is any reamining, so that we can address it properly. Until then I'll close it. :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants