feat(main): CC8 — file-backed leveled logger + renderer log sink
Diagnostics were invisible in a packaged build: main catches did a bare console.error (DevTools suppressed in prod, M31) and the renderer's logError (M29) wrote to an unreachable console. Add a small leveled logger (src/main/logger.ts) appending to <userData>/logs/aerofetch.log with size-capped rotation; all app/fs access is lazy+guarded so it's a safe no-op under tests. - Route the 5 main-process console.* catch sites (jsonStore write-fail, settings write-fail + plaintext-secret warn, cookie loadURL, yt-dlp auto-update) through the logger. - Add a fire-and-forget log:write IPC channel + preload logError() so the renderer's logError forwards failures to the main file sink (closes the M29 "sink" half the code comments deferred to CC8). mockApi + Api type updated. - Unit-test pure formatLine/composeMessage + no-op safety; update the R6 jsonStore test to assert against the logger. The user-facing toast half of CC8 ties to the global status surface (UI25/UX9). typecheck + 258 tests + lint + prettier green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -46,6 +46,13 @@ export const INDEX_MAX_BUFFER = 256 * 1024 * 1024
|
||||
/** How much of a failed download's stderr to retain for the error message. */
|
||||
export const STDERR_TAIL_BYTES = 4000
|
||||
|
||||
/**
|
||||
* Size cap for the diagnostic log file (logger.ts, CC8). Once the live
|
||||
* `aerofetch.log` passes this, it's rotated to `aerofetch.log.1` (one generation
|
||||
* kept) so app logging can't fill the disk.
|
||||
*/
|
||||
export const LOG_MAX_BYTES = 1024 * 1024
|
||||
|
||||
// --- JSON-store row caps ----------------------------------------------------
|
||||
// Each hand-rolled JSON store trims to its cap on write so a store file can't
|
||||
// grow without bound.
|
||||
|
||||
+2
-3
@@ -2,6 +2,7 @@ import { app, safeStorage, session, BrowserWindow, type Cookie, type WebContents
|
||||
import { existsSync, statSync, unlinkSync, writeFileSync, readFileSync } from 'fs'
|
||||
import { join } from 'path'
|
||||
import { assertHttpUrl } from './url'
|
||||
import { logger } from './logger'
|
||||
import type { CookiesStatus, CookiesLoginResult } from '@shared/ipc'
|
||||
|
||||
/**
|
||||
@@ -228,9 +229,7 @@ export function openCookieLoginWindow(
|
||||
|
||||
if (loginWindow && !loginWindow.isDestroyed()) {
|
||||
pendingResolvers.push(resolve)
|
||||
loginWindow
|
||||
.loadURL(validUrl)
|
||||
.catch((e) => console.error('[AeroFetch] cookie login loadURL failed:', e))
|
||||
loginWindow.loadURL(validUrl).catch((e) => logger.error('cookie login loadURL failed', e))
|
||||
loginWindow.focus()
|
||||
return
|
||||
}
|
||||
|
||||
+2
-1
@@ -21,6 +21,7 @@ import { flushAllStores } from './jsonStore'
|
||||
import { extractIncomingUrl, registerSendToShortcut, focusWindow } from './deeplink'
|
||||
import { isSyncLaunch } from './schedule'
|
||||
import { createTray, markQuitting, isQuitting } from './tray'
|
||||
import { logger } from './logger'
|
||||
|
||||
// Only one instance ever runs. A second launch — e.g. the OS invoking us again
|
||||
// for an aerofetch:// link or a "Send to AeroFetch" file — hands its argv to
|
||||
@@ -271,7 +272,7 @@ if (isPrimaryInstance) {
|
||||
if (mainWindow && !mainWindow.isDestroyed()) {
|
||||
mainWindow.webContents.send(IpcChannels.ytdlpAutoUpdateStatus, status)
|
||||
}
|
||||
}).catch((e) => console.error('[AeroFetch] yt-dlp auto-update failed:', e))
|
||||
}).catch((e) => logger.error('yt-dlp auto-update failed', e))
|
||||
})
|
||||
|
||||
// Any real quit path (tray "Quit", OS shutdown, the updater's app.quit) must set
|
||||
|
||||
@@ -50,6 +50,7 @@ import { syncWatchedSources } from './sync'
|
||||
import { getScheduledSync, setScheduledSync } from './schedule'
|
||||
import { getActiveBadge, getErrorBadge } from './badge'
|
||||
import { openPoTokenWindow } from './poToken'
|
||||
import { logger } from './logger'
|
||||
|
||||
// --- Theme helpers (shared with index's system-theme bridge) ----------------
|
||||
|
||||
@@ -87,6 +88,12 @@ export function registerIpcHandlers(getMainWindow: () => BrowserWindow | null):
|
||||
app.quit()
|
||||
})
|
||||
|
||||
// CC8: persist renderer-side failures (the M29 `logError` sites) to the main log
|
||||
// file — the renderer console is unreachable in a packaged build. Fire-and-forget.
|
||||
ipcMain.on(IpcChannels.logWrite, (_e, op: string, detail: string) =>
|
||||
logger.error(`[renderer] ${op}`, detail)
|
||||
)
|
||||
|
||||
ipcMain.handle(IpcChannels.appVersion, () => app.getVersion())
|
||||
|
||||
ipcMain.handle(IpcChannels.appUpdateCheck, () => checkForAppUpdate())
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { app } from 'electron'
|
||||
import { join } from 'path'
|
||||
import { readFileSync, writeFileSync, renameSync, existsSync, copyFileSync, unlinkSync } from 'fs'
|
||||
import { logger } from './logger'
|
||||
|
||||
/**
|
||||
* One shared persistence layer for the hand-rolled JSON array stores (history,
|
||||
@@ -69,9 +70,9 @@ export function writeJsonAtomic(path: string, value: unknown, pretty = true): bo
|
||||
return true
|
||||
} catch (e) {
|
||||
// R6: previously a silent catch — a failed persist (disk full, read-only
|
||||
// profile) dropped the change with no trace. Log it; the user-facing warning
|
||||
// is deferred to the leveled file logger (CC8).
|
||||
console.error(`[AeroFetch] failed to write ${path}:`, e)
|
||||
// profile) dropped the change with no trace. It now lands in the file-backed
|
||||
// diagnostic log (CC8); the user-facing toast ties to the global status surface (UI25/UX9).
|
||||
logger.error(`failed to write ${path}`, e)
|
||||
try {
|
||||
if (existsSync(tmp)) unlinkSync(tmp)
|
||||
} catch {
|
||||
|
||||
@@ -0,0 +1,106 @@
|
||||
import { app } from 'electron'
|
||||
import { join } from 'path'
|
||||
import { appendFileSync, mkdirSync, renameSync, statSync } from 'fs'
|
||||
import { LOG_MAX_BYTES } from './constants'
|
||||
|
||||
/**
|
||||
* One small leveled, file-backed logger for the whole app (audit CC8). Before this,
|
||||
* diagnostics were effectively invisible in a packaged build: main-process catches
|
||||
* did a bare `console.error` (unreachable — the app menu/DevTools are suppressed in
|
||||
* production, M31) and the renderer's `logError` (M29) wrote to a console nobody could
|
||||
* open. Every catch now routes here and lands in `<userData>/logs/aerofetch.log`, so a
|
||||
* field-reported failure is actually diagnosable.
|
||||
*
|
||||
* Scope: this is the diagnostic log. `errorlog.ts` remains separate — it's user-facing
|
||||
* *download* failure data shown in Settings → Diagnostics, not app logging. The
|
||||
* user-facing toast half of CC8 ties to the global status surface (UI25/UX9).
|
||||
*
|
||||
* All `app`/fs access is lazy and guarded so importing this module never touches
|
||||
* `app` before it's ready — and so it's a safe no-op under unit tests, where
|
||||
* `electron`'s `app` is undefined.
|
||||
*/
|
||||
|
||||
export type LogLevel = 'error' | 'warn' | 'info' | 'debug'
|
||||
|
||||
const LEVELS: Record<LogLevel, number> = { error: 0, warn: 1, info: 2, debug: 3 }
|
||||
|
||||
// Write everything at or above this level to the file. `debug` is dev-only noise, so
|
||||
// it never hits the file (but still prints to the dev console).
|
||||
const FILE_THRESHOLD = LEVELS.info
|
||||
|
||||
/** Format one log line: `2026-07-01T08:30:00.000Z [ERROR] message`. Pure, for tests. */
|
||||
export function formatLine(level: LogLevel, message: string, now: Date = new Date()): string {
|
||||
return `${now.toISOString()} [${level.toUpperCase()}] ${message}`
|
||||
}
|
||||
|
||||
/** Combine an operation label with an optional error/detail into one message. Pure. */
|
||||
export function composeMessage(label: string, detail?: unknown): string {
|
||||
if (detail === undefined) return label
|
||||
const text = detail instanceof Error ? detail.message : String(detail)
|
||||
return `${label}: ${text}`
|
||||
}
|
||||
|
||||
// `undefined` = not yet resolved, `null` = resolution failed (no app/userData, e.g.
|
||||
// under unit tests) so file logging is disabled; a string is the resolved path.
|
||||
let cachedFile: string | null | undefined
|
||||
function logFile(): string | null {
|
||||
if (cachedFile !== undefined) return cachedFile
|
||||
try {
|
||||
const dir = join(app.getPath('userData'), 'logs')
|
||||
mkdirSync(dir, { recursive: true })
|
||||
cachedFile = join(dir, 'aerofetch.log')
|
||||
} catch {
|
||||
// No app / userData available (unit test, or app not ready) — disable the file sink.
|
||||
cachedFile = null
|
||||
}
|
||||
return cachedFile
|
||||
}
|
||||
|
||||
/** Dev mode, resolved lazily and guarded (app is undefined in the test env). */
|
||||
function isDev(): boolean {
|
||||
try {
|
||||
return !app.isPackaged
|
||||
} catch {
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
// Rotate to `<file>.1` (single generation) once the live file passes the cap, so the
|
||||
// log can't grow without bound. Best-effort: any fs error here is swallowed — logging
|
||||
// must never throw into a catch handler.
|
||||
function rotateIfNeeded(file: string): void {
|
||||
try {
|
||||
if (statSync(file).size > LOG_MAX_BYTES) renameSync(file, `${file}.1`)
|
||||
} catch {
|
||||
/* file missing (first write) or rename race — ignore */
|
||||
}
|
||||
}
|
||||
|
||||
function writeToFile(line: string): void {
|
||||
const file = logFile()
|
||||
if (!file) return
|
||||
rotateIfNeeded(file)
|
||||
try {
|
||||
appendFileSync(file, line + '\n')
|
||||
} catch {
|
||||
/* disk full / read-only profile — nothing more we can safely do */
|
||||
}
|
||||
}
|
||||
|
||||
function emit(level: LogLevel, label: string, detail?: unknown): void {
|
||||
const message = composeMessage(label, detail)
|
||||
// Console in dev keeps the familiar `--inspect` workflow; the file is the sink that
|
||||
// survives into a packaged build.
|
||||
if (isDev()) {
|
||||
const fn = level === 'error' ? console.error : level === 'warn' ? console.warn : console.log
|
||||
fn(`[AeroFetch] ${message}`)
|
||||
}
|
||||
if (LEVELS[level] <= FILE_THRESHOLD) writeToFile(formatLine(level, message))
|
||||
}
|
||||
|
||||
export const logger = {
|
||||
error: (label: string, detail?: unknown): void => emit('error', label, detail),
|
||||
warn: (label: string, detail?: unknown): void => emit('warn', label, detail),
|
||||
info: (label: string, detail?: unknown): void => emit('info', label, detail),
|
||||
debug: (label: string, detail?: unknown): void => emit('debug', label, detail)
|
||||
}
|
||||
@@ -1,6 +1,7 @@
|
||||
import { app, safeStorage } from 'electron'
|
||||
import Store from 'electron-store'
|
||||
import { isSafeFilenameTemplate, isSafeOutputDir } from './validation'
|
||||
import { logger } from './logger'
|
||||
import {
|
||||
AUDIO_FORMATS,
|
||||
VIDEO_CONTAINERS,
|
||||
@@ -130,7 +131,7 @@ function encryptSecret(plain: string): string {
|
||||
// to be written in cleartext. Warn rather than fall back silently — a leaked
|
||||
// settings.json would then expose the proxy password / API token. (DPAPI is
|
||||
// effectively always present on Windows, so this should never fire in practice.)
|
||||
console.warn('[AeroFetch] OS secret encryption unavailable — storing credential as plaintext.')
|
||||
logger.warn('OS secret encryption unavailable — storing credential as plaintext')
|
||||
return plain
|
||||
}
|
||||
|
||||
@@ -238,7 +239,7 @@ export function setSettings(partial: Partial<Settings>): Settings {
|
||||
// handler or surface as an unhandled rejection. Log it; getSettings() below
|
||||
// returns the store's ACTUAL persisted state, so the renderer's reconciliation
|
||||
// (M34) reflects what truly saved instead of the optimistic value.
|
||||
console.error('[AeroFetch] settings write failed:', e)
|
||||
logger.error('settings write failed', e)
|
||||
}
|
||||
cachedSettings = null
|
||||
return getSettings()
|
||||
|
||||
@@ -83,6 +83,10 @@ const api = {
|
||||
|
||||
readClipboard: (): Promise<string> => ipcRenderer.invoke(IpcChannels.clipboardRead),
|
||||
|
||||
/** Forward a renderer-side failure to the main diagnostic log (CC8). Fire-and-forget. */
|
||||
logError: (op: string, detail: string): void =>
|
||||
ipcRenderer.send(IpcChannels.logWrite, op, detail),
|
||||
|
||||
getSettings: (): Promise<Settings> => ipcRenderer.invoke(IpcChannels.settingsGet),
|
||||
|
||||
setSettings: (partial: Partial<Settings>): Promise<Settings> =>
|
||||
|
||||
@@ -137,6 +137,7 @@ export const mockApi: Api = {
|
||||
openUrl: async () => {},
|
||||
showInFolder: async () => {},
|
||||
readClipboard: async () => '',
|
||||
logError: () => {},
|
||||
getSettings: async () => MOCK_SETTINGS,
|
||||
setSettings: async (partial) => Object.assign(MOCK_SETTINGS, partial),
|
||||
listHistory: async () => [],
|
||||
|
||||
@@ -4,10 +4,18 @@
|
||||
// Replaces the swallowed `.catch(() => {})` sites the audit flagged (M29) with a
|
||||
// single, consistent log format. `op` names the operation, e.g. 'addHistory'.
|
||||
//
|
||||
// Note: in production the DevTools console isn't reachable (the app menu is
|
||||
// suppressed), so this is primarily a development/`--inspect` diagnostic until a
|
||||
// persistent renderer log sink lands (CC8). It is still strictly better than
|
||||
// discarding the error.
|
||||
// The failure is both printed to the renderer console (dev/`--inspect`) and
|
||||
// forwarded to the main process's file-backed diagnostic log (CC8), so it survives
|
||||
// into a packaged build where the renderer console is unreachable.
|
||||
export function logError(op: string): (e: unknown) => void {
|
||||
return (e) => console.error(`[AeroFetch] ${op} failed:`, e)
|
||||
return (e) => {
|
||||
console.error(`[AeroFetch] ${op} failed:`, e)
|
||||
try {
|
||||
const detail = e instanceof Error ? e.message : String(e)
|
||||
// window.api is absent in the browser UI preview and in unit tests.
|
||||
if (typeof window !== 'undefined') window.api?.logError?.(op, detail)
|
||||
} catch {
|
||||
/* logging must never throw out of a catch handler */
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -55,6 +55,9 @@ export const IpcChannels = {
|
||||
/** preload → main: the contextBridge failed to expose the API; main shows a
|
||||
* hard error instead of letting the renderer silently fall back to mock mode (M30) */
|
||||
preloadBridgeFailure: 'preload:bridge-failure',
|
||||
/** renderer → main fire-and-forget: forward a renderer-side failure to the main
|
||||
* diagnostic log so it survives into a packaged build (CC8; the M29 sink half). */
|
||||
logWrite: 'log:write',
|
||||
/** main → renderer push channel: a URL handed to AeroFetch from outside the app
|
||||
* (the aerofetch:// protocol, or a .url file via Explorer's "Send to" menu) */
|
||||
externalUrl: 'app:external-url',
|
||||
|
||||
Reference in New Issue
Block a user