Centralize log storage (#1315)

## What is this PR doing?

It moves away from using the JS logger (console) to the
`@php-wasm/logger`.

## What problem is it solving?

It centralizes log collections which will allow us to better utilize
logs in the future.

## How is the problem addressed?

By replacing console calls with logger calls. 

## Testing Instructions

- Checkout this branch
- [Open this
blueprint](http://localhost:5400/website-server/?blueprint-url=https://gist.githubusercontent.com/bgrgicak/5fbeb98abec8221192a8b619f852148a/raw/75ff00581b76d9ed3e4844d228dce9b4eda1b163/error-log-blueprint.json)
- Open the report error modal (menu in upper right corner > Report
error)
- See JS logs in the _LOGS_ textarea

**Please review the log messages in this PR and suggest changes (using
the GitHub suggest feature) to messages if they could be improved.**

---------

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

.eslintrc.json

@@ -28,7 +28,7 @@
 				"no-inner-declarations": 0,
 				"no-use-before-define": "off",
 				"react/prop-types": 0,
-				"no-console": 0,
+				"no-console": 1,
 				"no-empty": 0,
 				"no-async-promise-executor": 0,
 				"no-constant-condition": 0,
@@ -60,6 +60,12 @@
 			"files": "*.json",
 			"parser": "jsonc-eslint-parser",
 			"rules": {}
+		},
+		{
+			"files": "*.spec.ts",
+			"rules": {
+				"no-console": 0
+			}
 		}
 	]
 }

packages/nx-extensions/.eslintrc.json

@@ -4,7 +4,9 @@
 	"overrides": [
 		{
 			"files": ["*.ts", "*.tsx", "*.js", "*.jsx"],
-			"rules": {}
+			"rules": {
+				"no-console": 0
+			}
 		},
 		{
 			"files": ["*.ts", "*.tsx"],

packages/php-wasm/fs-journal/src/lib/fs-journal.ts

@@ -5,6 +5,7 @@ import {
 } from '@php-wasm/universal';
 import type { IsomorphicLocalPHP } from '@php-wasm/universal';
 import { Semaphore, basename, joinPaths } from '@php-wasm/util';
+import { logger } from '@php-wasm/logger';
 
 export type EmscriptenFS = any;
 
@@ -401,7 +402,7 @@ export function normalizeFilesystemOperations(
 				//
 				// But that's not a straightforward transformation so let's just not handle
 				// it for now.
-				console.warn(
+				logger.warn(
 					'[FS Journal] Normalizing a double rename is not yet supported:',
 					{
 						current: latter,
@@ -547,11 +548,11 @@ async function hydrateOp(php: UniversalPHP, op: UpdateFileOperation) {
 		op.data = await php.readFileAsBuffer(op.path);
 	} catch (e) {
 		// Log the error but don't throw.
-		console.warn(
+		logger.warn(
 			`Journal failed to hydrate a file on flush: the ` +
 				`path ${op.path} no longer exists`
 		);
-		console.error(e);
+		logger.error(e);
 	}
 
 	release();

packages/php-wasm/logger/src/lib/collectors/collect-php-logs.ts

@@ -1,7 +1,4 @@
-import {
-	UniversalPHP,
-	PHPRequestErrorEvent,
-} from '@php-wasm/universal/src/lib/universal-php';
+import { UniversalPHP, PHPRequestErrorEvent } from '../../universal';
 import { Logger } from '../logger';
 
 let lastPHPLogLength = 0;

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

@@ -1,29 +1,11 @@
-import { NodePHP } from '@php-wasm/node';
-import { LatestSupportedPHPVersion } from '@php-wasm/universal';
-import { Logger, addCrashListener } from '../lib/logger';
-import { collectPhpLogs } from '../lib/log-collector';
+import { Logger } from '../lib/logger';
 import { clearMemoryLogs, logToMemory } from '../lib/log-handlers';
 
 describe('Logger', () => {
-	let php: NodePHP;
 	const logger = new Logger([logToMemory]);
 	beforeEach(async () => {
-		php = await NodePHP.load(LatestSupportedPHPVersion);
 		clearMemoryLogs();
 	});
-	it('Event listener should work', () => {
-		const listener = vi.fn();
-		collectPhpLogs(logger, php);
-		addCrashListener(logger, listener);
-		php.dispatchEvent({
-			type: 'request.error',
-			error: new Error('test'),
-		});
-		expect(listener).toBeCalledTimes(1);
-
-		const logs = logger.getLogs();
-		expect(logs.length).toBe(1);
-	});
 
 	it('Log message should be added', () => {
 		logger.warn('test');

packages/php-wasm/logger/src/universal.d.ts

@@ -0,0 +1,27 @@
+export interface UniversalPHP {
+	/**
+	 * Read the content of a file as text.
+	 *
+	 * @param path The path to the file
+	 * @returns string The content of the file
+	 */
+	readFileAsText(path: string): Promise<string>;
+	/**
+	 * Check if a file exists.
+	 *
+	 * @param path The path to the file
+	 * @returns boolean Whether the file exists
+	 */
+	fileExists(path: string): Promise<boolean>;
+
+	addEventListener(
+		event: string,
+		listener: (event: PHPRequestEndEvent | PHPRequestErrorEvent) => void
+	): void;
+}
+
+export interface PHPRequestErrorEvent {
+	type: 'request.error';
+	error: Error;
+	source?: 'request' | 'php-wasm';
+}

packages/php-wasm/node/src/lib/networking/outbound-ws-to-tcp-proxy.ts

@@ -35,7 +35,7 @@ function prependByte(
 		buffer.set(new Uint8Array(chunk), 1);
 		chunk = buffer.buffer;
 	} else {
-		console.log({ chunk });
+		log({ chunk });
 		throw new Error('Unsupported chunk type: ' + typeof chunk);
 	}
 	return chunk;

packages/php-wasm/node/src/lib/networking/utils.ts

@@ -1,8 +1,9 @@
 import * as net from 'net';
+import { logger } from '@php-wasm/logger';
 
-export function debugLog(...args: any[]) {
+export function debugLog(message: any, ...args: any[]) {
 	if (process.env['DEV'] && !process.env['TEST']) {
-		console.log(...args);
+		logger.log(message, ...args);
 	}
 }
 

packages/php-wasm/progress/src/lib/emscripten-download-monitor.ts

@@ -1,3 +1,4 @@
+import { logger } from '@php-wasm/logger';
 /*
  * An approximate total file size to use when the actual
  * total number of bytes is missing.
@@ -69,7 +70,7 @@ export class EmscriptenDownloadMonitor extends EventTarget {
 			fileSize = this.#assetsSizes[fileName];
 		}
 		if (!(fileName in this.#progress)) {
-			console.warn(
+			logger.warn(
 				`Registered a download #progress of an unregistered file "${fileName}". ` +
 					`This may cause a sudden **decrease** in the #progress percentage as the ` +
 					`total number of bytes increases during the download.`
@@ -156,7 +157,7 @@ export function cloneResponseMonitorProgress(
 							controller.enqueue(value);
 						}
 					} catch (e) {
-						console.error({ e });
+						logger.error({ e });
 						controller.error(e);
 						break;
 					}

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

@@ -27,6 +27,7 @@ import {
 	UnhandledRejectionsTarget,
 } from './wasm-error-reporting';
 import { Semaphore, createSpawnHandler, joinPaths } from '@php-wasm/util';
+import { logger } from '@php-wasm/logger';
 
 const STRING = 'string';
 const NUMBER = 'number';
@@ -292,14 +293,14 @@ export abstract class BasePHP implements IsomorphicLocalPHP {
 
 			const response = await this.#handleRequest();
 			if (response.exitCode !== 0) {
-				console.warn(`PHP.run() output was:`, response.text);
+				logger.warn(`PHP.run() output was:`, response.text);
 				const error = new PHPExecutionFailureError(
 					`PHP.run() failed with exit code ${response.exitCode} and the following output: ` +
 						response.errors,
 					response,
 					'request'
 				) as PHPExecutionFailureError;
-				console.error(error);
+				logger.error(error);
 				throw error;
 			}
 			return response;
@@ -521,7 +522,7 @@ export abstract class BasePHP implements IsomorphicLocalPHP {
 	#setRequestBody(body: string | Uint8Array) {
 		let size, contentLength;
 		if (typeof body === 'string') {
-			console.warn(
+			logger.warn(
 				'Passing a string as the request body is deprecated. Please use a Uint8Array instead. See ' +
 					'https://github.com/WordPress/wordpress-playground/issues/997 for more details'
 			);
@@ -633,8 +634,8 @@ export abstract class BasePHP implements IsomorphicLocalPHP {
 			// eslint-disable-next-line no-async-promise-executor
 			exitCode = await new Promise<number>((resolve, reject) => {
 				errorListener = (e: ErrorEvent) => {
-					console.error(e);
-					console.error(e.error);
+					logger.error(e);
+					logger.error(e.error);
 					const rethrown = new Error('Rethrown');
 					rethrown.cause = e.error;
 					(rethrown as any).betterMessage = e.message;
@@ -680,7 +681,7 @@ export abstract class BasePHP implements IsomorphicLocalPHP {
 			) as string;
 			const rethrown = new Error(message);
 			rethrown.cause = err;
-			console.error(rethrown);
+			logger.error(rethrown);
 			throw rethrown;
 		} finally {
 			this.#wasmErrorsTarget?.removeEventListener('error', errorListener);
@@ -785,7 +786,7 @@ export abstract class BasePHP implements IsomorphicLocalPHP {
 			}
 			return files;
 		} catch (e) {
-			console.error(e, { path });
+			logger.error(e, { path });
 			return [];
 		}
 	}

packages/php-wasm/universal/src/lib/http-cookie-store.ts

@@ -1,3 +1,5 @@
+import { logger } from '@php-wasm/logger';
+
 /**
  * @public
  */
@@ -20,7 +22,7 @@ export class HttpCookieStore {
 					.split(';')[0];
 				this.cookies[name] = value;
 			} catch (e) {
-				console.error(e);
+				logger.error(e);
 			}
 		}
 	}

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

@@ -1,3 +1,5 @@
+import { logger } from '@php-wasm/logger';
+
 const RuntimeId = Symbol('RuntimeId');
 const loadedRuntimes: Map<number, PHPRuntime> = new Map();
 let lastRuntimeId = 0;
@@ -130,7 +132,7 @@ export async function loadPHPRuntime(
 			rejectPHP(reason);
 			// This can happen after PHP has been initialized so
 			// let's just log it.
-			console.error(reason);
+			logger.error(reason);
 		},
 		ENV: {},
 		// Emscripten sometimes prepends a '/' to the path, which

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

@@ -14,6 +14,7 @@ import { PHPResponse } from './php-response';
 import { PHPRequest, PHPRunOptions } from './universal-php';
 import { encodeAsMultipart } from './encode-as-multipart';
 import { HttpCookieStore } from './http-cookie-store';
+import { logger } from '@php-wasm/logger';
 
 export type RewriteRule = {
 	match: RegExp;
@@ -311,7 +312,7 @@ export class PHPRequestHandler {
 			this.#semaphore.running > 0 &&
 			request.headers?.['x-request-issuer'] === 'php'
 		) {
-			console.warn(
+			logger.warn(
 				`Possible deadlock: Called request() before the previous request() have finished. ` +
 					`PHP likely issued an HTTP call to itself. Normally this would lead to infinite ` +
 					`waiting as Request 1 holds the lock that the Request 2 is waiting to acquire. ` +

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

@@ -1,5 +1,6 @@
 import { ErrorEvent } from './error-event-polyfill';
 import { isExitCodeZero } from './is-exit-code-zero';
+import { logger } from '@php-wasm/logger';
 
 type Runtime = {
 	asm: Record<string, unknown>;
@@ -143,11 +144,11 @@ export function showCriticalErrorBox(message: string) {
 	if (message?.trim().startsWith('Program terminated with exit')) {
 		return;
 	}
-	console.log(`${redBg}\n${eol}\n${bold}  WASM ERROR${reset}${redBg}`);
+	logger.log(`${redBg}\n${eol}\n${bold}  WASM ERROR${reset}${redBg}`);
 	for (const line of message.split('\n')) {
-		console.log(`${eol}  ${line} `);
+		logger.log(`${eol}  ${line} `);
 	}
-	console.log(`${reset}`);
+	logger.log(`${reset}`);
 }
 
 function extractPHPFunctionsFromStack(stack: string) {

packages/php-wasm/web-service-worker/.eslintrc.json

@@ -4,7 +4,9 @@
 	"overrides": [
 		{
 			"files": ["*.ts", "*.tsx", "*.js", "*.jsx"],
-			"rules": {}
+			"rules": {
+				"no-console": 0
+			}
 		},
 		{
 			"files": ["*.ts", "*.tsx"],

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

@@ -1,5 +1,6 @@
 import { PHPResponse, PHPResponseData } from '@php-wasm/universal';
 import * as Comlink from 'comlink';
+import { logger } from '@php-wasm/logger';
 
 export type WithAPIState = {
 	/**
@@ -133,7 +134,7 @@ function setupTransferHandlers() {
 	Comlink.transferHandlers.set('FUNCTION', {
 		canHandle: (obj: unknown): obj is Function => typeof obj === 'function',
 		serialize(obj: Function) {
-			console.debug('[Comlink][Performance] Proxying a function');
+			logger.debug('[Comlink][Performance] Proxying a function');
 			const { port1, port2 } = new MessageChannel();
 			Comlink.expose(obj, port1);
 			return [port2, [port2]];

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

@@ -2,6 +2,7 @@ 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 { logger } from '@php-wasm/logger';
 
 /**
  * Run this in the main application to register the service worker or
@@ -33,7 +34,7 @@ export async function registerServiceWorker<
 		}
 	}
 
-	console.debug(`[window][sw] Registering a Service Worker`);
+	logger.debug(`[window][sw] Registering a Service Worker`);
 	const registration = await sw.register(scriptUrl, {
 		type: 'module',
 		// Always bypass HTTP cache when fetching the new Service Worker script: