Blueprints: Remove the applyWordPressPatches step, enable the Site Health Plugin #1001
Conversation
Prior to this PR, Playground would apply a series of patches to the WordPress source code to make it compatible with the PHP-Wasm runtime. This PR removes the applyWordPressPatches step. All the adjustments are now done via filters in the Playground mu-plugin. ## List of changes * The Site Health plugin is now enabled when the networking is disabled. Unfortunately, Playground
…alth Plugin Prior to this PR, Playground would apply a series of patches to the WordPress source code to make it compatible with the PHP-Wasm runtime. This PR removes the applyWordPressPatches step. All the adjustments are now done via filters in the Playground mu-plugin. ## List of changes * Site Health plugin is now enabled by default. * The PHPRequestHandler will return a 502 status code when a request is received while another request is still being handled. Previously it would deadlock as handling a request requires an exclusive lock on the PHP instance. This prevents the Site Health plugin from trigerring a deadlock by requesting the WordPress site from inside the PHP code. * Playground mu-plugin lives in the playground-remote package, not in the Blueprints package. The plugin is only used by the Web Worker where Playground is set up, so let's bundle those together. * Network transports are now a part of the Playground mu-plugin and are not created conditionally. This solves #977. ## Testing instructions * Confirm the CI checks pass. * Customize the WordPress site in Playground, export it, restart Playground, import the file, confirm it worked. * Try out the WordPress PR previewer and confirm it still works.
adamziel
added
[Type] Enhancement
|
The unit test failure is intermittent and needs to be addressed separately. |
| if ( | ||
| this.#semaphore.running > 0 && | ||
| request.headers?.['x-request-issuer'] === 'php' | ||
| ) { | ||
| console.warn( | ||
| `Possible deadlock: Called request() before the previous request() have finished. ` + | ||
| `PHP likely issued an HTTP call to itself. Normally this would lead to infinite ` + | ||
| `waiting as Request 1 holds the lock that the Request 2 is waiting to acquire. ` + | ||
| `That's not useful, so PHPRequestHandler will return error 502 instead.` | ||
| ); | ||
| return new PHPResponse( | ||
| 502, | ||
| {}, | ||
| new TextEncoder().encode('502 Bad Gateway') | ||
| ); | ||
| } |
There was a problem hiding this comment.
@adamziel wouldn't it be expected for request two to continue after request one releases the
lock?
There was a problem hiding this comment.
This is for a situation when request 2 is issued by request 1. Then, request 1 can't finish without the response, but the response won't be generated until request 1 releases the lock.
There was a problem hiding this comment.
I will make that comment clearer.
There was a problem hiding this comment.
True, but wouldn't it still be expected for request 2 to process?
Is there a limitation that allows us to run only one request?
There was a problem hiding this comment.
A single PHP instance may only process a single request at a time. PHP is stateful. Internally it uses global variables to keep track of the request details, state, resources etc. To serve multiple concurrent requests, we’d need multiple PHP instances.
Prior to this PR, Playground would apply a series of patches to the WordPress source code to make it compatible with the PHP-Wasm runtime.
This PR removes most of those patches and the entire
applyWordPressPatchesstep.List of changes
wp_new_blog_notificationfunction is no longer overridden as one shipped by WordPress works.PHPRequestHandlernow returns a 502 status code when a request is received while another request is still being handled. Previously it would deadlock as handling a request requires an exclusive lock on the PHP instance. This prevents the Site Health plugin from triggering a deadlock by requesting the WordPress site from inside the PHP code.Testing instructions
cc @bgrgicak