Plugin fails to activate using TGMPA #6459

ktmn · 2021-07-17T07:24:54Z

Bug Description

When trying to activate the plugin using TGMPA I get the following error:

Fatal error: Uncaught AmpProject\AmpWP\Exception\InvalidService: The service ID "dev_tools.user_access" is not recognized and cannot be retrieved. in .../wp-content/plugins/amp/src/Exception/InvalidService.php:57 
Stack trace: 
#0 .../wp-content/plugins/amp/src/Infrastructure/ServiceContainer/SimpleServiceContainer.php(39): AmpProject\AmpWP\Exception\InvalidService::from_service_id() 
#1 .../wp-content/plugins/amp/src/Services.php(55): AmpProject\AmpWP\Infrastructure\ServiceContainer\SimpleServiceContainer->get() 
#2 .../wp-content/plugins/amp/src/Admin/ValidationCounts.php(47): AmpProject\AmpWP\Services::get() 
#3 .../wp-content/plugins/amp/src/Infrastructure/ServiceBasedPlugin.php(288): AmpProject\AmpWP\Admin\ValidationCounts::is_needed() 
#4 .../wp-content/plugins/amp/src/Infrastructure/ServiceBasedPlugin.php(199): AmpProject\AmpWP\Infrastructure\ServiceBasedPlugin->register_service() 
#5 .../wp-content/plugins/amp/src/Infrastructure/ServiceBasedPlugin.php(103): AmpProject\AmpWP\Infrastructure\ServiceBasedPlugin->register_services() 
#6 .../wp-content/plugins/amp/includes/amp-helper-functions.php(49): AmpProject\AmpWP\Infrastructure\ServiceBasedPlugin->activate() 
#7 .../wp-includes/class-wp-hook.php(292): amp_activate() 
#8 .../wp-includes/class-wp-hook.php(316): WP_Hook->apply_filters() 
#9 .../wp-includes/plugin.php(484): WP_Hook->do_action() 
#10 .../wp-admin/includes/plugin.php(701): do_action() 
#11 .../wp-content/mu-plugins/TGM-Plugin-Activation-2.6.1/class-tgm-plugin-activation.php(1050): activate_plugin() 
#12 .../wp-content/mu-plugins/TGM-Plugin-Activation-2.6.1/class-tgm-plugin-activation.php(931): TGM_Plugin_Activation->activate_single_plugin() 
#13 .../wp-content/mu-plugins/TGM-Plugin-Activation-2.6.1/class-tgm-plugin-activation.php(751): TGM_Plugin_Activation->do_plugin_install() 
#14 .../wp-includes/class-wp-hook.php(292): TGM_Plugin_Activation->install_plugins_page() 
#15 .../wp-includes/class-wp-hook.php(316): WP_Hook->apply_filters() 
#16 .../wp-includes/plugin.php(484): WP_Hook->do_action() 
#17 .../wp-admin/admin.php(259): do_action() 
#18 .../wp-admin/plugins.php(10): require_once('...') 
#19 {main} thrown in .../wp-content/plugins/amp/src/Exception/InvalidService.php on line 57

Expected Behaviour

It used to work with TGMPA I think.

Steps to reproduce

Generate a TGMPA "Plugin" here: http://tgmpluginactivation.com/download/
Add the plugin to /wp-content/mu-plugins/TGM-Plugin-Activation-2.6.1
Change the $plugins variable in /mu-plugins/TGM-Plugin-Activation-2.6.1/example.php to:

$plugins = array(
	// This is an example of how to include a plugin from the WordPress Plugin Repository.
	array(
		'name'      => 'AMP',
		'slug'      => 'amp',
		'required'  => false,
	),
);

Create /mu-plugins/tgmpa.php with the content <?php require_once __DIR__ . '/TGM-Plugin-Activation-2.6.1/example.php';
In the WordPress admin dashboard you should be prompted to install the AMP plugin (assuming it's not already installed).
Once installed, you should be prompted to activate the AMP plugin.
When trying to activate through TGMPA you should get the error, while when activating from Plugins page it activates fine.

Screenshots

Additional context

WordPress version: 5.7.2
Plugin version: 2.1.3
PHP version: 8.0.0
OS: Win 10/WSL2
Browser: Chrome

Site health:

### wp-core ###

version: 5.7.2
site_language: en_US
user_language: en_US
timezone: +00:00
permalink: /%year%/%monthnum%/%day%/%postname%/
https_status: false
multisite: false
user_registration: 0
blog_public: 1
default_comment_status: open
environment_type: production
user_count: 1
dotorg_communication: true

### wp-paths-sizes ###

wordpress_path: .../public_html
wordpress_size: 44.27 MB (46423796 bytes)
uploads_path: .../public_html/wp-content/uploads
uploads_size: 0.00 B (0 bytes)
themes_path: .../public_html/wp-content/themes
themes_size: 6.44 MB (6755717 bytes)
plugins_path: .../public_html/wp-content/plugins
plugins_size: 8.08 MB (8467305 bytes)
database_size: 1.66 MB (1736704 bytes)
total_size: 60.45 MB (63383522 bytes)

### wp-active-theme ###

name: Twenty Twenty-One (twentytwentyone)
version: 1.3
author: the WordPress team
author_website: https://wordpress.org/
parent_theme: none
theme_features: core-block-patterns, automatic-feed-links, title-tag, post-formats, post-thumbnails, menus, html5, custom-logo, customize-selective-refresh-widgets, wp-block-styles, align-wide, editor-styles, editor-style, editor-font-sizes, custom-background, editor-color-palette, editor-gradient-presets, responsive-embeds, custom-line-height, experimental-link-color, custom-spacing, custom-units, widgets
theme_path: .../public_html/wp-content/themes/twentytwentyone
auto_update: Disabled

### wp-themes-inactive (2) ###

Twenty Nineteen: version: 2.0, author: the WordPress team, Auto-updates disabled
Twenty Twenty: version: 1.7, author: the WordPress team, Auto-updates disabled

### wp-mu-plugins (1) ###

tgmpa.php: author: (undefined), version: (undefined)

### wp-plugins-active (2) ###

WordPress Beta Tester: version: 3.1.1, author: Peter Westwood, Andy Fragen, Auto-updates disabled
WP Auto Login: version: 1.0.1, author: Ren Ventura, Auto-updates disabled

### wp-plugins-inactive (1) ###

AMP: version: 2.1.3, author: AMP Project Contributors, Auto-updates disabled

### wp-media ###

image_editor: WP_Image_Editor_GD
imagick_module_version: Not available
imagemagick_version: Not available
file_uploads: File uploads is turned off
post_max_size: 128M
upload_max_filesize: 256M
max_effective_size: 128 MB
max_file_uploads: 20
gd_version: 2.3.0
ghostscript_version: not available

### wp-server ###

server_architecture: Linux 4.19.128-microsoft-standard x86_64
httpd_software: Apache/2.4.41 (Ubuntu)
php_version: 8.0.0 64bit
php_sapi: apache2handler
max_input_variables: 1000
time_limit: 60
memory_limit: 256M
max_input_time: 60
upload_max_filesize: 256M
php_post_max_size: 128M
curl_version: 7.68.0 OpenSSL/1.1.1f
suhosin: false
imagick_availability: false
pretty_permalinks: true
htaccess_extra_rules: false

### wp-database ###

extension: mysqli
server_version: 8.0.21-0ubuntu0.20.04.4
client_version: mysqlnd 8.0.0

### wp-constants ###

WP_HOME: undefined
WP_SITEURL: undefined
WP_CONTENT_DIR: .../public_html/wp-content
WP_PLUGIN_DIR: .../public_html/wp-content/plugins
WP_MEMORY_LIMIT: 40M
WP_MAX_MEMORY_LIMIT: 256M
WP_DEBUG: false
WP_DEBUG_DISPLAY: true
WP_DEBUG_LOG: false
SCRIPT_DEBUG: false
WP_CACHE: false
CONCATENATE_SCRIPTS: undefined
COMPRESS_SCRIPTS: undefined
COMPRESS_CSS: undefined
WP_LOCAL_DEV: undefined
DB_CHARSET: utf8
DB_COLLATE: undefined

### wp-filesystem ###

wordpress: writable
wp-content: writable
uploads: writable
plugins: writable
themes: writable
mu-plugins: writable

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

Acceptance criteria

Implementation brief

QA testing instructions

Demo

Changelog entry

The text was updated successfully, but these errors were encountered:

pierlon · 2021-07-19T14:36:15Z

Hi @ktmn thanks for reporting the bug. The issue seems to be how the TGMPA plugin activates plugins. By the time it calls activate_plugin(), much of the admin screen has already been initialized , leading to some of our delayed services being immediately registered and thus causing errors like this. Specifically, in this case the dev_tools.user_access service is being immediately registered upon plugin activation due to the admin_enqueue_scripts action already being executed.

I'm not sure how to resolve this correctly. Maybe if a service is delayed and depends upon another service wait for the dependent service to be registered first? Any thoughts @westonruter @schlessera?

schlessera · 2021-07-19T15:37:30Z

@pierlon We can add dependency resolution to the services, so that the services can state which other services they depend on, and only get registered once all dependencies are met.

I've done that in the past on more complex projects, and it works very well. It also allows you to define "virtual services" whose only purpose is to serve as a dependency, like a UserIsLoggedIn service or an OnCheckoutPage service.

(see this and the following slides for a case study of how I solved that in the past: https://schlessera.github.io/wcldn-2017/#/dependency-resolution-1)

pierlon · 2021-07-19T16:10:16Z

Dependency resolution for the services sounds perfect for the job. Also thanks for sharing those slides!

westonruter · 2021-07-19T23:25:32Z

@schlessera something like described as part of ampproject/amp-toolbox-php#219?

schlessera · 2021-07-20T05:20:36Z

@westonruter It would probably need to be a little more robust, but generally yes. Is this something you want me to take on, at least for the initial design?

westonruter · 2021-07-20T05:55:26Z

@schlessera yes, please do. Up until now there has been a need to order the services in a specific way to avoid such dependency resolution problems.

@pierlon Maybe you'd like to implement this with input from Alain?

pierlon · 2021-07-20T11:01:39Z

@pierlon Maybe you'd like to implement this with input from Alain?

Sure that sounds like a plan.

westonruter · 2021-08-05T00:23:44Z

@ktmn The fix has been merged and cherry-picked into the 2.1 branch. It would be very helpful if you could test the 2.1.x production build linked from the Development Builds wiki page.

westonruter · 2021-08-10T17:51:32Z

@pierlon @schlessera There seems to be a regression introduced with #6484. In particular, the ValidationCounts service is no longer getting registered:

I checked in both in the develop branch and the 2.1 branch and I see the same thing. If I try the 2.1.3 release, then I get the ValidationCounts loading as expected.

ktmn · 2021-08-18T16:03:45Z

The fix has been merged and cherry-picked into the 2.1 branch. It would be very helpful if you could test the 2.1.x production build linked from the Development Builds wiki page.

Works for me, thanks!

westonruter · 2021-08-30T23:00:45Z

QA Passed

I added the “Activate AMP Plugin” plugin as described in #6484:

Upon activating, the AMP plugin was also activated successfully:

Nothing is added to the error log.

ktmn added the Bug Something isn't working label Jul 17, 2021

westonruter added this to the v2.1.4 milestone Jul 19, 2021

pierlon self-assigned this Jul 25, 2021

pierlon mentioned this issue Jul 25, 2021

Implement dependency resolution for services #6484

Merged

2 tasks

westonruter closed this as completed in #6484 Aug 4, 2021

westonruter reopened this Aug 10, 2021

pierlon mentioned this issue Aug 11, 2021

Fix service registration delay causing next service to be skipped #6529

Merged

2 tasks

westonruter closed this as completed in #6529 Aug 13, 2021

westonruter assigned westonruter and unassigned pierlon Aug 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Plugin fails to activate using TGMPA #6459

Plugin fails to activate using TGMPA #6459

ktmn commented Jul 17, 2021

pierlon commented Jul 19, 2021

schlessera commented Jul 19, 2021

pierlon commented Jul 19, 2021

westonruter commented Jul 19, 2021

schlessera commented Jul 20, 2021

westonruter commented Jul 20, 2021

pierlon commented Jul 20, 2021

westonruter commented Aug 5, 2021

westonruter commented Aug 10, 2021

ktmn commented Aug 18, 2021

westonruter commented Aug 30, 2021

Plugin fails to activate using TGMPA #6459

Plugin fails to activate using TGMPA #6459

Comments

ktmn commented Jul 17, 2021

Bug Description

Expected Behaviour

Steps to reproduce

Screenshots

Additional context

Acceptance criteria

Implementation brief

QA testing instructions

Demo

Changelog entry

pierlon commented Jul 19, 2021

schlessera commented Jul 19, 2021

pierlon commented Jul 19, 2021

westonruter commented Jul 19, 2021

schlessera commented Jul 20, 2021

westonruter commented Jul 20, 2021

pierlon commented Jul 20, 2021

westonruter commented Aug 5, 2021

westonruter commented Aug 10, 2021

ktmn commented Aug 18, 2021

westonruter commented Aug 30, 2021