From 4a3be7c3ea42400720a672f363a96770bb05c050 Mon Sep 17 00:00:00 2001 From: Antony Agrios Date: Tue, 16 Apr 2024 12:16:57 +0300 Subject: [PATCH 1/6] Fix: handle request error gracefully --- .../universal/src/lib/php-request-handler.ts | 32 ++++++++++++------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/packages/php-wasm/universal/src/lib/php-request-handler.ts b/packages/php-wasm/universal/src/lib/php-request-handler.ts index 3ced1a94b8..776c2ff0d7 100644 --- a/packages/php-wasm/universal/src/lib/php-request-handler.ts +++ b/packages/php-wasm/universal/src/lib/php-request-handler.ts @@ -242,17 +242,27 @@ 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) { + return ( + error as { + message: string; + response: PHPResponse; + source: string; + } + ).response; + } } finally { release(); } From 33c36cb4a86638af0103663cf20ca4de231f2693 Mon Sep 17 00:00:00 2001 From: Antony Agrios Date: Tue, 16 Apr 2024 14:41:55 +0300 Subject: [PATCH 2/6] Update: Create interface for better type handling --- .../php-wasm/universal/src/lib/base-php.ts | 6 ++++++ .../universal/src/lib/php-request-handler.ts | 18 ++++++++++-------- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/packages/php-wasm/universal/src/lib/base-php.ts b/packages/php-wasm/universal/src/lib/base-php.ts index d3cde60cdc..812cea3395 100644 --- a/packages/php-wasm/universal/src/lib/base-php.ts +++ b/packages/php-wasm/universal/src/lib/base-php.ts @@ -33,6 +33,12 @@ const STRING = 'string'; const NUMBER = 'number'; export const __private__dont__use = Symbol('__private__dont__use'); + +export interface PHPExecutionFailureError extends Error { + response: PHPResponse; + source: 'request' | 'php-wasm'; +} + /** * An environment-agnostic wrapper around the Emscripten PHP runtime * that universals the super low-level API and provides a more convenient diff --git a/packages/php-wasm/universal/src/lib/php-request-handler.ts b/packages/php-wasm/universal/src/lib/php-request-handler.ts index 776c2ff0d7..a765b887b4 100644 --- a/packages/php-wasm/universal/src/lib/php-request-handler.ts +++ b/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'; @@ -255,13 +259,11 @@ export class PHPRequestHandler implements RequestHandler { headers, }); } catch (error) { - return ( - error as { - message: string; - response: PHPResponse; - source: string; - } - ).response; + const executionError = error as PHPExecutionFailureError; + if (executionError?.response) { + return executionError.response; + } + throw error; } } finally { release(); From 8eb54468aa6cbc54c8116650e6047e43126b0988 Mon Sep 17 00:00:00 2001 From: Antony Agrios Date: Tue, 16 Apr 2024 14:42:33 +0300 Subject: [PATCH 3/6] Update: In case of non zero exit code, mark response as 500 --- packages/php-wasm/universal/src/lib/base-php.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/php-wasm/universal/src/lib/base-php.ts b/packages/php-wasm/universal/src/lib/base-php.ts index 812cea3395..32e15b371f 100644 --- a/packages/php-wasm/universal/src/lib/base-php.ts +++ b/packages/php-wasm/universal/src/lib/base-php.ts @@ -667,7 +667,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'), From 10016d8a7249bb28a3ccd78725867d6b4ebcfc05 Mon Sep 17 00:00:00 2001 From: Antony Agrios Date: Wed, 17 Apr 2024 11:57:06 +0300 Subject: [PATCH 4/6] Update: Add tests --- .../node/src/test/php-request-handler.spec.ts | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/packages/php-wasm/node/src/test/php-request-handler.spec.ts b/packages/php-wasm/node/src/test/php-request-handler.spec.ts index e5358aea3b..dece3eef27 100644 --- a/packages/php-wasm/node/src/test/php-request-handler.spec.ts +++ b/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', + ` { /** * Tests against calling phpwasm_init_uploaded_files_hash() when From 1b3383a0a9db561383f113e07b144003844dbb8a Mon Sep 17 00:00:00 2001 From: Adam Zielinski Date: Wed, 17 Apr 2024 11:28:20 +0200 Subject: [PATCH 5/6] adjust the test example --- packages/php-wasm/node/src/test/php-request-handler.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/php-wasm/node/src/test/php-request-handler.spec.ts b/packages/php-wasm/node/src/test/php-request-handler.spec.ts index dece3eef27..da46e81f62 100644 --- a/packages/php-wasm/node/src/test/php-request-handler.spec.ts +++ b/packages/php-wasm/node/src/test/php-request-handler.spec.ts @@ -190,7 +190,7 @@ describe.each(SupportedPHPVersions)( php.writeFile( '/index.php', ` Date: Wed, 17 Apr 2024 11:38:23 +0200 Subject: [PATCH 6/6] Make PHPExecutionFailureError a class, remove an extra error handler from the service worker message listener --- .../php-wasm/universal/src/lib/base-php.ts | 23 +++++++++++-------- packages/php-wasm/web/src/lib/api.ts | 4 ++-- .../web/src/lib/register-service-worker.ts | 17 +------------- 3 files changed, 16 insertions(+), 28 deletions(-) diff --git a/packages/php-wasm/universal/src/lib/base-php.ts b/packages/php-wasm/universal/src/lib/base-php.ts index 32e15b371f..e5632bdd22 100644 --- a/packages/php-wasm/universal/src/lib/base-php.ts +++ b/packages/php-wasm/universal/src/lib/base-php.ts @@ -34,9 +34,14 @@ const NUMBER = 'number'; export const __private__dont__use = Symbol('__private__dont__use'); -export interface PHPExecutionFailureError extends Error { - response: PHPResponse; - source: 'request' | 'php-wasm'; +export class PHPExecutionFailureError extends Error { + constructor( + message: string, + public response: PHPResponse, + public source: 'request' | 'php-wasm' + ) { + super(message); + } } /** @@ -284,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; } diff --git a/packages/php-wasm/web/src/lib/api.ts b/packages/php-wasm/web/src/lib/api.ts index 3dd0d89798..836b270259 100644 --- a/packages/php-wasm/web/src/lib/api.ts +++ b/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) => { diff --git a/packages/php-wasm/web/src/lib/register-service-worker.ts b/packages/php-wasm/web/src/lib/register-service-worker.ts index d7a5b281ed..2fa3cb0610 100644 --- a/packages/php-wasm/web/src/lib/register-service-worker.ts +++ b/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)); } );