fix(logger): batch fix audit-logger, circular meta, and sync callers

- Make logAudit and closeAuditLogger synchronous (were async for no reason)
- Set audit-logger silent:true initially, enable on applyAuditConfig()
- Add try-catch for circular references in consoleFormat meta JSON
- Update all callers to remove unnecessary await/.catch() on sync functions
- Add comment to shared.ts explaining acceptable sync FS usage
- Fix audit-logger test for sync closeAuditLogger

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Misaka
2026-04-03 21:25:06 +08:00
parent a2e3681c8f
commit 6a9d144bbc
10 changed files with 37 additions and 24 deletions

View File

@@ -3,9 +3,9 @@ import logger from '../services/logger/index'
import { logAudit, closeAuditLogger } from '../services/logger/audit-logger' import { logAudit, closeAuditLogger } from '../services/logger/audit-logger'
export function setupProcessGuards(): void { export function setupProcessGuards(): void {
process.on('uncaughtException', async (err) => { process.on('uncaughtException', (err) => {
logger.error('Uncaught exception', { error: err }) logger.error('Uncaught exception', { error: err })
await logAudit('SYSTEM_CRASH', 'system', { logAudit('SYSTEM_CRASH', 'system', {
username: 'system', username: 'system',
computerName: process.env.COMPUTERNAME || 'unknown', computerName: process.env.COMPUTERNAME || 'unknown',
resource: 'main-process', resource: 'main-process',
@@ -15,9 +15,9 @@ export function setupProcessGuards(): void {
setTimeout(() => process.exit(1), 1000) setTimeout(() => process.exit(1), 1000)
}) })
process.on('unhandledRejection', async (reason) => { process.on('unhandledRejection', (reason) => {
logger.error('Unhandled Rejection', { reason: String(reason) }) logger.error('Unhandled Rejection', { reason: String(reason) })
await logAudit('SYSTEM_ERROR', 'system', { logAudit('SYSTEM_ERROR', 'system', {
username: 'system', username: 'system',
computerName: process.env.COMPUTERNAME || 'unknown', computerName: process.env.COMPUTERNAME || 'unknown',
resource: 'main-process', resource: 'main-process',

View File

@@ -292,7 +292,7 @@ export function registerExtractorHandlers(): void {
recordCount: result.recordCount, recordCount: result.recordCount,
errorCount: result.errors.length errorCount: result.errors.length
} }
}).catch((err) => log.warn('Failed to write audit log', { err })) })
} }
return result return result

View File

@@ -9,7 +9,7 @@
* - Error-level logs bypass circuit breaker * - Error-level logs bypass circuit breaker
*/ */
import { ipcMain, BrowserWindow } from 'electron' import { ipcMain } from 'electron'
import winston from 'winston' import winston from 'winston'
import { createLogger } from '../services/logger' import { createLogger } from '../services/logger'
import logger from '../services/logger' import logger from '../services/logger'

View File

@@ -74,7 +74,7 @@ export function registerSettingsHandlers(): void {
resource: 'ERP_CONFIG', resource: 'ERP_CONFIG',
status: 'success', status: 'success',
metadata: { changeType: 'erp_credentials', usernameChanged: !!settings.erp.username } metadata: { changeType: 'erp_credentials', usernameChanged: !!settings.erp.username }
}).catch((err) => log.warn('Failed to write audit log', { err })) })
} }
return { success: true } return { success: true }

View File

@@ -176,8 +176,6 @@ export class AuthApplicationService {
actorId: string, actorId: string,
payload: Parameters<typeof logAudit>[2] payload: Parameters<typeof logAudit>[2]
): void { ): void {
logAudit(action, actorId, payload).catch((err) => logAudit(action, actorId, payload)
log.warn('Failed to write audit log', { err })
)
} }
} }

View File

@@ -274,7 +274,7 @@ export class CleanerApplicationService {
? 'failure' ? 'failure'
: 'success' : 'success'
await logAudit('CLEAN', String(currentUser.id), { logAudit('CLEAN', String(currentUser.id), {
username: currentUser.username, username: currentUser.username,
computerName: (await import('os')).hostname(), computerName: (await import('os')).hostname(),
resource: 'MATERIAL_PLAN', resource: 'MATERIAL_PLAN',
@@ -288,7 +288,7 @@ export class CleanerApplicationService {
materialsSkipped: result.materialsSkipped, materialsSkipped: result.materialsSkipped,
errorCount: result.errors.length errorCount: result.errors.length
} }
}).catch((err) => log.warn('Failed to write audit log', { err })) })
} }
private async generateAndUploadReport( private async generateAndUploadReport(

View File

@@ -46,7 +46,7 @@ const jsonlFormat = winston.format.printf(({ message }) => {
*/ */
const auditLogger = winston.createLogger({ const auditLogger = winston.createLogger({
level: 'info', level: 'info',
silent: false, silent: true,
transports: [] transports: []
}) })
@@ -57,6 +57,9 @@ const auditLogger = winston.createLogger({
* @param retentionDays - Number of days to retain audit logs * @param retentionDays - Number of days to retain audit logs
*/ */
export function applyAuditConfig(retentionDays: number): void { export function applyAuditConfig(retentionDays: number): void {
// Enable logging now that config is loaded
auditLogger.silent = false
// Remove existing DailyRotateFile transports // Remove existing DailyRotateFile transports
const existingTransports = auditLogger.transports.filter((t) => t instanceof DailyRotateFile) const existingTransports = auditLogger.transports.filter((t) => t instanceof DailyRotateFile)
for (const transport of existingTransports) { for (const transport of existingTransports) {
@@ -83,9 +86,8 @@ export function applyAuditConfig(retentionDays: number): void {
* @param action - The action that was performed * @param action - The action that was performed
* @param userId - User ID who performed the action * @param userId - User ID who performed the action
* @param details - Additional details including username, computerName, resource, status, and optional metadata * @param details - Additional details including username, computerName, resource, status, and optional metadata
* @returns Promise that resolves when the log is written (non-blocking)
*/ */
export async function logAudit( export function logAudit(
action: string, action: string,
userId: string, userId: string,
details: { details: {
@@ -95,7 +97,7 @@ export async function logAudit(
status: 'success' | 'failure' | 'partial' status: 'success' | 'failure' | 'partial'
metadata?: Record<string, unknown> metadata?: Record<string, unknown>
} }
): Promise<void> { ): void {
const entry: AuditEntry = { const entry: AuditEntry = {
timestamp: new Date().toISOString(), timestamp: new Date().toISOString(),
action, action,
@@ -115,8 +117,7 @@ export async function logAudit(
/** /**
* Flush and close the audit logger (call on app shutdown) * Flush and close the audit logger (call on app shutdown)
*/ */
export async function closeAuditLogger(): Promise<void> { export function closeAuditLogger(): void {
// Winston logger.close() is synchronous
auditLogger.close() auditLogger.close()
} }

View File

@@ -56,7 +56,21 @@ const consoleFormat = winston.format.combine(
} }
} }
const metaStr = Object.keys(meta).length > 0 ? ` ${JSON.stringify(meta, null, 2)}` : '' let metaStr = ''
if (Object.keys(meta).length > 0) {
try {
metaStr = ` ${JSON.stringify(meta, null, 2)}`
} catch {
// Fallback for circular references: stringify primitives, replace objects with placeholder
metaStr = ` ${JSON.stringify(
Object.fromEntries(
Object.entries(meta).map(([k, v]) => [k, typeof v === 'object' ? '[Object]' : v])
),
null,
2
)}`
}
}
return `${timestamp} [${level}]${contextStr} ${message}${errorStr}${metaStr}` return `${timestamp} [${level}]${contextStr} ${message}${errorStr}${metaStr}`
}) })
) )
@@ -77,9 +91,7 @@ const fileFormat = winston.format.combine(
// Serialize any error in meta fields (skip if already serialized) // Serialize any error in meta fields (skip if already serialized)
for (const key of Object.keys(info)) { for (const key of Object.keys(info)) {
if (key !== 'error' && info[key] instanceof Error) { if (key !== 'error' && info[key] instanceof Error) {
info[key] = IS_PROD info[key] = IS_PROD ? sanitizeError(serializeError(info[key])) : serializeError(info[key])
? sanitizeError(serializeError(info[key]))
: serializeError(info[key])
} }
} }

View File

@@ -16,6 +16,8 @@ export function getLogDir(): string {
return app.getPath('logs') return app.getPath('logs')
} }
// Fallback for development or before app is ready // Fallback for development or before app is ready
// Note: synchronous FS calls are acceptable here because this branch only
// executes in dev environments when app is not yet ready (rare, at startup).
const devLogDir = path.join(process.cwd(), 'logs') const devLogDir = path.join(process.cwd(), 'logs')
if (!fs.existsSync(devLogDir)) { if (!fs.existsSync(devLogDir)) {
fs.mkdirSync(devLogDir, { recursive: true }) fs.mkdirSync(devLogDir, { recursive: true })

View File

@@ -204,8 +204,8 @@ describe('Audit Logger - Real File Integration', () => {
it('should close audit logger without errors', async () => { it('should close audit logger without errors', async () => {
const { closeAuditLogger } = await import('../../src/main/services/logger/audit-logger') const { closeAuditLogger } = await import('../../src/main/services/logger/audit-logger')
// Should resolve without throwing // Should complete without throwing
await expect(closeAuditLogger()).resolves.toBeUndefined() expect(() => closeAuditLogger()).not.toThrow()
}) })
it('should handle special characters in fields', async () => { it('should handle special characters in fields', async () => {