Skip to content

Commit e170be1

Browse files
adamzieladamziel
committed
Fix PHP worker request handler recovery
1 parent 9b11ae0 commit e170be1

5 files changed

Lines changed: 178 additions & 35 deletions

File tree

‎packages/php-wasm/universal/src/lib/api.spec.ts‎

Lines changed: 0 additions & 18 deletions
This file was deleted.

‎packages/php-wasm/universal/src/lib/api.ts‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -199,11 +199,7 @@ function prepareForExpose<Methods, PipedAPI>(
199199
} else if (prop in target) {
200200
return target[prop];
201201
}
202-
const pipedValue = (pipedApi as any)?.[prop];
203-
if (typeof pipedValue === 'function') {
204-
return (...args: any[]) => (pipedApi as any)[prop](...args);
205-
}
206-
return pipedValue;
202+
return (pipedApi as any)?.[prop];
207203
},
208204
}) as unknown as PublicAPI<Methods, PipedAPI>;
209205

‎packages/php-wasm/universal/src/lib/php-worker.spec.ts‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { PHPWorker } from './php-worker';
22
import { describe, expect, test, vi } from 'vitest';
33
import type { PHP } from './php';
4+
import type { PHPRequestHandler } from './php-request-handler';
45

56
type PhpEvent = { type: string; [key: string]: unknown };
67
type PhpEventListener = (event: PhpEvent) => void | Promise<void>;
@@ -51,6 +52,20 @@ class TestEndpoint extends PHPWorker {
5152
async boot(_: unknown = undefined) {}
5253
}
5354

55+
class EndpointWithoutRequestHandler extends TestEndpoint {
56+
protected override getRequestHandler(required?: true): PHPRequestHandler;
57+
protected override getRequestHandler(
58+
required: false
59+
): PHPRequestHandler | undefined;
60+
protected override getRequestHandler(
61+
_required = true
62+
): PHPRequestHandler | undefined {
63+
throw new Error(
64+
'request handler lookup should not run before primary PHP fallback'
65+
);
66+
}
67+
}
68+
5469
describe('PlaygroundWorkerEndpoint', () => {
5570
test('listeners receive events from each PHP instance', async () => {
5671
const endpoint = new TestEndpoint();
@@ -81,4 +96,91 @@ describe('PlaygroundWorkerEndpoint', () => {
8196
expect.any(Function)
8297
);
8398
});
99+
100+
test('recovers request handler from the primary PHP instance', async () => {
101+
const endpoint = new TestEndpoint();
102+
const response = {
103+
finished: Promise.resolve(),
104+
};
105+
const cliPhp = {
106+
chdir: vi.fn(),
107+
cli: vi.fn().mockResolvedValue(response),
108+
addEventListener: vi.fn(),
109+
onMessage: vi.fn(),
110+
};
111+
const requestHandler = {
112+
absoluteUrl: 'http://127.0.0.1/',
113+
documentRoot: '/wordpress',
114+
instanceManager: {
115+
acquirePHPInstance: vi.fn().mockResolvedValue({
116+
php: cliPhp,
117+
reap: vi.fn(),
118+
}),
119+
},
120+
};
121+
const primaryPhp = {
122+
...createMockPHP(),
123+
requestHandler,
124+
};
125+
126+
await endpoint.setPrimaryPHP(primaryPhp as unknown as PHP);
127+
await endpoint.cli(['php', '/tmp/script.php']);
128+
129+
expect(
130+
requestHandler.instanceManager.acquirePHPInstance
131+
).toHaveBeenCalled();
132+
expect(cliPhp.cli).toHaveBeenCalledWith(
133+
['php', '/tmp/script.php'],
134+
undefined
135+
);
136+
});
137+
138+
test('runs CLI on the primary PHP instance without a request handler', async () => {
139+
const endpoint = new TestEndpoint();
140+
const response = {
141+
finished: Promise.resolve(),
142+
};
143+
const primaryPhp = {
144+
...createMockPHP(),
145+
cli: vi.fn().mockResolvedValue(response),
146+
};
147+
148+
await endpoint.setPrimaryPHP(primaryPhp as unknown as PHP);
149+
const actualResponse = await endpoint.cli(['php', '/tmp/script.php']);
150+
151+
expect(actualResponse).toBe(response);
152+
expect(primaryPhp.cli).toHaveBeenCalledWith(
153+
['php', '/tmp/script.php'],
154+
undefined
155+
);
156+
});
157+
158+
test('uses the primary PHP instance before resolving a missing request handler', async () => {
159+
const endpoint = new EndpointWithoutRequestHandler();
160+
const cliResponse = {
161+
finished: Promise.resolve(),
162+
};
163+
const runResponse = {};
164+
const primaryPhp = {
165+
...createMockPHP(),
166+
cli: vi.fn().mockResolvedValue(cliResponse),
167+
run: vi.fn().mockResolvedValue(runResponse),
168+
};
169+
170+
await endpoint.setPrimaryPHP(primaryPhp as unknown as PHP);
171+
172+
await expect(endpoint.cli(['php', '/tmp/script.php'])).resolves.toBe(
173+
cliResponse
174+
);
175+
await expect(endpoint.run({ code: "<?php echo 'hi!';" })).resolves.toBe(
176+
runResponse
177+
);
178+
expect(primaryPhp.cli).toHaveBeenCalledWith(
179+
['php', '/tmp/script.php'],
180+
undefined
181+
);
182+
expect(primaryPhp.run).toHaveBeenCalledWith({
183+
code: "<?php echo 'hi!';",
184+
});
185+
});
84186
});

‎packages/php-wasm/universal/src/lib/php-worker.ts‎

Lines changed: 43 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ export class PHPWorker implements LimitedPHPApi, AsyncDisposable {
129129
* a warning.
130130
*/
131131
protected __internal_getRequestHandler() {
132-
return _private.get(this)!.requestHandler;
132+
return this.getRequestHandler();
133133
}
134134

135135
async setPrimaryPHP(php: PHP) {
@@ -141,14 +141,12 @@ export class PHPWorker implements LimitedPHPApi, AsyncDisposable {
141141

142142
/** @inheritDoc @php-wasm/universal!PHPRequestHandler.pathToInternalUrl */
143143
pathToInternalUrl(path: string): string {
144-
return _private.get(this)!.requestHandler!.pathToInternalUrl(path);
144+
return this.getRequestHandler().pathToInternalUrl(path);
145145
}
146146

147147
/** @inheritDoc @php-wasm/universal!PHPRequestHandler.internalUrlToPath */
148148
internalUrlToPath(internalUrl: string): string {
149-
return _private
150-
.get(this)!
151-
.requestHandler!.internalUrlToPath(internalUrl);
149+
return this.getRequestHandler().internalUrlToPath(internalUrl);
152150
}
153151

154152
/**
@@ -174,7 +172,7 @@ export class PHPWorker implements LimitedPHPApi, AsyncDisposable {
174172

175173
/** @inheritDoc @php-wasm/universal!PHPRequestHandler.request */
176174
async request(request: PHPRequest): Promise<PHPResponse> {
177-
const requestHandler = _private.get(this)!.requestHandler!;
175+
const requestHandler = this.getRequestHandler();
178176
return await requestHandler.request(request);
179177
}
180178

@@ -189,12 +187,21 @@ export class PHPWorker implements LimitedPHPApi, AsyncDisposable {
189187
* @param request - PHP Request data.
190188
*/
191189
async requestStreamed(request: PHPRequest): Promise<StreamedPHPResponse> {
192-
const requestHandler = _private.get(this)!.requestHandler!;
190+
const requestHandler = this.getRequestHandler();
193191
return await requestHandler.requestStreamed(request);
194192
}
195193

196194
/** @inheritDoc @php-wasm/universal!/PHP.run */
197195
async run(request: PHPRunOptions): Promise<PHPResponse> {
196+
const state = _private.get(this)!;
197+
const primaryPhp = state.php;
198+
if (
199+
!state.requestHandler &&
200+
!primaryPhp?.requestHandler &&
201+
primaryPhp
202+
) {
203+
return await primaryPhp.run(request);
204+
}
198205
const { php, reap } = await this.acquirePHPInstance();
199206
try {
200207
return await php.run(request);
@@ -208,6 +215,15 @@ export class PHPWorker implements LimitedPHPApi, AsyncDisposable {
208215
argv: string[],
209216
options?: { env?: Record<string, string> }
210217
): Promise<StreamedPHPResponse> {
218+
const state = _private.get(this)!;
219+
const primaryPhp = state.php;
220+
if (
221+
!state.requestHandler &&
222+
!primaryPhp?.requestHandler &&
223+
primaryPhp
224+
) {
225+
return await primaryPhp.cli(argv, options);
226+
}
211227
const { php, reap } = await this.acquirePHPInstance();
212228
let response: StreamedPHPResponse;
213229
try {
@@ -244,9 +260,8 @@ export class PHPWorker implements LimitedPHPApi, AsyncDisposable {
244260
* @returns A PHP instance with a consistent chroot.
245261
*/
246262
private async acquirePHPInstance() {
247-
const { php, reap } = await _private
248-
.get(this)!
249-
.requestHandler!.instanceManager.acquirePHPInstance();
263+
const { php, reap } =
264+
await this.getRequestHandler().instanceManager.acquirePHPInstance();
250265
if (this.chroot !== null) {
251266
php.chdir(this.chroot);
252267
}
@@ -373,6 +388,23 @@ export class PHPWorker implements LimitedPHPApi, AsyncDisposable {
373388
}
374389

375390
async [Symbol.asyncDispose]() {
376-
await _private.get(this)!.requestHandler?.[Symbol.asyncDispose]();
391+
await this.getRequestHandler(false)?.[Symbol.asyncDispose]();
392+
}
393+
394+
protected getRequestHandler(required?: true): PHPRequestHandler;
395+
protected getRequestHandler(required: false): PHPRequestHandler | undefined;
396+
protected getRequestHandler(required = true) {
397+
const state = _private.get(this)!;
398+
if (state.requestHandler) {
399+
return state.requestHandler;
400+
}
401+
if (state.php?.requestHandler) {
402+
this.__internal_setRequestHandler(state.php.requestHandler);
403+
return state.php.requestHandler;
404+
}
405+
if (!required) {
406+
return undefined;
407+
}
408+
throw new Error('PHPWorker is not connected to a request handler.');
377409
}
378410
}

‎packages/playground/remote/src/lib/playground-worker-endpoint.ts‎

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,12 @@ import transportFetch from './playground-mu-plugin/playground-includes/wp_http_f
3030
/* @ts-ignore */
3131
import transportDummy from './playground-mu-plugin/playground-includes/wp_http_dummy.php?raw';
3232
import { logger } from '@php-wasm/logger';
33-
import type { AllPHPVersion, PathAlias, PHP } from '@php-wasm/universal';
33+
import type {
34+
AllPHPVersion,
35+
PathAlias,
36+
PHP,
37+
PHPRequestHandler,
38+
} from '@php-wasm/universal';
3439
import {
3540
isLegacyPHPVersion,
3641
PHPResponse,
@@ -55,6 +60,8 @@ import playgroundWebMuPlugin from './playground-mu-plugin/0-playground.php?raw';
5560
import playgroundWebMuPluginPhp52 from './playground-mu-plugin/0-playground-php52.php?raw';
5661
import { WordPressFetchNetworkTransport } from './wordpress-fetch-network-transport';
5762

63+
let activeRequestHandler: PHPRequestHandler | undefined;
64+
5865
export interface MountDescriptor {
5966
mountpoint: string;
6067
device: MountDevice;
@@ -115,6 +122,7 @@ export abstract class PlaygroundWorkerEndpoint extends PHPWorker {
115122
createNullPrototypeRecord();
116123

117124
private networkTransport: WordPressFetchNetworkTransport | undefined;
125+
private requestHandler: PHPRequestHandler | undefined;
118126

119127
protected downloadMonitor: EmscriptenDownloadMonitor;
120128
protected memoizedFetch: ReturnType<typeof createMemoizedFetch>;
@@ -313,8 +321,11 @@ export abstract class PlaygroundWorkerEndpoint extends PHPWorker {
313321
});
314322

315323
const primaryPhp = await requestHandler.getPrimaryPhp();
324+
primaryPhp.requestHandler ??= requestHandler;
316325
await this.setPrimaryPHP(primaryPhp);
317326
this.__internal_setRequestHandler(requestHandler);
327+
this.requestHandler = requestHandler;
328+
activeRequestHandler = requestHandler;
318329
return requestHandler;
319330
}
320331

@@ -324,6 +335,9 @@ export abstract class PlaygroundWorkerEndpoint extends PHPWorker {
324335
knownRemoteAssetPaths: Set<string>
325336
) {
326337
const primaryPhp = await requestHandler.getPrimaryPhp();
338+
primaryPhp.requestHandler ??= requestHandler;
339+
this.requestHandler = requestHandler;
340+
activeRequestHandler = requestHandler;
327341

328342
if (withNetworking) {
329343
/**
@@ -386,6 +400,23 @@ export abstract class PlaygroundWorkerEndpoint extends PHPWorker {
386400
this.__internal_setRequestHandler(requestHandler);
387401
}
388402

403+
protected override getRequestHandler(required?: true): PHPRequestHandler;
404+
protected override getRequestHandler(
405+
required: false
406+
): PHPRequestHandler | undefined;
407+
protected override getRequestHandler(required = true) {
408+
const requestHandler =
409+
super.getRequestHandler(false) ??
410+
this.requestHandler ??
411+
activeRequestHandler;
412+
if (requestHandler || !required) {
413+
return requestHandler;
414+
}
415+
throw new Error(
416+
'Playground worker is not connected to a request handler.'
417+
);
418+
}
419+
389420
// NOTE: Version-specific boot methods are implemented in the concrete worker entrypoints
390421

391422
/**

0 commit comments

Comments
 (0)