diff --git a/electron/crashReports.js b/electron/crashReports.js index 65f617b2..2128fbc8 100644 --- a/electron/crashReports.js +++ b/electron/crashReports.js @@ -21,6 +21,8 @@ const p_lastByFingerprint = new Map(); function init(app, notifyFn) { p_app = app; p_notify = notifyFn || null; + p_written = 0; + p_lastByFingerprint.clear(); } function reportsDir() { @@ -58,12 +60,26 @@ function fingerprint(kind, details) { return kind + '|' + stack.split('\n').slice(0, 2).join('|').slice(0, 300); } +// Why a crash report was NOT written. Silence here is what made ENG-265 undiagnosable: four real +// renderer crashes produced no file and no line, so nobody could tell a suppressed report from a +// handler that never ran. Never throws: the logging path is exactly what may already be broken. +function p_declineLog(reason, detail) { + try { console.warn(`[crash-reports] declined (${reason}): ${detail}`); } catch (_) { /* stdout is gone */ } +} + function writeCrashReport(kind, details) { const now = Date.now(); const fp = fingerprint(kind, details); const seen = p_lastByFingerprint.get(fp); - if (seen && now - seen.at < DEDUPE_WINDOW_MS) { seen.count += 1; return null; } - if (p_written >= MAX_REPORTS_PER_SESSION) return null; + if (seen && now - seen.at < DEDUPE_WINDOW_MS) { + seen.count += 1; + p_declineLog('deduped', `${fp} seen ${seen.count}x within ${DEDUPE_WINDOW_MS}ms`); + return null; + } + if (p_written >= MAX_REPORTS_PER_SESSION) { + p_declineLog('capped', `${p_written} reports already written this session`); + return null; + } p_lastByFingerprint.set(fp, { at: now, count: 1 }); p_written += 1; try { @@ -116,4 +132,4 @@ function unseenReports() { } } -module.exports = { init, writeCrashReport, unseenReports }; +module.exports = { init, writeCrashReport, unseenReports, DEDUPE_WINDOW_MS, MAX_REPORTS_PER_SESSION }; diff --git a/electron/crashReports.test.js b/electron/crashReports.test.js new file mode 100644 index 00000000..7d9f0e91 --- /dev/null +++ b/electron/crashReports.test.js @@ -0,0 +1,79 @@ +// Run: node --test electron/crashReports.test.js +// +// ENG-265 / ENG-285. Four real renderer crashes were fired at the packaged app and NO crash report +// appeared, with no error line either. That left three indistinguishable explanations: the handler +// never ran, dedupe suppressed it, or the session cap was hit. `writeCrashReport` returned a bare +// null on two of those paths and said nothing, so the instrumentation built to diagnose a crash was +// itself undiagnosable. +// +// These assert that a decline is always ANNOUNCED. A crash handler that silently declines is worse +// than one that fails loudly, because the silence is read as "no crash happened". +const test = require('node:test'); +const assert = require('node:assert/strict'); +const path = require('path'); +const os = require('os'); +const fs = require('fs'); + +const crash = require('./crashReports.js'); + +function withCapturedWarn(fn) { + const lines = []; + const original = console.warn; + console.warn = (...a) => lines.push(a.join(' ')); + try { fn(); } finally { console.warn = original; } + return lines; +} + +function initInTemp() { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'osw-crash-')); + crash.init({ + getVersion: () => '0.0.0-test', + getPath: () => dir, + }, null); + return dir; +} + +test('a deduped report says so instead of returning silently', () => { + initInTemp(); + const first = crash.writeCrashReport('renderer-gone', { message: 'crashed' }); + assert.ok(first, 'the first report should be written'); + const lines = withCapturedWarn(() => { + const second = crash.writeCrashReport('renderer-gone', { message: 'crashed' }); + assert.equal(second, null, 'an identical crash inside the window must still be suppressed'); + }); + assert.ok( + lines.some((l) => l.includes('declined') && l.includes('deduped')), + `a suppressed report announced nothing; lines were ${JSON.stringify(lines)}`, + ); +}); + +test('the announcement carries the fingerprint and the repeat count, not just a word', () => { + initInTemp(); + crash.writeCrashReport('gpu-gone', { message: 'oom' }); + const lines = withCapturedWarn(() => crash.writeCrashReport('gpu-gone', { message: 'oom' })); + assert.ok(lines.some((l) => /seen \d+x/.test(l)), `no repeat count in ${JSON.stringify(lines)}`); +}); + +test('a DIFFERENT crash is not deduped, so the guard cannot swallow real reports', () => { + initInTemp(); + assert.ok(crash.writeCrashReport('renderer-gone', { message: 'crashed' })); + assert.ok( + crash.writeCrashReport('renderer-gone', { message: 'oom' }), + 'a different fingerprint was suppressed; the dedupe is too broad', + ); +}); + +test('the session cap announces itself rather than going quiet', () => { + initInTemp(); + for (let i = 0; i < crash.MAX_REPORTS_PER_SESSION; i++) { + crash.writeCrashReport('renderer-gone', { message: `distinct-${i}` }); + } + const lines = withCapturedWarn(() => { + const over = crash.writeCrashReport('renderer-gone', { message: 'one-too-many' }); + assert.equal(over, null, 'the cap must still hold'); + }); + assert.ok( + lines.some((l) => l.includes('declined') && l.includes('capped')), + `hitting the cap announced nothing; lines were ${JSON.stringify(lines)}`, + ); +}); diff --git a/electron/package.json b/electron/package.json index 30ee7c25..334c39c5 100644 --- a/electron/package.json +++ b/electron/package.json @@ -15,7 +15,7 @@ "dist:win": "electron-builder --win --x64 --publish never", "dist:win:publish": "electron-builder --win --x64 --publish always", "dist:all": "electron-builder --mac --win --linux", - "test": "node --test affiliateTracking.test.js updateErrorMessage.test.js selectWebauthnAccount.test.js voice/streamingVoice.test.js", + "test": "node --test \"*.test.js\" \"voice/*.test.js\"", "test:mouseclamp": "bash native/mouseclamp/run-tests.sh" }, "dependencies": {