cedaro / satispress

Expose installed WordPress plugins and themes as Composer packages.
500 stars 49 forks source link

Fatal Error when updating Visual Composer #106

Closed tomasvanrijsse closed 3 years ago

tomasvanrijsse commented 4 years ago

I got this error when I was filling in plugin licenses and updating some.

PHP message: PHP Fatal error:  Uncaught TypeError: Argument 3 passed to SatisPress\\Release::__construct() must be of the type string, boolean given, called in /wp-content/plugins/satispress/src/Provider/PackageArchiver.php on line 172 and defined in /wp-content/plugins/satispress/src/Release.php:50

Stack trace:
#0 /wp-content/plugins/satispress/src/Provider/PackageArchiver.php(172): SatisPress\\Release->__construct(Object(SatisPress\\PackageType\\Plugin), '6.0.5', true)
#1 /wp-includes/class-wp-hook.php(288): SatisPress\\Provider\\PackageArchiver->archive_updates(Object(stdClass))
#2 /wp-includes/plugin.php(208): WP_Hook->apply_filters(Object(stdClass), Array)
#3 /wp-includes/option.php(1815): apply_filters('pre_set_site_tr...'...',

The error was fixed by removing the plugin js_composer a.k.a Visual Composer. Both VisualComposer and Satispress are the latest version (respectively 6.0.3 and 0.4.1)

tomasvanrijsse commented 4 years ago

I tried to reproduce this error but Visual Composer is already on 6.0.5 and this error doesn't occur anymore.

bradyvercher commented 4 years ago

Thanks for following up on this, @tomasvanrijsse! Feel free to reopen this if you start receiving the error again in the future.

tyrann0us commented 4 years ago

Just for the records: We faced the same issue while updating WPBakery Page Builder (formerly Visual Composer) to 6.0.5. I had to rename the satispress plugin dir to disable it, only then I was able to update the page builder plugin. Afterwards, I could re-enable SatisPress.

tyrann0us commented 4 years ago

@bradyvercher, there has been an update to the plugin "WPBakery Page Builder" today (v6.1.0, see https://kb.wpbakery.com/docs/preface/release-notes/) that started to trigger the issue again.

Please consider reopening the issue. Thanks!

bradyvercher commented 4 years ago

@tyrann0us I don't use WPBakery, so don't have access to it to troubleshoot. I'll need more information about what exactly is going wrong or if someone can get in touch with me directly with a copy, I can try to take a look.

bradyvercher commented 4 years ago

@tyrann0us I don't have enough information to look into this. If you or anyone else has anything else that's helpful, feel free to get in touch with me or post it here.

tyrann0us commented 4 years ago

@bradyvercher, I will discuss this internally in my company this week and get back to you afterwards.

tyrann0us commented 4 years ago

@bradyvercher, I just dropped you an email.

bradyvercher commented 4 years ago

@tyrann0us Thanks for sending everything over! I spent a good deal of time this morning taking a look, but I don't think there's an easy way to add support.

The problem is that when WordPress checks for package updates, Visual Composer just checks to see if a new version is available. If there is a new release, then it inserts unexpected data into the update_plugins transient, which is what's causing the issue.

WordPress expects a URL for the package key in the transient when an update is available, but Visual Composer sets the value to a boolean based on whether the license is active. It then hooks into upgrader_pre_download just before a package is downloaded and retrieves a download link that expires after 120 seconds.

At the very least, SatisPress should prevent the errors that are occurring, but that wouldn't provide any feedback about updates for Visual Composer not being automatically cached. Adding full support would likely require writing a custom adapter.

tyrann0us commented 4 years ago

Thanks @bradyvercher for the detailed check! Since I won't have time for this in the near future, I was wondering if you, @tomasvanrijsse, could open a ticket at support.wpbakery.com and report the problem (including a link to this issue)? Thanks!

tomasvanrijsse commented 4 years ago

@tyrann0us I'm not working for the same company anymore and I'm not able to submit a ticket for this issue.

tyrann0us commented 4 years ago

@pavelthq, @say2me, @MMakijenko, it seems you work for WPBakery. Would it be possible to take a look at what @bradyvercher wrote here (or forward it to the people in charge)? https://github.com/cedaro/satispress/issues/106#issuecomment-617907883.

Seems like WPBakery Page Builder's auto updater does quite some "unusual stuff" with the update_plugins transient which breaks other plugins. Thanks!

Edit: Note that both the title of the issue and the comment refers to "Visual Composer", but it is about WPBakery Page Builder.

pavelthq commented 4 years ago

@tyrann0us how can we replicate that locally? maybe @bradyvercher can send some files to my email pavel@wpbakery.com and then I will check. we need to see an error locally to define how to solve this on our side.

also, there is not yet planned bugfixes releases, so it will make time till new release appear.

pavelthq commented 4 years ago

If you were right, this is a patch:

Index: include/classes/updaters/class-vc-updater.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- include/classes/updaters/class-vc-updater.php   (revision 0c56c52d5ab56288871cdd5c6aa1a6eedd724b51)
+++ include/classes/updaters/class-vc-updater.php   (date 1587983687797)
@@ -138,7 +138,7 @@

        if ( ! vc_license()->isActivated() ) {
            if ( vc_is_as_theme() && vc_get_param( 'action' ) !== 'update-selected' ) {
-               return false;
+               return $reply;
            }
            $url = self::getUpdaterUrl();

but, as I said the next release not planned yet, and it may take time till new release will be published.

tyrann0us commented 4 years ago

@pavelthq, thanks for responding so fast!

@tyrann0us how can we replicate that locally?

It should be possible to replicate the issue (locally) by following these steps:

  1. Install SatisPress (refer to the documentation for more details)
  2. Install and activate an outdated version of WPBakery, but don't activate it yet
  3. Enable it for SatisPress
  4. Activate WPBakery plugin
  5. Activate the license
  6. Trigger an update

SatisPress will now try to "inject" itself into the update process to archive the new version and should fail with the error mentioned before.

If you were right, this is a patch:

I won't have time to test it. If you follow the steps, you should be able to test it yourself.

but, as I said the next release not planned yet, and it may take time till new release will be published.

Kind of off-topic, but maybe it would be worth considering providing a composer repository for WPBakery Page Builder, so people wouldn't need SatisPress for that at all. Something similar to how Delicious Brains does it for WP Migrate DB Pro (https://deliciousbrains.com/wp-migrate-db-pro/doc/installing-via-composer/) or Yoast for Yoast SEO Premium (https://yoast.com/help/how-to-install-yoast-plugins-using-composer/). Thanks!

tyrann0us commented 3 years ago

Hi @pavelthq, WPBakery Page Builder v6.4.2 has been released (https://kb.wpbakery.com/docs/preface/release-notes/) and it again broke SatisPress. 😞

Unfortunately, the changes discussed here https://github.com/cedaro/satispress/issues/106#issuecomment-619893029 have not been implemented.

See this screenshot of v6.4.1 vs. v6.4.2; the file is identical:

Bildschirmfoto 2020-11-20 um 06 18 42

This gives us (and probably all SatisPress users who install WPBakery Page Builder) a headache and we have to repeatedly spend time to solve the issue. Time that we would like to use productively elsewhere.

It would be really great if you could create a ticket internally so that this is not forgotten again.

Thanks!

bradyvercher commented 3 years ago

@tyrann0us I made an update in the latest release that should prevent fatal errors from being caused by WPBakery, so let me know if you're still having issues with that after updating.

On top of that, I pushed up a new branch that should allow an adapter to be written to retrieve updates from the WPBakery website if you'd like to look into that further.

bradyvercher commented 3 years ago

I removed the adapter for the latest release since I don't have a way to test it, but at the very least, new releases shouldn't throw errors.

pavelthq commented 3 years ago

6.5.0 contains also fixes return $reply in failure, so issue should be resolved