Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PHP: Handle request errors in PHPRequestHandler, return response code 500 #1249

Merged
merged 10 commits into from
Apr 17, 2024
60 changes: 60 additions & 0 deletions packages/php-wasm/node/src/test/php-request-handler.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
25 changes: 17 additions & 8 deletions packages/php-wasm/universal/src/lib/base-php.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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'),
Expand Down
36 changes: 24 additions & 12 deletions packages/php-wasm/universal/src/lib/php-request-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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();
}
Expand Down
4 changes: 2 additions & 2 deletions packages/php-wasm/web/src/lib/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
17 changes: 1 addition & 16 deletions packages/php-wasm/web/src/lib/register-service-worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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));
}
);
Expand Down
Loading