Skip to content

Commit 70b96eb

Browse files
committed
review: hoist logger to file scope, drop logFatalSync
Applying @grypez review feedback on PR #966: - Move the logger and its path from inside main() to file scope, matching the entrypoint convention already used by app.ts, background.ts, and vat-worker.ts. - Delete logFatalSync — the logger's dispatch routine is synchronous (Logger.#dispatch iterates transports via .forEach) and the file transport itself is appendFileSync, so logger.error(...) from inside a fatal handler flushes to disk before the process exits. - Fatal handlers now call logger.error directly instead of the parallel sync helper. Behaviour is unchanged: every terminating path still writes a line to daemon.log before the process goes away.
1 parent 9b9d82b commit 70b96eb

1 file changed

Lines changed: 25 additions & 50 deletions

File tree

packages/kernel-cli/src/commands/daemon-entry.ts

Lines changed: 25 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,13 @@ import { join } from 'node:path';
1111
import { getOcapHome } from '../ocap-home.ts';
1212
import { isProcessAlive } from '../utils.ts';
1313

14+
const ocapDir = getOcapHome();
15+
const logPath = join(ocapDir, 'daemon.log');
16+
const logger = new Logger({
17+
tags: ['daemon'],
18+
transports: [makeFileTransport(logPath)],
19+
});
20+
1421
// Install exit-cause handlers at module load, before main() runs, so
1522
// failures during kernel init also leave a fingerprint. daemon-entry
1623
// runs with `stdio: 'ignore'` under the CLI spawner (see
@@ -20,7 +27,7 @@ import { isProcessAlive } from '../utils.ts';
2027
// time — see the run-notes for two past cases where a daemon
2128
// disappeared with no trace. Every terminating path now writes at
2229
// least one line before the process goes away.
23-
installFatalHandlers(join(getOcapHome(), 'daemon.log'));
30+
installFatalHandlers();
2431

2532
main().catch((error) => {
2633
process.stderr.write(`Daemon fatal: ${String(error)}\n`);
@@ -31,15 +38,8 @@ main().catch((error) => {
3138
* Main daemon entry point. Starts the daemon process and keeps it running.
3239
*/
3340
async function main(): Promise<void> {
34-
const ocapDir = getOcapHome();
3541
await mkdir(ocapDir, { recursive: true });
3642

37-
const logPath = join(ocapDir, 'daemon.log');
38-
const logger = new Logger({
39-
tags: ['daemon'],
40-
transports: [makeFileTransport(logPath)],
41-
});
42-
4343
const socketPath =
4444
process.env.OCAP_SOCKET_PATH ?? join(ocapDir, 'daemon.sock');
4545

@@ -141,49 +141,27 @@ async function readDaemonPid(pidPath: string): Promise<number | undefined> {
141141
/**
142142
* Create a file transport that writes logs to a file.
143143
*
144-
* @param logPath - The log file path.
144+
* @param logFilePath - The log file path.
145145
* @returns A log transport function.
146146
*/
147-
function makeFileTransport(logPath: string) {
147+
function makeFileTransport(logFilePath: string) {
148148
return (entry: LogEntry): void => {
149149
const line = `[${new Date().toISOString()}] [${entry.level}] ${entry.message ?? ''} ${(entry.data ?? []).map(String).join(' ')}\n`;
150150
// eslint-disable-next-line n/no-sync -- synchronous write needed for log transport reliability
151-
appendFileSync(logPath, line);
151+
appendFileSync(logFilePath, line);
152152
};
153153
}
154154

155-
/**
156-
* Append a fatal-path entry to `daemon.log` synchronously. Used from
157-
* `process.on('uncaughtException' | 'unhandledRejection' | 'SIGHUP')`
158-
* handlers where the async logger pipeline can't be trusted to
159-
* flush before the process exits. Best-effort: if the log file is
160-
* unwritable we swallow the error rather than throw from a fatal
161-
* handler.
162-
*
163-
* @param logPath - The daemon-log file path.
164-
* @param message - Short label for the entry.
165-
* @param detail - Optional extra data (stack, error, etc.) — coerced
166-
* to string.
167-
*/
168-
function logFatalSync(
169-
logPath: string,
170-
message: string,
171-
detail?: string | number,
172-
): void {
173-
try {
174-
const tail = detail === undefined ? '' : ` ${detail}`;
175-
const line = `[${new Date().toISOString()}] [error] ${message}${tail}\n`;
176-
// eslint-disable-next-line n/no-sync -- fatal handler must flush before exit
177-
appendFileSync(logPath, line);
178-
} catch {
179-
// Best-effort — the daemon is dying either way.
180-
}
181-
}
182-
183155
/**
184156
* Install process-level handlers that guarantee a log line is
185157
* written for every terminating event before the daemon exits.
186158
*
159+
* The `@metamask/logger` dispatch routine is synchronous and the
160+
* file transport we're using here is `appendFileSync` under the
161+
* hood, so `logger.error(...)` from inside a fatal handler flushes
162+
* to disk before the process exits — no separate sync-write path
163+
* is required.
164+
*
187165
* Handlers registered:
188166
*
189167
* - `uncaughtException` — the classic silent-death path. Node's
@@ -199,33 +177,30 @@ function logFatalSync(
199177
* process; installing a handler lets us log the fact before
200178
* exiting.
201179
* - `exit` — last-ditch record. Fires during every exit, including
202-
* the ones already logged by the handlers above. Sync-safe: only
203-
* sync APIs are usable here.
204-
*
205-
* @param logPath - The daemon-log file path.
180+
* the ones already logged by the handlers above.
206181
*/
207-
function installFatalHandlers(logPath: string): void {
208-
/* eslint-disable n/no-sync, n/no-process-exit -- fatal handlers must flush synchronously and terminate deterministically */
182+
function installFatalHandlers(): void {
183+
/* eslint-disable n/no-process-exit -- fatal handlers must terminate deterministically */
209184
process.on('uncaughtException', (error: unknown) => {
210185
const detail =
211186
error instanceof Error ? (error.stack ?? error.message) : String(error);
212-
logFatalSync(logPath, 'Uncaught exception (about to exit):', detail);
187+
logger.error('Uncaught exception', detail);
213188
process.exit(1);
214189
});
215190
process.on('unhandledRejection', (reason: unknown) => {
216191
const detail =
217192
reason instanceof Error
218193
? (reason.stack ?? reason.message)
219194
: String(reason);
220-
logFatalSync(logPath, 'Unhandled rejection (about to exit):', detail);
195+
logger.error('Unhandled rejection', detail);
221196
process.exit(1);
222197
});
223198
process.on('SIGHUP', () => {
224-
logFatalSync(logPath, 'SIGHUP received; exiting.');
199+
logger.error('SIGHUP received; exiting.');
225200
process.exit(0);
226201
});
227202
process.on('exit', (code) => {
228-
logFatalSync(logPath, `Process exiting (code=${code}).`);
203+
logger.error(`Process exiting (code=${code}).`);
229204
});
230-
/* eslint-enable n/no-sync, n/no-process-exit */
205+
/* eslint-enable n/no-process-exit */
231206
}

0 commit comments

Comments
 (0)