Skip to content

Commit cfe0856

Browse files
authored
[PHP] Dispatch request.error for all non-zero-exit request handler errors (#2429)
## Motivation for the change, related issues The [recent PR](#2357), that altered how error reporting works, unintentionally removed the `request.error` event in some PHP.run() code paths. This PR makes sure the `request.error` event is emitted anytime a non-zero-exit error happens. ## Testing Instructions (or ideally a Blueprint) CI tests
1 parent d5a74b5 commit cfe0856

File tree

2 files changed

+75
-17
lines changed

2 files changed

+75
-17
lines changed

packages/php-wasm/node/src/test/php.spec.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2397,6 +2397,44 @@ phpLoaderOptions.forEach((options) => {
23972397
});
23982398
});
23992399

2400+
describe('Event dispatching', () => {
2401+
it('Should emit a request.error event when PHP.run() exits with a non-zero exit code', async () => {
2402+
const spyListener = vi.fn();
2403+
php.addEventListener('request.error', spyListener);
2404+
try {
2405+
await php.run({
2406+
code: `<?php throw new Error('mock error');`,
2407+
});
2408+
} catch {
2409+
// Ignore the thrown error
2410+
}
2411+
expect(spyListener).toHaveBeenCalledTimes(1);
2412+
expect(spyListener).toHaveBeenCalledWith({
2413+
type: 'request.error',
2414+
error: new Error('PHP.run() failed with exit code 255.'),
2415+
source: 'php-wasm',
2416+
});
2417+
});
2418+
it('Should emit a request.error event when PHP.runStream() exits with a non-zero exit code', async () => {
2419+
const spyListener = vi.fn();
2420+
php.addEventListener('request.error', spyListener);
2421+
try {
2422+
const response = await php.runStream({
2423+
code: `<?php throw new Error('mock error');`,
2424+
});
2425+
await response.finished;
2426+
} catch {
2427+
// Ignore the thrown error
2428+
}
2429+
expect(spyListener).toHaveBeenCalledTimes(1);
2430+
expect(spyListener).toHaveBeenCalledWith({
2431+
type: 'request.error',
2432+
error: new Error('PHP.run() failed with exit code 255.'),
2433+
source: 'php-wasm',
2434+
});
2435+
});
2436+
});
2437+
24002438
/**
24012439
* libsqlite3 path needs to be explicitly provided in Dockerfile
24022440
* for PHP < 7.4 – let's make sure it works

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

Lines changed: 37 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -467,9 +467,11 @@ export class PHP implements Disposable {
467467
);
468468

469469
if (syncResponse.exitCode !== 0) {
470-
// Legacy behavior: throw if PHP exited with a non-zero exit code.
470+
// Legacy run() behavior: throw if PHP exited with a non-zero exit code.
471471
// It could be a WASM crash, but it could be a PHP userland error such
472472
// as "Fatal error: Uncaught Error: Call to undefined function no_such_function()".
473+
//
474+
// runStream() does not throw just because an exitCode is non-zero.
473475
throw new PHPExecutionFailureError(
474476
`PHP.run() failed with exit code ${syncResponse.exitCode}. \n\n=== Stdout ===\n ${syncResponse.text}\n\n=== Stderr ===\n ${syncResponse.errors}`,
475477
syncResponse,
@@ -654,21 +656,6 @@ export class PHP implements Disposable {
654656

655657
// Free up resources when the response is done
656658
await streamedResponsePromise
657-
.catch((error) => {
658-
/**
659-
* Dispatch a request.error event for any global crash handlers. For example,
660-
* Playground web uses this to automatically display a "Report crash" modal.
661-
*/
662-
this.dispatchEvent({
663-
type: 'request.error',
664-
error: error as Error,
665-
// Distinguish between PHP request and PHP-wasm errors
666-
source: (error as any).source ?? 'php-wasm',
667-
});
668-
669-
// Rethrow the error. We don't want to swallow it.
670-
throw error;
671-
})
672659
.finally(() => {
673660
if (heapBodyPointer) {
674661
this[__private__dont__use].free(heapBodyPointer);
@@ -1035,7 +1022,40 @@ export class PHP implements Disposable {
10351022
}
10361023
};
10371024

1038-
const exitCodePromise = runExecutionFunction();
1025+
/**
1026+
* Dispatch a request.error event for any global crash handlers. For example,
1027+
* Playground web uses this to automatically display a "Report crash" modal.
1028+
*/
1029+
const exitCodePromise = runExecutionFunction().then(
1030+
(exitCode) => {
1031+
/**
1032+
* Emit errors related to PHP script failures (exit code other than 0)
1033+
*/
1034+
if (exitCode !== 0) {
1035+
this.dispatchEvent({
1036+
type: 'request.error',
1037+
error: new Error(
1038+
`PHP.run() failed with exit code ${exitCode}.`
1039+
),
1040+
// Distinguish between PHP request and PHP-wasm errors
1041+
source: 'php-wasm',
1042+
});
1043+
}
1044+
return exitCode;
1045+
},
1046+
(error) => {
1047+
/**
1048+
* Emit all other errors.
1049+
*/
1050+
this.dispatchEvent({
1051+
type: 'request.error',
1052+
error: error as any as Error,
1053+
// Distinguish between PHP request and PHP-wasm errors
1054+
source: (error as any).source ?? 'php-wasm',
1055+
});
1056+
throw error;
1057+
}
1058+
);
10391059

10401060
return new StreamedPHPResponse(
10411061
headers.stream,

0 commit comments

Comments
 (0)