Skip to content

Commit c37a255

Browse files
committed
fix(electron): address round-6 review feedback
- Add main-safety-net.cjs and services/*.cjs to Jest collectCoverageFrom so the 13 hermetic tests count toward the 50% coverage threshold - Move process.exit(1) to end of safety-net catch block so _fatalHandler fallback is reachable before the process terminates; fix operator precedence to String((err && err.stack) || err) in both dialog calls - Clean up instance 1 listeners before installing instance 2 in test 6 (counter increment) so the test does not rely on _inFatalHandler being stuck true due to mocked process.exit - Add test 13 asserting installSafetyNet returns { fatal } -- guards against a refactor that drops the return value silently no-oping the app.whenReady().catch(_fatalHandler) call in main.cjs - Replace _gaiaLogTeeBound stream mutation with a WeakSet so installLogTee idempotency does not mutate the caller-supplied stream object
1 parent 852e1d6 commit c37a255

4 files changed

Lines changed: 34 additions & 16 deletions

File tree

src/gaia/apps/webui/main-safety-net.cjs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -149,12 +149,13 @@ function installSafetyNet({ logPath, dialogModule, appModule, homedirFn }) {
149149
* @param {object} opts
150150
* @param {EventEmitter} opts.stream - The writable stream to guard.
151151
* @param {string} opts.logPath - Path for fallback error logging.
152-
* @note Must be called at most once per stream instance. Calling it twice
153-
* registers a second 'error' listener and doubles log entries per error.
152+
* @note Must be called at most once per stream instance. The internal WeakSet
153+
* guard enforces this — a second call on the same stream is a no-op.
154154
*/
155+
const _teedStreams = new WeakSet();
155156
function installLogTee({ stream, logPath }) {
156-
if (stream._gaiaLogTeeBound) return;
157-
stream._gaiaLogTeeBound = true;
157+
if (_teedStreams.has(stream)) return;
158+
_teedStreams.add(stream);
158159
stream.on("error", (err) => {
159160
const detail = (err && err.message) || (err && err.stack) || String(err);
160161
appendLog(logPath, `[${new Date().toISOString()}] STREAM_ERROR ${detail}`);

src/gaia/apps/webui/main.cjs

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -46,17 +46,16 @@ try {
4646
}));
4747
} catch (err) {
4848
try { process.stderr.write(`[main] safety-net load failed: ${err.message}\n`); } catch { }
49-
try { dialog.showErrorBox("GAIA failed to start", String(err && err.stack || err)); } catch { }
50-
// Inline fallback so any later .catch(_fatalHandler) still has something callable
51-
// if app.exit(1) hasn't taken effect yet (app.exit schedules, not instant).
49+
try { dialog.showErrorBox("GAIA failed to start", String((err && err.stack) || err)); } catch { }
50+
// Inline fallback: app.whenReady().catch(_fatalHandler) at line ~800 still
51+
// has something callable in the narrow window before process.exit() below
52+
// terminates the process.
5253
_fatalHandler = (e) => {
53-
try { dialog.showErrorBox("GAIA crashed", String(e && e.stack || e)); } catch { }
54+
try { dialog.showErrorBox("GAIA crashed", String((e && e.stack) || e)); } catch { }
5455
try { process.exit(1); } catch { }
5556
};
56-
app.exit(1);
57-
// Stop loading service modules synchronously — app.exit() is scheduled,
58-
// not instant, so execution would otherwise continue into the requires
59-
// below with no uncaughtException handler installed.
57+
// Synchronous exit stops service module requires from running with no
58+
// uncaughtException handler — app.exit() is scheduled, not instant.
6059
process.exit(1);
6160
}
6261

tests/electron/package.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414
"coverageDirectory": "coverage",
1515
"collectCoverageFrom": [
1616
"../../src/gaia/electron/**/*.js",
17+
"../../src/gaia/apps/webui/*.cjs",
18+
"../../src/gaia/apps/webui/services/*.cjs",
1719
"../../src/gaia/apps/**/webui/src/**/*.js",
1820
"!../../src/gaia/apps/**/webui/src/renderer/**/*.js",
1921
"!**/node_modules/**",

tests/electron/test_main_error_handling.js

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -196,10 +196,11 @@ describe("installSafetyNet", () => {
196196
const after1 = JSON.parse(fs.readFileSync(counterPath, "utf8"));
197197
expect(after1.count).toBe(1);
198198

199-
// Each installSafetyNet() call creates a fresh closure with its own
200-
// _inFatalHandler flag. The resetModules()+re-require here creates a
201-
// second independent instance so the second fatal() call is not blocked
202-
// by the first instance's re-entry guard.
199+
// Remove instance 1's listeners so instance 2's fatal() runs cleanly
200+
// without relying on instance 1's _inFatalHandler being stuck true.
201+
addedListeners.forEach(({ event, handler }) => process.removeListener(event, handler));
202+
addedListeners.length = 0;
203+
203204
jest.resetModules();
204205
const { installSafetyNet: installSafetyNet2 } = require(SAFETY_NET_PATH);
205206
installSafetyNet2({
@@ -352,4 +353,19 @@ describe("installSafetyNet", () => {
352353

353354
exitSpy.mockRestore();
354355
});
356+
357+
// ── Test 13: installSafetyNet returns { fatal } ────────────────────────────
358+
// main.cjs destructures { fatal: _fatalHandler } and routes
359+
// app.whenReady().catch() through it. If a refactor stops returning fatal,
360+
// _fatalHandler becomes undefined and the catch silently no-ops.
361+
362+
test("returns { fatal } function so main.cjs can route .catch() through it", () => {
363+
const { installSafetyNet } = require(SAFETY_NET_PATH);
364+
const result = installSafetyNet({
365+
logPath,
366+
dialogModule: mockDialog(),
367+
appModule: mockApp(false),
368+
});
369+
expect(typeof result.fatal).toBe("function");
370+
});
355371
});

0 commit comments

Comments
 (0)