Centralize log storage #1315
Centralize log storage #1315
Conversation
Co-authored-by: Adam Zielinski <adam@adamziel.com>
Co-authored-by: Adam Zielinski <adam@adamziel.com>
Co-authored-by: Adam Zielinski <adam@adamziel.com>
| @@ -29,7 +29,7 @@ export function ErrorReportModal(props: { blueprint: Blueprint }) { | |||
| useEffect(() => { | |||
| resetForm(); | |||
| if (showErrorModal) { | |||
| setLogs(logger.getLogs().join('')); | |||
| setLogs(logger.getLogs().join('\n')); | |||
There was a problem hiding this comment.
Ensure log entries are displayed as lines
| 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); | ||
| }); |
There was a problem hiding this comment.
This test would require a lot of mocking and it's not that useful, so I removed it.
| UniversalPHP, | ||
| PHPRequestErrorEvent, | ||
| } from '@php-wasm/universal/src/lib/universal-php'; | ||
| import { UniversalPHP, PHPRequestErrorEvent } from '../../universal'; |
There was a problem hiding this comment.
These imports were causing circular dependency issues, so I added type declarations.
There was a problem hiding this comment.
Not great, not terrible 🤷
There was a problem hiding this comment.
I can always switch to any 😅
|
@WordPress/playground-maintainers I'm asking for a review early to get your suggestions for log message improvements. I will work tomorrow on fixing the e2e errors. |
It seems like this PR is focused on replacing |
Co-authored-by: Adam Zielinski <adam@adamziel.com>
bgrgicak
added
[Type] Enhancement
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
Please review the log messages in this PR and suggest changes (using the GitHub suggest feature) to messages if they could be improved.