-
Notifications
You must be signed in to change notification settings - Fork 271
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
Add logging support to Playground #1035
Merged
Merged
Changes from 34 commits
Commits
Show all changes
37 commits
Select commit
Hold shift + click to select a range
6b21516
Basic error logging
bgrgicak d804cec
Move logger to a new file
bgrgicak 8480857
Update log message param
bgrgicak 7f2c27a
Cleanup
bgrgicak 46542e5
Return n lines
bgrgicak 315a622
Move logger to website
bgrgicak c23dd92
Display debug.log for request
bgrgicak 60aa62f
Rewrite compiler error to return all error data
bgrgicak 0923c3e
Remove test log
bgrgicak 6d6bdaf
Clean up
bgrgicak d13033d
Merge branch 'trunk' into add/wp-logs
bgrgicak 194bcbd
Merge branch 'add/wp-logs' of https://github.com/WordPress/wordpress-…
bgrgicak b84bdc0
Collect PHP logs on compile error
bgrgicak 46ca5ee
Merge branch 'trunk' into add/wp-logs
bgrgicak 42e47d8
Move logger to blueprints
bgrgicak 3ea93c5
Remove unused dependency
bgrgicak b1b1c71
Fix linter error
bgrgicak c3e2c77
Move logger length to Logger
bgrgicak 9eed92f
Cleanup unused changes
bgrgicak cabbd9b
Address feedback
bgrgicak 5ab98c7
Reset base-php
bgrgicak 6ec3c4a
Merge branch 'trunk' into add/wp-logs
bgrgicak c9f0b5c
Move logger to package
bgrgicak f28c20a
Start logger in client and node
bgrgicak 65dd662
Merge branch 'trunk' into add/wp-logs
bgrgicak be90919
Remove node logger
bgrgicak d0398a3
Set WP_DEBUG only if WP exists
bgrgicak 0650878
Update packages/playground/logger/logger.ts
bgrgicak a192ceb
Update packages/playground/remote/src/lib/playground-mu-plugin/playgr…
bgrgicak 06158e7
Merge branch 'trunk' into add/wp-logs
bgrgicak b14f4a5
Move logger to a buildable package
bgrgicak d0546aa
Fix linter
bgrgicak 451ede1
Merge branch 'trunk' into add/wp-logs
bgrgicak 1d840ed
Move logger code to main plugin file
bgrgicak a3feb32
Attach JS logger in Website only
bgrgicak ef4993b
Revert compile error
bgrgicak 988a9a5
Move window log collection to main
bgrgicak File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
{ | ||
"extends": ["../../../.eslintrc.json"], | ||
"ignorePatterns": ["!**/*"], | ||
"overrides": [ | ||
{ | ||
"files": ["*.ts", "*.tsx", "*.js", "*.jsx"], | ||
"rules": {} | ||
}, | ||
{ | ||
"files": ["*.ts", "*.tsx"], | ||
"rules": {} | ||
}, | ||
{ | ||
"files": ["*.js", "*.jsx"], | ||
"rules": {} | ||
} | ||
] | ||
} |
Empty file.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
{ | ||
"name": "@php-wasm/logger", | ||
"version": "0.0.1", | ||
"description": "A logger for PHP-wasm clients like Playground and WP-now.", | ||
"repository": { | ||
"type": "git", | ||
"url": "https://github.com/WordPress/wordpress-playground" | ||
}, | ||
"homepage": "https://developer.wordpress.org/playground", | ||
"author": "The WordPress contributors", | ||
"typedoc": { | ||
"entryPoint": "./src/index.ts", | ||
"readmeFile": "./README.md", | ||
"displayName": "@php-wasm/logger", | ||
"tsconfig": "./tsconfig.lib.json" | ||
}, | ||
"publishConfig": { | ||
"access": "public", | ||
"directory": "../../../dist/packages/php-wasm/logger" | ||
}, | ||
"license": "GPL-2.0-or-later", | ||
"main": "index.cjs", | ||
"types": "index.d.ts", | ||
"engines": { | ||
"node": ">=18.18.2", | ||
"npm": ">=8.11.0" | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
{ | ||
"name": "php-wasm-logger", | ||
"$schema": "../../../node_modules/nx/schemas/project-schema.json", | ||
"sourceRoot": "packages/php-wasm/logger/src", | ||
"projectType": "library", | ||
"targets": { | ||
"build": { | ||
"executor": "@nx/vite:build", | ||
"outputs": ["{options.outputPath}"], | ||
"options": { | ||
"outputPath": "dist/packages/php-wasm/logger" | ||
} | ||
}, | ||
"test": { | ||
"executor": "nx:noop", | ||
"dependsOn": ["test:vite"] | ||
}, | ||
"test:esmcjs": { | ||
"executor": "@wp-playground/nx-extensions:assert-built-esm-and-cjs", | ||
"options": { | ||
"outputPath": "dist/packages/php-wasm/logger" | ||
}, | ||
"dependsOn": ["build"] | ||
}, | ||
"test:vite": { | ||
"executor": "@nx/vite:test", | ||
"outputs": ["{workspaceRoot}/coverage/packages/php-wasm/logger"], | ||
"options": { | ||
"passWithNoTests": true, | ||
"reportsDirectory": "../../../coverage/packages/php-wasm/logger" | ||
} | ||
}, | ||
"lint": { | ||
"executor": "@nx/linter:eslint", | ||
"outputs": ["{options.outputFile}"], | ||
"options": { | ||
"lintFilePatterns": ["packages/php-wasm/logger/**/*.ts"] | ||
} | ||
} | ||
}, | ||
"typecheck": { | ||
"executor": "nx:run-commands", | ||
"options": { | ||
"commands": [ | ||
"tsc -p packages/php-wasm/logger/tsconfig.lib.json --noEmit", | ||
"tsc -p packages/php-wasm/logger/tsconfig.spec.json --noEmit" | ||
] | ||
} | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export * from './logger'; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,156 @@ | ||
import { UniversalPHP } from "@php-wasm/universal/src/lib/universal-php"; | ||
/** | ||
* Log severity levels. | ||
*/ | ||
export type LogSeverity = 'debug' | 'info' | 'warn' | 'error' | 'fatal'; | ||
|
||
/** | ||
* A logger for Playground. | ||
*/ | ||
export class Logger { | ||
private readonly LOG_PREFIX = 'Playground'; | ||
|
||
/** | ||
* The length of the last PHP log. | ||
*/ | ||
private lastPHPLogLength = 0; | ||
|
||
/** | ||
* The path to the error log file. | ||
*/ | ||
private errorLogPath = '/wordpress/wp-content/debug.log'; | ||
|
||
constructor(errorLogPath?: string) { | ||
if (errorLogPath) { | ||
this.errorLogPath = errorLogPath; | ||
} | ||
this.collectJavaScriptErrors(); | ||
} | ||
|
||
/** | ||
* Collect errors from JavaScript window events like error and log them. | ||
*/ | ||
private collectJavaScriptErrors() { | ||
if (typeof window !== 'undefined') { | ||
window.addEventListener('error', (event) => { | ||
this.log( | ||
`${event.message} in ${event.filename} on line ${event.lineno}:${event.colno}`, | ||
'fatal' | ||
); | ||
}); | ||
window.addEventListener('unhandledrejection', (event) => { | ||
this.log( | ||
`${event.reason.stack}`, | ||
'fatal' | ||
); | ||
}); | ||
window.addEventListener('rejectionhandled', (event) => { | ||
this.log( | ||
`${event.reason.stack}`, | ||
'fatal' | ||
); | ||
}); | ||
} | ||
} | ||
|
||
/** | ||
* Read the WordPress debug.log file and return its content. | ||
* | ||
* @param UniversalPHP playground instance | ||
* @returns string The content of the debug.log file | ||
*/ | ||
private async getRequestPhpErrorLog(playground: UniversalPHP) { | ||
if (!await playground.fileExists(this.errorLogPath)) { | ||
return ''; | ||
} | ||
return await playground.readFileAsText(this.errorLogPath); | ||
} | ||
/** | ||
* Register a listener for the request.end event and log the data. | ||
* @param UniversalPHP playground instance | ||
*/ | ||
public addPlaygroundRequestEndListener(playground: UniversalPHP) { | ||
playground.addEventListener('request.end', async () => { | ||
const log = await this.getRequestPhpErrorLog(playground); | ||
if (log.length > this.lastPHPLogLength) { | ||
this.logRaw(log.substring(this.lastPHPLogLength)); | ||
this.lastPHPLogLength = log.length; | ||
} | ||
} ); | ||
} | ||
|
||
|
||
/** | ||
* Get UTC date in the PHP log format https://github.com/php/php-src/blob/master/main/main.c#L849 | ||
* | ||
* @param date | ||
* @returns string | ||
*/ | ||
private formatLogDate(date: Date): string { | ||
const formattedDate = new Intl.DateTimeFormat('en-GB', { | ||
year: 'numeric', | ||
month: 'short', | ||
day: '2-digit', | ||
timeZone: 'UTC', | ||
}).format(date).replace(/ /g, '-'); | ||
|
||
const formattedTime = new Intl.DateTimeFormat('en-GB', { | ||
hour: '2-digit', | ||
minute: '2-digit', | ||
second: '2-digit', | ||
hour12: false, | ||
timeZone: 'UTC', | ||
timeZoneName: 'short' | ||
}).format(date); | ||
return formattedDate + ' ' + formattedTime; | ||
} | ||
|
||
/** | ||
* Format log message and severity and log it. | ||
* @param string message | ||
* @param LogSeverity severity | ||
*/ | ||
public formatMessage(message: string, severity: LogSeverity): string { | ||
const now = this.formatLogDate(new Date()); | ||
return `[${now}] ${this.LOG_PREFIX} ${severity}: ${message}`; | ||
} | ||
|
||
/** | ||
* Log message with severity and timestamp. | ||
* @param string message | ||
* @param LogSeverity severity | ||
*/ | ||
public log(message: string, severity?: LogSeverity): void { | ||
if (severity === undefined) { | ||
severity = 'info'; | ||
} | ||
const log = this.formatMessage(message, severity); | ||
this.logRaw(log); | ||
} | ||
|
||
/** | ||
* Log message without severity and timestamp. | ||
* @param string log | ||
*/ | ||
public logRaw(log: string): void { | ||
console.debug(log); | ||
} | ||
}; | ||
|
||
/** | ||
* The logger instance. | ||
*/ | ||
let logger: Logger | undefined = undefined; | ||
|
||
/** | ||
* Get the logger instance. | ||
* | ||
* @param errorLogPath The path to the error log file. | ||
* @returns Logger | ||
*/ | ||
export function getLogger(errorLogPath?: string): Logger { | ||
if (!logger) { | ||
logger = new Logger(errorLogPath); | ||
} | ||
return logger; | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
{ | ||
"extends": "../../../tsconfig.base.json", | ||
"compilerOptions": { | ||
"forceConsistentCasingInFileNames": true, | ||
"strict": true, | ||
"noImplicitOverride": true, | ||
"allowSyntheticDefaultImports": true, | ||
"noPropertyAccessFromIndexSignature": true, | ||
"noImplicitReturns": true, | ||
"noFallthroughCasesInSwitch": true, | ||
"types": ["vitest"] | ||
}, | ||
"files": [], | ||
"include": [], | ||
"references": [ | ||
{ | ||
"path": "./tsconfig.lib.json" | ||
}, | ||
{ | ||
"path": "./tsconfig.spec.json" | ||
} | ||
] | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
{ | ||
"extends": "./tsconfig.json", | ||
"compilerOptions": { | ||
"outDir": "../../../dist/out-tsc", | ||
"declaration": true, | ||
"types": ["node"] | ||
}, | ||
"include": ["src/**/*.ts"], | ||
"exclude": ["jest.config.ts", "src/**/*.spec.ts", "src/**/*.test.ts"] | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
{ | ||
"extends": "./tsconfig.json", | ||
"compilerOptions": { | ||
"outDir": "../../dist/out-tsc", | ||
"types": ["vitest/globals", "vitest/importMeta", "vite/client", "node"] | ||
}, | ||
"include": [ | ||
"vite.config.ts", | ||
"src/**/*.test.ts", | ||
"src/**/*.spec.ts", | ||
"src/**/*.test.tsx", | ||
"src/**/*.spec.tsx", | ||
"src/**/*.test.js", | ||
"src/**/*.spec.js", | ||
"src/**/*.test.jsx", | ||
"src/**/*.spec.jsx", | ||
"src/**/*.d.ts" | ||
] | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
/// <reference types="vitest" /> | ||
import { defineConfig } from 'vite'; | ||
|
||
import dts from 'vite-plugin-dts'; | ||
import { join } from 'path'; | ||
|
||
// eslint-disable-next-line @nx/enforce-module-boundaries | ||
import { viteTsConfigPaths } from '../../vite-ts-config-paths'; | ||
|
||
export default defineConfig({ | ||
cacheDir: '../../../node_modules/.vite/php-wasm-logger', | ||
|
||
plugins: [ | ||
dts({ | ||
entryRoot: 'src', | ||
tsconfigPath: join(__dirname, 'tsconfig.lib.json'), | ||
}), | ||
|
||
viteTsConfigPaths({ | ||
root: '../../../', | ||
}), | ||
], | ||
|
||
// Configuration for building your library. | ||
// See: https://vitejs.dev/guide/build.html#library-mode | ||
build: { | ||
lib: { | ||
// Could also be a dictionary or array of multiple entry points. | ||
entry: 'src/index.ts', | ||
name: 'php-wasm-logger', | ||
fileName: 'index', | ||
formats: ['es', 'cjs'], | ||
}, | ||
rollupOptions: { | ||
// External packages that should not be bundled into your library. | ||
external: [], | ||
}, | ||
}, | ||
|
||
test: { | ||
globals: true, | ||
cache: { | ||
dir: '../../../node_modules/.vitest', | ||
}, | ||
environment: 'jsdom', | ||
include: ['src/**/*.{test,spec}.{js,mjs,cjs,ts,mts,cts,jsx,tsx}'], | ||
}, | ||
}); |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice to make this pluggable, maybe as a Constructor argument? This way different runtimes could ship their own global error collecting logic, e.g. the VS Code extension will need highly customized logic that I don't think belongs to this repo. Also, there might be environments where the
window
object exists, and yet it either doesn't provide any useful error information or only provides Playground-unrelated noise – I vaguely remember VS Code might be one of them.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it necessary? What would the constructor argument do?
The logger should be accessible by all environments, so a runtime can submit logs and when we add full support it will also be able to read logs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can merge without that. I'd just eventually move the responsibility for selecting the logging method to the consumer of this API instead of guessing what the environment is.
On the website we'd call something like
collectWindowErrors()
, in web worker that could becollectWorkerErrors()
, and then there could be bindings for service workers, Node workers etc.My point was mostly about exposing a loosely coupled API like
collectWindowErrors(logger)
orcollectPlaygroundErrors(logger, playground);
vs tightly coupledlogger.collectWindowErrors()
logger.addPlaygroundRequestEndListener()
. This would make the Logger class lean and responsible for a single thing, while the "error collectors" could be plentiful, separate, and also provided by other modules.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I refactored the code to match what you described in a3feb32.
Let me know what you think.
collectWindowErrors
runs only in the Website package now.