Add electron-log logging service with renderer IPC bridge#62
Conversation
- Install electron-log ^5 dependency - Create app/components/logService.js (renderer-side IPC logging wrapper) - Create __mocks__/electron-log.js (Jest manual mock) - Update main.js: import electron-log, configure transports, add log:send IPC handler - Update app/preload.js: add log:send to allowed IPC channels - Update app/games/registry.js: replace console.warn with electron-log - Update app/interface.js: replace console.error with logger from logService - Create app/components/tests/logService.test.js (14 new tests) - Update app/preload.test.js: add log:send channel coverage - Update app/games/registry.test.js: mock electron-log instead of console.warn - Update app/interface.test.js: assert log:send IPC calls instead of console.error - Update eslint.config.js: enforce no-console in main.js and registry.js - Update .github/copilot-instructions.md: document logging service for future agents Agent-Logs-Url: https://github.com/acrosman/BrainSpeedExercises/sessions/d8111c46-baba-4217-910e-7673656aef6a Co-authored-by: acrosman <2972053+acrosman@users.noreply.github.com>
Agent-Logs-Url: https://github.com/acrosman/BrainSpeedExercises/sessions/d8111c46-baba-4217-910e-7673656aef6a Co-authored-by: acrosman <2972053+acrosman@users.noreply.github.com>
acrosman
left a comment
There was a problem hiding this comment.
Make sure to run all tests to validate they pass, and provide required code coverage, before completing your next draft.
| * @enum {string} | ||
| */ | ||
| export const LOG_LEVELS = /** @type {const} */ ( | ||
| ['error', 'warn', 'info', 'verbose', 'debug', 'silly'] |
There was a problem hiding this comment.
Remove silly. Remove any other references to silly in the logging process.
There was a problem hiding this comment.
Removed silly from LOG_LEVELS and updated all references in logService.js, main.js, __mocks__/electron-log.js, tests, and docs in 6fee3eb.
| /** Log a debug-level message. @param {...*} args */ | ||
| debug: (...args) => log('debug', ...args), | ||
| /** Log a silly-level message. @param {...*} args */ | ||
| silly: (...args) => log('silly', ...args), |
There was a problem hiding this comment.
Remove all references to silly log level.
There was a problem hiding this comment.
Done in 6fee3eb — silly removed from the logger object, LOG_LEVELS, main.js validLevels, mock file, tests, and copilot instructions.
| }, | ||
| }, | ||
| // Main process (Node.js) | ||
| // Main process (Node.js) — scripts still allowed; main.js uses electron-log |
There was a problem hiding this comment.
Comments in the code should not reflect the change change, they should reflect the purpose of the current code.
There was a problem hiding this comment.
Fixed in 6fee3eb. The comment now reads // Scripts (Node.js) — console allowed for build tooling to describe purpose rather than context of the change.
| }, | ||
| }, | ||
| // Game registry and progress (Node.js backend) | ||
| // Game registry and progress (Node.js backend) — console is disallowed (use electron-log) |
There was a problem hiding this comment.
Don't include a note about the default setting here. Also it looks like main.js could be folded into this section.
There was a problem hiding this comment.
Done in 6fee3eb. Merged main.js into the Node.js backend section alongside app/games/registry.js and app/progress/**/*.js. The comment about the default setting was also removed.
| * @param {{ level: string, message: string }} params | ||
| */ | ||
| ipcMain.handle('log:send', (event, { level, message }) => { | ||
| const validLevels = ['error', 'warn', 'info', 'verbose', 'debug', 'silly']; |
There was a problem hiding this comment.
Remove all references to silly log level.
There was a problem hiding this comment.
Removed in 6fee3eb — silly is gone from the validLevels array in the log:send IPC handler.
…nfig - Remove 'silly' log level from logService.js, main.js, __mocks__/electron-log.js, registry.test.js, logService.test.js, and copilot-instructions.md - Add moduleNameMapper for electron-log in jest.config.js so the integration test always uses the mock instead of the real electron package - Merge main.js into the Node.js backend section in eslint.config.js - Clean up eslint.config.js comments to reflect purpose rather than changes Agent-Logs-Url: https://github.com/acrosman/BrainSpeedExercises/sessions/a6e448d7-ca97-42b8-8266-a10f337209ef Co-authored-by: acrosman <2972053+acrosman@users.noreply.github.com>
All 33 test suites pass (1261 tests) with coverage thresholds met and lint clean after 6fee3eb. The integration test failure is also fixed. |
acrosman
left a comment
There was a problem hiding this comment.
This is closer but there is still work to do. Everything runs without errors, but I don't see any logs getting actually created. That's probably because there isn't actually enough logging happening. Make sure there are log messages generated with the application starts up, when it saves updates to user history (including the name of the file that was saved), and when the application is quit. They should all be info level messages.
- main.js: log.info on app ready (startup) and before-quit (shutdown) - progressManager.js: import electron-log; log.info after saveProgress includes the full file path that was saved - progressManager.test.js: mock electron-log via jest.unstable_mockModule; add test verifying log.info is called with the saved file path Agent-Logs-Url: https://github.com/acrosman/BrainSpeedExercises/sessions/1f1ab5e9-02ce-4a88-a7c4-5abb2ff115ea Co-authored-by: acrosman <2972053+acrosman@users.noreply.github.com>
Done in f098222. Added three
The |
There was a problem hiding this comment.
Pull request overview
This PR introduces persistent application logging via electron-log in the main process, plus a renderer-side logService that forwards logs over a new log:send IPC channel so UI code and game plugins can log without console.*.
Changes:
- Add
electron-logand initialize/configure it inmain.js, including an IPC handler for renderer-forwarded logs. - Add renderer
app/components/logService.js(logger.*) and update renderer code/tests to use IPC logging instead ofconsole.*. - Update Jest/ESLint configuration and add mocks/tests to support the new logging pipeline.
Reviewed changes
Copilot reviewed 15 out of 17 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Adds electron-log dependency. |
| package-lock.json | Locks electron-log resolved version/metadata. |
| main.js | Initializes/configures electron-log; adds lifecycle logs; adds log:send IPC handler. |
| jest.config.js | Maps electron-log to a Jest manual mock. |
| eslint.config.js | Adjusts per-context ESLint sections around console usage (needs alignment with stated “error” enforcement). |
| app/progress/progressManager.js | Logs successful progress saves via electron-log. |
| app/progress/progressManager.test.js | Mocks electron-log; asserts save path is included in log output. |
| app/preload.js | Adds log:send to the IPC allowlist. |
| app/preload.test.js | Tests log:send is allowed for invoke. |
| app/interface.js | Switches renderer error logging to logger.error(...). |
| app/interface.test.js | Updates assertions to check log:send IPC calls instead of console spying. |
| app/games/registry.js | Replaces console.warn with log.warn. |
| app/games/registry.test.js | Updates tests to mock/assert electron-log warnings. |
| app/components/logService.js | Adds renderer-side IPC logging wrapper (log, logger, LOG_LEVELS). |
| app/components/tests/logService.test.js | Adds unit tests for logService. |
| mocks/electron-log.js | Adds Jest manual mock for electron-log. |
| .github/copilot-instructions.md | Documents the logging service usage and testing patterns. |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
- logService.js: extract serializeArg() helper that special-cases Error (preserves name, message, stack) and wraps JSON.stringify in try/catch to fall back to String() for circular objects - logService.test.js: add three new tests covering Error serialization, circular object fallback, and null serialization - eslint.config.js: elevate no-console from 'warn' to 'error' in the global default config so the rule is an error for all app code Agent-Logs-Url: https://github.com/acrosman/BrainSpeedExercises/sessions/df28bd81-f296-416e-ad15-8dc61ea337db Co-authored-by: acrosman <2972053+acrosman@users.noreply.github.com>
All
console.*calls in application code were ad-hoc with no persistent output. This replaces them withelectron-log(writing to OS-appropriate log files) and adds a shared renderer-side wrapper so game plugins and UI code can log through the same pipeline via IPC.New:
app/components/logService.jsRenderer-side logging service shared across all games and UI code. Fire-and-forget over
log:sendIPC — failures are swallowed so logging never interrupts gameplay.Arguments are serialized via a safe
serializeArg()helper that special-casesErrorobjects (preservingname,message, andstack) and wrapsJSON.stringifyin atry/catchto fall back toString()for circular or non-serializable values — so logging never throws and error details are never silently dropped.Main process (
main.js,registry.js,progressManager.js)Import
electron-logdirectly:log.initialize()is called before transport level overrides. Console level isdebugin dev,warnin production; file transport always atinfo.Key
info-level lifecycle events are logged:app.on('ready', ...)handler inmain.js.progressManager.jsafter each successful save, including the full path of the file written.app.on('before-quit', ...)handler inmain.js.IPC channel:
log:sendAdded to the
preload.jsallowlist and handled inmain.js. Renderer messages are prefixed with[renderer]in the log output. Supported levels:error,warn,info,verbose,debug.ESLint
no-consoleis nowerrorin the global default config —console.*is a lint error everywhere exceptscripts/and test files. Themain.jsESLint config entry is merged into the shared Node.js backend section.Tests & mocks
__mocks__/electron-log.js— Jest manual mock with stubs for all log levelsjest.config.js—moduleNameMapperrouteselectron-logimports to the manual mock in all test environmentsregistry.test.js— switched from patchingconsole.warntojest.unstable_mockModule('electron-log', ...)interface.test.js— assertions now verifylog:sendIPC calls instead ofconsoleErrorSpyprogressManager.test.js— mockselectron-log; new test asserts the saved file path appears in the log messagelogService.js, including coverage ofErrorserialization, circular object fallback, andnullCopilot instructions
Added §5c documenting when to use
electron-logvslogService, theloggerAPI, and mocking patterns for future agents.