Skip to content

Commit 8c80ae9

Browse files
authored
[Blueprints] Don't treat extra output during plugin activation as a failure (#2904)
Some WordPress plugins emit output during activation. Before this PR, Playground treated that as an activation error. But that's not right, the plugin may still get activated. This PR checks in with WordPress whether or not the plugin is activated after we're tried to activate it. If it is, that's our success. ## Testing Instructions (or ideally a Blueprint) CI, this PR ships a new test.
1 parent 32aa2ff commit 8c80ae9

File tree

2 files changed

+89
-46
lines changed

2 files changed

+89
-46
lines changed

packages/playground/blueprints/src/lib/steps/activate-plugin.spec.ts

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,52 @@ describe('Blueprint step activatePlugin()', () => {
180180
).resolves.not.toThrow();
181181
});
182182

183+
it('should log noisy activation output and still treat the plugin as active', async () => {
184+
const docroot = handler.documentRoot;
185+
php.writeFile(
186+
`${docroot}/wp-content/plugins/noisy-plugin.php`,
187+
`<?php
188+
/**
189+
* Plugin Name: Noisy Plugin
190+
*/
191+
register_activation_hook( __FILE__, function() {
192+
echo 'Activation says hi';
193+
} );
194+
195+
register_shutdown_function( function() {
196+
echo 'Shutdown chimes in too';
197+
} );
198+
`
199+
);
200+
201+
await expect(
202+
activatePlugin(php, {
203+
pluginPath: 'noisy-plugin.php',
204+
})
205+
).resolves.not.toThrow();
206+
});
207+
208+
it('should throw an error if the plugin was not activated and noisy output is present', async () => {
209+
const docroot = handler.documentRoot;
210+
php.writeFile(
211+
`${docroot}/wp-content/plugins/noisy-plugin.php`,
212+
`<?php
213+
/**
214+
* Plugin Name: Noisy Plugin
215+
*/
216+
register_activation_hook( __FILE__, function() {
217+
throw new Exception( 'Activation failed' );
218+
} );
219+
`
220+
);
221+
222+
await expect(
223+
activatePlugin(php, {
224+
pluginPath: 'noisy-plugin.php',
225+
})
226+
).rejects.toThrow(/Uncaught Exception: Activation failed/);
227+
});
228+
183229
it('should not throw an error if the plugin is already active', async () => {
184230
const docroot = handler.documentRoot;
185231
php.writeFile(

packages/playground/blueprints/src/lib/steps/activate-plugin.ts

Lines changed: 43 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,21 @@ export const activatePlugin: StepHandler<ActivatePluginStep> = async (
3737
progress?.tracker.setCaption(`Activating ${pluginName || pluginPath}`);
3838

3939
const docroot = await playground.documentRoot;
40+
/**
41+
* Instead of checking the plugin activation response,
42+
* check if the plugin is active by looking at the active plugins list.
43+
*
44+
* We have to split the activation and the check into two PHP runs
45+
* because some plugins might redirect during activation,
46+
* which would prevent any output that happens after activation from being returned.
47+
*
48+
* Relying on the plugin activation response is not reliable because if the plugin activation
49+
* produces any output, WordPress will assume it's an activation error and return a WP_Error.
50+
* WordPress will still activate the plugin and load the required page,
51+
* but it will also show the error as a notice in wp-admin.
52+
* See WordPress source code for more details:
53+
* https://github.com/WordPress/wordpress-develop/blob/6.7/src/wp-admin/includes/plugin.php#L733
54+
*/
4055
const activatePluginResult = await playground.run({
4156
code: `<?php
4257
define( 'WP_ADMIN', true );
@@ -81,47 +96,18 @@ export const activatePlugin: StepHandler<ActivatePluginStep> = async (
8196
}
8297

8398
/**
84-
* Instead of checking the plugin activation response,
85-
* check if the plugin is active by looking at the active plugins list.
99+
* Instead of trusting the activation response, check the active plugins list.
86100
*
87-
* We have to split the activation and the check into two PHP runs
88-
* because some plugins might redirect during activation,
89-
* which would prevent any output that happens after activation from being returned.
90-
*
91-
* Relying on the plugin activation response is not reliable because if the plugin activation
92-
* produces any output, WordPress will assume it's an activation error and return a WP_Error.
93-
* WordPress will still activate the plugin and load the required page,
94-
* but it will also show the error as a notice in wp-admin.
95-
* See WordPress source code for more details:
96-
* https://github.com/WordPress/wordpress-develop/blob/6.7/src/wp-admin/includes/plugin.php#L733
97-
*
98-
* Because some plugins can create an output, we need to use output buffering
99-
* to ensure the 'true' response is not polluted by other outputs.
100-
* If the plugin activation fails, we will return the buffered output as it might
101-
* contain more information about the failure.
101+
* We try to discard any extra output via output buffering. The output of the script below
102+
* end with `{"success": true}` or `{"success": false}`. Only `{"success": true}` is
103+
* treated as a successful plugin activation.
102104
*/
103-
const isActiveCheckResult = await playground.run({
105+
const activationStatusResult = await playground.run({
104106
code: `<?php
105107
ob_start();
106108
require_once( getenv( 'DOCROOT' ) . "/wp-load.php" );
107109
108-
/**
109-
* Extracts the relative plugin path from either an absolute or relative plugin path.
110-
*
111-
* Absolute paths starting with plugin directory (e.g., '/wordpress/wp-content/plugins/test-plugin/index.php')
112-
* should be converted to relative paths (e.g., 'test-plugin/index.php')
113-
*
114-
* Directories should finish with a trailing slash to ensure we match the full plugin directory name.
115-
*
116-
* Examples:
117-
* - '/wordpress/wp-content/plugins/test-plugin/index.php' → 'test-plugin/index.php'
118-
* - '/wordpress/wp-content/plugins/test-plugin/' → 'test-plugin/'
119-
* - '/wordpress/wp-content/plugins/test-plugin' → 'test-plugin/'
120-
* - 'test-plugin/index.php' → 'test-plugin/index.php'
121-
* - 'test-plugin/' → 'test-plugin/'
122-
* - 'test-plugin' → 'test-plugin/'
123-
*/
124-
$plugin_directory = WP_PLUGIN_DIR . '/';
110+
$plugin_directory = rtrim( WP_PLUGIN_DIR, '/' ) . '/';
125111
$relative_plugin_path = getenv( 'PLUGIN_PATH' );
126112
if (strpos($relative_plugin_path, $plugin_directory) === 0) {
127113
$relative_plugin_path = substr($relative_plugin_path, strlen($plugin_directory));
@@ -132,28 +118,39 @@ export const activatePlugin: StepHandler<ActivatePluginStep> = async (
132118
}
133119
134120
$active_plugins = get_option( 'active_plugins' );
135-
foreach ( $active_plugins as $plugin ) {
136-
if ( substr( $plugin, 0, strlen( $relative_plugin_path ) ) === $relative_plugin_path ) {
137-
ob_end_clean();
138-
die( 'true' );
139-
}
121+
if ( ! is_array( $active_plugins ) ) {
122+
$active_plugins = array();
140123
}
141-
die( ob_get_flush() ?: 'false' );
124+
ob_end_clean();
125+
126+
/**
127+
* Use a shutdown function to ensure the activation-related output comes
128+
* last in stdout.
129+
*/
130+
register_shutdown_function( function() use ( $relative_plugin_path, $active_plugins ) {
131+
foreach ( $active_plugins as $plugin ) {
132+
if ( substr( $plugin, 0, strlen( $relative_plugin_path ) ) === $relative_plugin_path ) {
133+
die('{"success": true}');
134+
break;
135+
}
136+
}
137+
die('{"success": false}');
138+
});
142139
`,
143140
env: {
144141
DOCROOT: docroot,
145142
PLUGIN_PATH: pluginPath,
146143
},
147144
});
148145

149-
if (isActiveCheckResult.text === 'true') {
150-
// Plugin activation was successful, yay!
146+
const rawStatus = (activationStatusResult.text ?? '').trim();
147+
if (rawStatus.endsWith('{"success": true}')) {
151148
return;
152149
}
153-
154-
if (isActiveCheckResult.text !== 'false') {
155-
logger.debug(isActiveCheckResult.text);
150+
if (rawStatus !== '{"success": false}') {
151+
logger.debug(rawStatus);
156152
}
153+
157154
throw new Error(
158155
`Plugin ${pluginPath} could not be activated – WordPress exited with no error. ` +
159156
`Sometimes, when $_SERVER or site options are not configured correctly, ` +

0 commit comments

Comments
 (0)