PHP: Handle request errors in PHPRequestHandler, return response code…

… 500 (#1249)

Moves the handling of PHP fatal errors from a service worker message
listener, where only the in-browser Playground can use it, to
`PHPRequestHandler`, that allows all the Playground apps like Studio and
`wp-now` to benefit.

In addition, this PR ensures that HTTP 500 Internal Server Error is
returned if PHP exits with a non-zero code. This is required because PHP
itself seems to happily return HTTP 200 in those cases.

## What problem is it solving?

Studio app is showing error logs and is hung under certain scenarios.
This pr fixes that.

## How is the problem addressed?

By catching the error, and returning the error response.

## Testing Instructions

The easiest way I found to test it was via Studio, where the problem
appeared originally.

- Clone the Studio app
- Change `package.json` and replace `@php-wasm` references with the
following:
```		"@php-wasm/node": "file:../wordpress-playground/dist/packages/php-wasm/node",
		"@php-wasm/util": "file:../wordpress-playground/dist/packages/php-wasm/util",
		"@php-wasm/node-polyfills": "file:../wordpress-playground/dist/packages/php-wasm/node-polyfills",
		"@php-wasm/universal": "file:../wordpress-playground/dist/packages/php-wasm/universal",
```
- Change `webpack.main.config.ts` and replace the externals object with
the following:

```	
externals: {
		'@php-wasm/node': '@php-wasm/node',
		'@php-wasm/universal': '@php-wasm/universal',
		'@php-wasm/util': '@php-wasm/util',
		'@php-wasm/node-polyfills': '@php-wasm/node-polyfills',
	},
```
- Clone this pr.
- Run `npm i`
- Run `npx nx reset && npm run build`
- Back to the Studio repo, run `npm i`
- Create a new site.
- Install `WooCommerce`
- Reload the `wp-admin`
- Ensure that the dashboard is showing as expected instead of hanging.

### Run tests manually
1 From the root of the project run: `npx --expose-gc nx run
php-wasm-node:test --testFile
./packages/php-wasm/node/src/test/php-request-handler.spec.ts
--skipNxCache`

---------

Co-authored-by: Adam Zielinski <adam@adamziel.com>

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

@@ -177,6 +177,66 @@ describe.each(SupportedPHPVersions)(
 			});
 		});
 
+		it('should return httpStatus 500 if exit code is not 0', async () => {
+			php.writeFile(
+				'/index.php',
+				`<?php
+				 echo 'Hello World';
+				`
+			);
+			const response1Result = await handler.request({
+				url: '/index.php',
+			});
+			php.writeFile(
+				'/index.php',
+				`<?php
+				echo 'Hello World' // note there is no closing semicolon
+				`
+			);
+			const response2Result = await handler.request({
+				url: '/index.php',
+			});
+			php.writeFile(
+				'/index.php',
+				`<?php
+				 echo 'Hello World!';
+				`
+			);
+			const response3Result = await handler.request({
+				url: '/index.php',
+			});
+			expect(response1Result).toEqual({
+				httpStatusCode: 200,
+				headers: {
+					'content-type': ['text/html; charset=UTF-8'],
+					'x-powered-by': [expect.any(String)],
+				},
+				bytes: new TextEncoder().encode('Hello World'),
+				errors: '',
+				exitCode: 0,
+			});
+			expect(response2Result).toEqual({
+				httpStatusCode: 500,
+				headers: {
+					'content-type': ['text/html; charset=UTF-8'],
+					'x-powered-by': [expect.any(String)],
+				},
+				bytes: expect.any(Uint8Array),
+				errors: expect.any(String),
+				exitCode: 255,
+			});
+			expect(response3Result).toEqual({
+				httpStatusCode: 200,
+				headers: {
+					'content-type': ['text/html; charset=UTF-8'],
+					'x-powered-by': [expect.any(String)],
+				},
+				bytes: new TextEncoder().encode('Hello World!'),
+				errors: '',
+				exitCode: 0,
+			});
+		});
+
 		it('Should accept `body` as a JavaScript object', async () => {
 			/**
 			 * Tests against calling phpwasm_init_uploaded_files_hash() when

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

@@ -33,6 +33,17 @@ const STRING = 'string';
 const NUMBER = 'number';
 
 export const __private__dont__use = Symbol('__private__dont__use');
+
+export class PHPExecutionFailureError extends Error {
+	constructor(
+		message: string,
+		public response: PHPResponse,
+		public source: 'request' | 'php-wasm'
+	) {
+		super(message);
+	}
+}
+
 /**
  * An environment-agnostic wrapper around the Emscripten PHP runtime
  * that universals the super low-level API and provides a more convenient
@@ -278,14 +289,12 @@ export abstract class BasePHP implements IsomorphicLocalPHP {
 			const response = await this.#handleRequest();
 			if (response.exitCode !== 0) {
 				console.warn(`PHP.run() output was:`, response.text);
-				const error = new Error(
+				const error = new PHPExecutionFailureError(
 					`PHP.run() failed with exit code ${response.exitCode} and the following output: ` +
-						response.errors
-				);
-				// @ts-ignore
-				error.response = response;
-				// @ts-ignore
-				error.source = 'request';
+						response.errors,
+					response,
+					'request'
+				) as PHPExecutionFailureError;
 				console.error(error);
 				throw error;
 			}
@@ -661,7 +670,7 @@ export abstract class BasePHP implements IsomorphicLocalPHP {
 
 		const { headers, httpStatusCode } = this.#getResponseHeaders();
 		return new PHPResponse(
-			httpStatusCode,
+			exitCode === 0 ? httpStatusCode : 500,
 			headers,
 			this.readFileAsBuffer('/internal/stdout'),
 			this.readFileAsText('/internal/stderr'),

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

@@ -5,7 +5,11 @@ import {
 	removePathPrefix,
 	DEFAULT_BASE_URL,
 } from './urls';
-import { BasePHP, normalizeHeaders } from './base-php';
+import {
+	BasePHP,
+	PHPExecutionFailureError,
+	normalizeHeaders,
+} from './base-php';
 import { PHPResponse } from './php-response';
 import { PHPRequest, PHPRunOptions, RequestHandler } from './universal-php';
 import { encodeAsMultipart } from './encode-as-multipart';
@@ -242,17 +246,25 @@ export class PHPRequestHandler implements RequestHandler {
 				);
 			}
 
-			return await this.php.run({
-				relativeUri: ensurePathPrefix(
-					toRelativeUrl(requestedUrl),
-					this.#PATHNAME
-				),
-				protocol: this.#PROTOCOL,
-				method: request.method || preferredMethod,
-				body,
-				scriptPath,
-				headers,
-			});
+			try {
+				return await this.php.run({
+					relativeUri: ensurePathPrefix(
+						toRelativeUrl(requestedUrl),
+						this.#PATHNAME
+					),
+					protocol: this.#PROTOCOL,
+					method: request.method || preferredMethod,
+					body,
+					scriptPath,
+					headers,
+				});
+			} catch (error) {
+				const executionError = error as PHPExecutionFailureError;
+				if (executionError?.response) {
+					return executionError.response;
+				}
+				throw error;
+			}
 		} finally {
 			release();
 		}

packages/php-wasm/web/src/lib/api.ts

@@ -160,8 +160,8 @@ function setupTransferHandlers() {
 		},
 	});
 	// Augment Comlink's throw handler to include Error the response and source
-	// information in the serialized error object. BasePHP throws may include
-	// those information and we'll want to display them for the user.
+	// information in the serialized error object. BasePHP may throw PHPExecutionFailureError
+	// which includes those information and we'll want to display them for the user.
 	const throwHandler = Comlink.transferHandlers.get('throw')!;
 	const originalSerialize = throwHandler?.serialize;
 	throwHandler.serialize = ({ value }: any) => {

packages/php-wasm/web/src/lib/register-service-worker.ts

@@ -2,7 +2,6 @@ import { PhpWasmError } from '@php-wasm/util';
 import type { WebPHPEndpoint } from './web-php-endpoint';
 import { responseTo } from '@php-wasm/web-service-worker';
 import { Remote } from 'comlink';
-import { PHPResponse } from '@php-wasm/universal';
 
 /**
  * Run this in the main application to register the service worker or
@@ -63,21 +62,7 @@ export async function registerServiceWorker<
 			const args = event.data.args || [];
 
 			const method = event.data.method as keyof Client;
-			let result;
-			try {
-				result = await (phpApi[method] as Function)(...args);
-			} catch (e) {
-				if (
-					method === 'request' &&
-					e &&
-					typeof e === 'object' &&
-					'response' in e
-				) {
-					result = e.response as PHPResponse;
-				} else {
-					throw e;
-				}
-			}
+			const result = await (phpApi[method] as Function)(...args);
 			event.source!.postMessage(responseTo(event.data.requestId, result));
 		}
 	);