From 4669959eb930cab00badfc364cda6368d53dd227 Mon Sep 17 00:00:00 2001 From: MHO <17177411+mho22@users.noreply.github.com> Date: Fri, 12 Jan 2024 10:49:23 +0100 Subject: [PATCH] Quit callback overwrites the quit callback template (#937) By default, even if you add a `quit` method in `emscryptenOptions` : ``` const php = await NodePHP.load( '8.2', { emscriptenOptions : { quit : ( status, toThrow ) => { if( status === 0 ){ process.exit() } return toThrow } } } ); ``` it is overwritten by the actual one on line `57` in `node-php.ts` : ``` quit: function (code, error) { throw error; }, ``` This PR is moving this template callback to be overwritten by `emscryptenOptions` object given by user. ## What problem is it solving? To avoid throwing error from `php.exit()` even if it is exiting quietly. ## Testing Instructions ``` const php = await NodePHP.load( '8.2', emscriptenOptions : { quit : ( status, toThrow ) => { if( status === 0 ){ process.exit() } return toThrow } } } ); php.exit(0); ``` This should display nothing. --- .../compile/php/phpwasm-emscripten-library.js | 1 - packages/php-wasm/node/src/lib/node-php.ts | 2 +- packages/php-wasm/node/src/test/php.spec.ts | 21 +++++++++++++++---- 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/packages/php-wasm/compile/php/phpwasm-emscripten-library.js b/packages/php-wasm/compile/php/phpwasm-emscripten-library.js index 7856f21fd6..bc58843d77 100644 --- a/packages/php-wasm/compile/php/phpwasm-emscripten-library.js +++ b/packages/php-wasm/compile/php/phpwasm-emscripten-library.js @@ -345,7 +345,6 @@ const LibraryExample = { // Pass data from child process's stderr to PHP's end of the stdout pipe. const stderrStream = SYSCALLS.getStreamFromFD(stderrChildFd); cp.stderr.on('data', function (data) { - console.log('Writing error', data.toString()); PHPWASM.proc_fds[stderrParentFd].hasData = true; PHPWASM.proc_fds[stderrParentFd].emit('data'); stderrStream.stream_ops.write( diff --git a/packages/php-wasm/node/src/lib/node-php.ts b/packages/php-wasm/node/src/lib/node-php.ts index 627604dc31..c5a02dfb86 100644 --- a/packages/php-wasm/node/src/lib/node-php.ts +++ b/packages/php-wasm/node/src/lib/node-php.ts @@ -48,7 +48,6 @@ export class NodePHP extends BasePHP { return await NodePHP.loadSync(phpVersion, { ...options, emscriptenOptions: { - ...(options.emscriptenOptions || {}), /** * Emscripten default behavior is to kill the process when * the WASM program calls `exit()`. We want to throw an @@ -57,6 +56,7 @@ export class NodePHP extends BasePHP { quit: function (code, error) { throw error; }, + ...(options.emscriptenOptions || {}), }, }).phpReady; } diff --git a/packages/php-wasm/node/src/test/php.spec.ts b/packages/php-wasm/node/src/test/php.spec.ts index 8708bef266..96bde95027 100644 --- a/packages/php-wasm/node/src/test/php.spec.ts +++ b/packages/php-wasm/node/src/test/php.spec.ts @@ -88,7 +88,7 @@ describe.each(SupportedPHPVersions)('PHP %s', (phpVersion) => { $pipes ); - // Yields back to JS event loop to capture and process the + // Yields back to JS event loop to capture and process the // child_process output. This is fine. Regular PHP scripts // typically wait for the child process to finish. sleep(1); @@ -133,7 +133,7 @@ describe.each(SupportedPHPVersions)('PHP %s', (phpVersion) => { // This test fails if (!['7.0', '7.1', '7.2', '7.3'].includes(phpVersion)) { /* - There is a race condition in this variant of the test which + There is a race condition in this variant of the test which causes the following failure (but only sometimes): src/test/php.spec.ts > PHP 8.2 > proc_open() > cat – stdin=pipe, stdout=file, stderr=file @@ -153,7 +153,7 @@ describe.each(SupportedPHPVersions)('PHP %s', (phpVersion) => { ); fwrite($pipes[0], 'WordPress\n'); - // Yields back to JS event loop to capture and process the + // Yields back to JS event loop to capture and process the // child_process output. This is fine. Regular PHP scripts // typically wait for the child process to finish. sleep(1); @@ -184,7 +184,7 @@ describe.each(SupportedPHPVersions)('PHP %s', (phpVersion) => { $pipes ); - // Yields back to JS event loop to capture and process the + // Yields back to JS event loop to capture and process the // child_process output. This is fine. Regular PHP scripts // typically wait for the child process to finish. sleep(1); @@ -1073,3 +1073,16 @@ describe.each(['7.0', '7.1', '7.3', '7.4', '8.0', '8.1'])( }, 500_000); } ); + +describe.each(SupportedPHPVersions)('PHP %s', (phpVersion) => { + describe('emscripten options', () => { + it('calls quit callback', async () => { + let result = ''; + const php: NodePHP = await NodePHP.load(phpVersion as any, { + emscriptenOptions: { quit: () => (result = 'WordPress') }, + }); + php.exit(0); + expect(result).toEqual('WordPress'); + }); + }); +});