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

critical error on this website: An error of type E_COMPILE_ERROR was caused #3830

Closed
jamesozzie opened this issue Aug 11, 2021 · 8 comments
Closed
Labels
P0 High priority QA: Eng Requires specialized QA by an engineer Type: Bug Something isn't working Type: Support Support request
Milestone

Comments

@jamesozzie
Copy link
Collaborator

jamesozzie commented Aug 11, 2021

Bug Description

A support topic was opened in relation to users encountering a critical WordPress error, with 2 users reporting the error coming from Site Kit.

One user has encountered this on all the sites where he has Site Kit active on. Users are unable to access their site, I suspect both front end and admin panel. Once Site Kit is re-activated after "restoring" their site via a recovery link the same issue occurs.

The error appears as below:
An error of type E_COMPILE_ERROR was caused in line 34 of the file /var/www/wp-content/plugins/google-site-kit/includes/loader.php. Error message: require_once(): Failed opening required ‘/var/www/wp-content/plugins/google-site-kit/third-party/vendor/composer/InstalledVersions.php’ (include_path=’.:/opt/remi/php73/root/usr/share/pear:/opt/remi/php73/root/usr/share/php:/usr/share/pear:/usr/share/php’)

image

image

There have been other mentions of this on social media. awaiting further insights, unreproducible in support so far.

Support Topic(s):
https://wordpress.org/support/topic/error-there-has-been-a-critical-error-on-this-website-3/ - Open - no SH info

Additional Context

  • Site Kit 1.38.0
  • Possible multiple users impacted
  • No obvious cause looking at the front end of one users site. There are ads related console errors

Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

  • Site Kit should not try to autoload classes which are not part of the Site Kit build ZIP.

Implementation Brief

Test Coverage

  • N/A

Visual Regression Changes

  • N/A

QA Brief

  • Verify that the autoloader used in the SK plugin doesn't include classes that start not from the Google\Site_Kit namespace.

Changelog entry

  • Fix bug where other plugins using an unprefixed version of Composer could cause Site Kit to trigger a fatal error.
@jamesozzie jamesozzie self-assigned this Aug 11, 2021
@jamesozzie jamesozzie added Type: Bug Something isn't working Type: Support Support request labels Aug 11, 2021
@eugene-manuilov eugene-manuilov self-assigned this Aug 11, 2021
@felixarntz
Copy link
Member

@eugene-manuilov @bethanylang If this is not caused by some odd edge-case situation, definitely a hot fix candidate. At a minimum, we'd need to include it in 1.39.0.

@eugene-manuilov
Copy link
Collaborator

@felixarntz Ok, I think I figured out what could cause this issue. Our classmap list (in the autoload_classmap.php file) contains a record for the Composer\InstalledVersions class added by composer automatically that points to the {$vendorDir}/composer/InstalledVersions.php file. The problem is that we don't include the vendor folder into our release build and therefore there is no such file on the disk when another plugin or a theme tries to use the InstalledVersions class and our autoloader tries to require that file since that class is listed in our classmap list.

// autoload_classmap.php

...

return array(
    'Composer\\InstalledVersions' => $vendorDir . '/composer/InstalledVersions.php',
    'Google\\Site_Kit\\Context' => $baseDir . '/Context.php',
    'Google\\Site_Kit\\Core\\Admin\\Available_Tools' => $baseDir . '/Core/Admin/Available_Tools.php',
    'Google\\Site_Kit\\Core\\Admin\\Dashboard' => $baseDir . '/Core/Admin/Dashboard.php',
    'Google\\Site_Kit\\Core\\Admin\\Notice' => $baseDir . '/Core/Admin/Notice.php',
    ...
);

I think we need to update our autoload_classes function to require classes only if they start with Google\\Site_Kit namespace. Something like this:

 function autoload_classes() {
 	$class_map = array_merge(
 		// Site Kit classes.
 		include GOOGLESITEKIT_PLUGIN_DIR_PATH . 'includes/vendor/composer/autoload_classmap.php',
 		// Third-party classes.
 		include GOOGLESITEKIT_PLUGIN_DIR_PATH . 'third-party/vendor/composer/autoload_classmap.php'
 	);

 	spl_autoload_register(
 		function ( $class ) use ( $class_map ) {
-			if ( isset( $class_map[ $class ] ) ) {
+			if ( isset( $class_map[ $class ] ) && substr( $class, 0, 15 ) === 'Google\\Site_Kit' ) {
 				require_once $class_map[ $class ];
 
 				return true;
 			}
 		},
 		true,
 		true
 	);
 }

What do you think? cc @aaemnnosttv

@felixarntz
Copy link
Member

felixarntz commented Aug 11, 2021

@eugene-manuilov This seems like a rather critical fix to push. Wouldn't it be more appropriate to provide a file_exists check though? Nevermind, on second thought I agree we should limit it to Site Kit classes only.

Can you get a PR up for this against main? If we want to push as a hotfix, the change may be simple enough so we can just modify the latest ZIP.

@eugene-manuilov
Copy link
Collaborator

@felixarntz PR #3835 is created.

@felixarntz
Copy link
Member

@eugene-manuilov IB / PR ✅

Can you add a QA Brief? Would be good to test with one of the plugins that were reported to result in that error due to their usage of Composer. Also we need to make sure to test with the ZIP build and not a regular main or develop build.

@felixarntz felixarntz added this to the Sprint 55 milestone Aug 11, 2021
@eugene-manuilov eugene-manuilov added the QA: Eng Requires specialized QA by an engineer label Aug 11, 2021
@eugene-manuilov
Copy link
Collaborator

@felixarntz QAB is added. Unfortunately, we don't know which plugin uses the Composer\InstalledVersions class.

@eugene-manuilov eugene-manuilov removed their assignment Aug 11, 2021
@felixarntz
Copy link
Member

@felixarntz felixarntz self-assigned this Aug 11, 2021
@felixarntz
Copy link
Member

QA: Eng ✅

Due to the extra logic it's no longer possible for a non-Site Kit class to be accidentally picked up by Site Kit's autoloader. Only classes starting in Google\Site_Kit are considered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P0 High priority QA: Eng Requires specialized QA by an engineer Type: Bug Something isn't working Type: Support Support request
Projects
None yet
Development

No branches or pull requests

3 participants