Skip to content

Commit cf36ff3

Browse files
[CLI] Avoid breaks during Playground boot when --xdebug enabled (#2835)
## Motivation for the change, related issues Today, Playground enables Xdebug during boot which may be surprising to users. If the debugger breaks at the first line, the user will be debugging the Playground boot process. Let's skip this by default and consider adding a command line flag to enable boot-time Xdebug breaks. ## Implementation details A big change in this PR is that the first Playground worker is now only used during boot. We discard it after that boot and setup process completes. Some positive consequences of this change are: - Reflects the truth that the Playground worker has special configuration and responsibilities as part of the boot process. - Allows us to be clear that all post-boot workers are based on the same configuration and truth. There is no remaining special boot-time worker that might behave differently from the others. - Allows us to control whether the boot-and-setup worker has Xdebug enabled separately from the regular workers. ## Testing Instructions (or ideally a Blueprint) Before checking out this branch: - Create a local directory at the top level of your Playground working dir that can be mounted as `/wordpress` - Run `npx nx unbuilt-asyncify playground-cli -- server --mount-dir <local-dir> /wordpress` so Playground will download and install WordPress in your local directory. - Open the local directory as a project root in Visual Studio Code, select the debug pane, and create a launch.json file containing the following configuration: ``` { "name": "PR test for WP Playground CLI - Listen for Xdebug", "type": "php", "request": "launch", "port": 9003, "pathMappings": { "/": "${workspaceFolder}/.playground-xdebug-root", "/wordpress": "${workspaceFolder}/" }, "stopOnEntry": true, } ``` - Start the "PR test for WP Playground CLI - Listen for Xdebug" debug target within vscode - `cd` into the local WP dir and run the following command and confirm that vscode hit a breakpoint before the "WordPress is running on http://127.0.0.1:9400" message is printed: `node --experimental-strip-types --experimental-transform-types --disable-warning=ExperimentalWarning --import ../packages/meta/src/node-es-module-loader/register.mts ../packages/playground/cli/src/cli.ts server --auto-mount=. --verbosity=debug --skip-wordpress-setup --xdebug --experimental-unsafe-ide-integration --php=8.3 --experimental-multi-worker=4` After checking out this branch: - Open the previously created local WP dir as the workspace root within vscode - Run the "PR test for WP Playground CLI - Listen for Xdebug" debug target - `cd` into the local WP dir, re-run the above node command, and confirm that no breakpoint was hit during the boot process. --------- Co-authored-by: Adam Zieliński <adam@adamziel.com>
1 parent 4f5d8e2 commit cf36ff3

File tree

7 files changed

+140
-107
lines changed

7 files changed

+140
-107
lines changed

packages/playground/cli/src/blueprints-v1/blueprints-v1-handler.ts

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import { logger } from '@php-wasm/logger';
22
import { EmscriptenDownloadMonitor, ProgressTracker } from '@php-wasm/progress';
3-
import type { SupportedPHPVersion } from '@php-wasm/universal';
43
import { consumeAPI } from '@php-wasm/universal';
54
import type { BlueprintV1Declaration } from '@wp-playground/blueprints';
65
import {
@@ -34,7 +33,6 @@ import {
3433
* implemented in TypeScript and orchestrated by this class.
3534
*/
3635
export class BlueprintsV1Handler {
37-
private phpVersion: SupportedPHPVersion | undefined;
3836
private lastProgressMessage = '';
3937

4038
private siteUrl: string;
@@ -57,7 +55,7 @@ export class BlueprintsV1Handler {
5755
return 'v1';
5856
}
5957

60-
async bootPrimaryWorker(
58+
async bootAndSetUpInitialPlayground(
6159
phpPort: NodeMessagePort,
6260
fileLockManagerPort: NodeMessagePort,
6361
nativeInternalDirPath: string
@@ -135,7 +133,7 @@ export class BlueprintsV1Handler {
135133
this.getEffectiveBlueprint()
136134
);
137135
await playground.useFileLockManager(fileLockManagerPort);
138-
await playground.bootAsPrimaryWorker({
136+
await playground.bootAndSetUpInitialWorker({
139137
phpVersion: runtimeConfiguration.phpVersion,
140138
wpVersion: runtimeConfiguration.wpVersion,
141139
siteUrl: this.siteUrl,
@@ -149,7 +147,11 @@ export class BlueprintsV1Handler {
149147
followSymlinks,
150148
trace,
151149
internalCookieStore: this.args.internalCookieStore,
152-
withXdebug: !!this.args.xdebug,
150+
// We do not enable Xdebug by default for the initial worker
151+
// because we do not imagine users expect to hit breakpoints
152+
// until Playground has fully booted.
153+
// TODO: Consider supporting Xdebug for the initial worker via a dedicated flag.
154+
withXdebug: false,
153155
nativeInternalDirPath,
154156
});
155157

@@ -169,7 +171,7 @@ export class BlueprintsV1Handler {
169171
return playground;
170172
}
171173

172-
async bootSecondaryWorker({
174+
async bootPlayground({
173175
worker,
174176
fileLockManagerPort,
175177
firstProcessId,
@@ -180,14 +182,17 @@ export class BlueprintsV1Handler {
180182
firstProcessId: number;
181183
nativeInternalDirPath: string;
182184
}) {
183-
const additionalPlayground = consumeAPI<PlaygroundCliBlueprintV1Worker>(
185+
const playground = consumeAPI<PlaygroundCliBlueprintV1Worker>(
184186
worker.phpPort
185187
);
186188

187-
await additionalPlayground.isConnected();
188-
await additionalPlayground.useFileLockManager(fileLockManagerPort);
189-
await additionalPlayground.bootAsSecondaryWorker({
190-
phpVersion: this.phpVersion!,
189+
await playground.isConnected();
190+
const runtimeConfiguration = await resolveRuntimeConfiguration(
191+
this.getEffectiveBlueprint()
192+
);
193+
await playground.useFileLockManager(fileLockManagerPort);
194+
await playground.bootWorker({
195+
phpVersion: runtimeConfiguration.phpVersion,
191196
siteUrl: this.siteUrl,
192197
mountsBeforeWpInstall: this.args['mount-before-install'] || [],
193198
mountsAfterWpInstall: this.args['mount'] || [],
@@ -201,8 +206,8 @@ export class BlueprintsV1Handler {
201206
withXdebug: !!this.args.xdebug,
202207
nativeInternalDirPath,
203208
});
204-
await additionalPlayground.isReady();
205-
return additionalPlayground;
209+
await playground.isReady();
210+
return playground;
206211
}
207212

208213
async compileInputBlueprint(additionalBlueprintSteps: any[]) {

packages/playground/cli/src/blueprints-v1/worker-thread-v1.ts

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,14 +56,15 @@ export type PrimaryWorkerBootOptions = WorkerBootOptions & {
5656

5757
interface WorkerBootRequestHandlerOptions {
5858
siteUrl: string;
59-
allow?: string;
59+
followSymlinks: boolean;
6060
phpVersion: SupportedPHPVersion;
6161
firstProcessId: number;
6262
processIdSpaceLength: number;
6363
trace: boolean;
6464
nativeInternalDirPath: string;
6565
mountsBeforeWpInstall: Array<Mount>;
6666
mountsAfterWpInstall: Array<Mount>;
67+
withXdebug?: boolean;
6768
}
6869

6970
/**
@@ -123,7 +124,7 @@ export class PlaygroundCliBlueprintV1Worker extends PHPWorker {
123124
}
124125
}
125126

126-
async bootAsPrimaryWorker({
127+
async bootAndSetUpInitialWorker({
127128
siteUrl,
128129
mountsBeforeWpInstall,
129130
mountsAfterWpInstall,
@@ -229,20 +230,21 @@ export class PlaygroundCliBlueprintV1Worker extends PHPWorker {
229230
}
230231
}
231232

232-
async bootAsSecondaryWorker(args: WorkerBootOptions) {
233+
async bootWorker(args: WorkerBootOptions) {
233234
await this.bootRequestHandler(args);
234235
}
235236

236237
async bootRequestHandler({
237238
siteUrl,
238-
allow,
239+
followSymlinks,
239240
phpVersion,
240241
firstProcessId,
241242
processIdSpaceLength,
242243
trace,
243244
nativeInternalDirPath,
244245
mountsBeforeWpInstall,
245246
mountsAfterWpInstall,
247+
withXdebug,
246248
}: WorkerBootRequestHandlerOptions) {
247249
if (this.booted) {
248250
throw new Error('Playground already booted');
@@ -275,7 +277,8 @@ export class PlaygroundCliBlueprintV1Worker extends PHPWorker {
275277
},
276278
phpWasmInitOptions: { nativeInternalDirPath },
277279
},
278-
followSymlinks: allow?.includes('follow-symlinks'),
280+
followSymlinks,
281+
withXdebug,
279282
});
280283
},
281284
onPHPInstanceCreated: async (php) => {

packages/playground/cli/src/blueprints-v2/blueprints-v2-handler.ts

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ export class BlueprintsV2Handler {
3838
return 'v2';
3939
}
4040

41-
async bootPrimaryWorker(
41+
async bootAndSetUpInitialPlayground(
4242
phpPort: NodeMessagePort,
4343
fileLockManagerPort: NodeMessagePort,
4444
nativeInternalDirPath: string
@@ -56,19 +56,20 @@ export class BlueprintsV2Handler {
5656
processIdSpaceLength: this.processIdSpaceLength,
5757
trace: this.args.debug || false,
5858
blueprint: this.args.blueprint!,
59-
withXdebug: !!this.args.xdebug,
60-
xdebug:
61-
typeof this.args.xdebug === 'object'
62-
? this.args.xdebug
63-
: undefined,
59+
// We do not enable Xdebug by default for the initial worker
60+
// because we do not imagine users expect to hit breakpoints
61+
// until Playground has fully booted.
62+
// TODO: Consider supporting Xdebug for the initial worker via a dedicated flag.
63+
withXdebug: false,
64+
xdebug: undefined,
6465
nativeInternalDirPath,
6566
};
6667

67-
await playground.bootAsPrimaryWorker(workerBootArgs);
68+
await playground.bootAndSetUpInitialWorker(workerBootArgs);
6869
return playground;
6970
}
7071

71-
async bootSecondaryWorker({
72+
async bootPlayground({
7273
worker,
7374
fileLockManagerPort,
7475
firstProcessId,
@@ -86,7 +87,7 @@ export class BlueprintsV2Handler {
8687

8788
const workerBootArgs: SecondaryWorkerBootArgs = {
8889
...this.args,
89-
phpVersion: this.phpVersion!,
90+
phpVersion: this.phpVersion,
9091
siteUrl: this.siteUrl,
9192
firstProcessId,
9293
processIdSpaceLength: this.processIdSpaceLength,
@@ -97,7 +98,7 @@ export class BlueprintsV2Handler {
9798
mountsAfterWpInstall: this.args.mount || [],
9899
};
99100

100-
await playground.bootAsSecondaryWorker(workerBootArgs);
101+
await playground.bootWorker(workerBootArgs);
101102

102103
return playground;
103104
}

packages/playground/cli/src/blueprints-v2/worker-thread-v2.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ export class PlaygroundCliBlueprintV2Worker extends PHPWorker {
204204
}
205205
}
206206

207-
async bootAsPrimaryWorker(args: PrimaryWorkerBootArgs) {
207+
async bootAndSetUpInitialWorker(args: PrimaryWorkerBootArgs) {
208208
const constants = {
209209
WP_DEBUG: true,
210210
WP_DEBUG_LOG: true,
@@ -252,7 +252,7 @@ export class PlaygroundCliBlueprintV2Worker extends PHPWorker {
252252
await this.runBlueprintV2(args);
253253
}
254254

255-
async bootAsSecondaryWorker(args: SecondaryWorkerBootArgs) {
255+
async bootWorker(args: SecondaryWorkerBootArgs) {
256256
await this.bootRequestHandler({
257257
...args,
258258
onPHPInstanceCreated: async (php: PHP) => {

packages/playground/cli/src/load-balancer.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,20 @@ export class LoadBalancer {
3333
activeRequests: new Set(),
3434
});
3535
}
36+
async removeWorker(worker: RemoteAPI<PlaygroundCliWorker>) {
37+
const workerIndex = this.workerLoads.findIndex(
38+
(workerLoad) => workerLoad.worker === worker
39+
);
40+
if (workerIndex === -1) {
41+
return;
42+
}
43+
44+
const [removedWorker] = this.workerLoads.splice(workerIndex, 1);
45+
46+
// A worker can only be considered fully removed once all
47+
// its active requests have settled.
48+
await Promise.allSettled(removedWorker.activeRequests);
49+
}
3650

3751
async handleRequest(request: PHPRequest) {
3852
let smallestWorkerLoad = this.workerLoads[0];

0 commit comments

Comments
 (0)