diff --git a/apps/observer/src/bin/daemon.ts b/apps/observer/src/bin/daemon.ts index cbe3859c..7874d97f 100755 --- a/apps/observer/src/bin/daemon.ts +++ b/apps/observer/src/bin/daemon.ts @@ -36,6 +36,7 @@ import { registeredDaemonModuleProviders } from "../daemon-extensions.js"; import { createDaemonShutdown } from "../daemon-shutdown.js"; import { DeviceSyncSignal } from "../device-sync-signal.js"; import { + type ErrorReporterLike, bridgeErrorLogs, configureErrorReporting, getErrorReporter, @@ -248,9 +249,30 @@ const DaemonStartupMessageSchema = z.discriminatedUnion("type", [ message: z.string().min(1).max(2_048), }) .strict(), + z + .object({ + type: z.literal("already-running"), + pid: z.number().int().positive().optional(), + }) + .strict(), ]); type DaemonStartupMessage = z.infer; +/** + * Another daemon of this Resin home holds the lock. Asking for a daemon while one runs (a second + * `resin-daemon`, several MCP shims starting one at once, a service restart beside a manual + * daemon) is an expected outcome, not a crash: it is said plainly and never reported. + */ +export class DaemonAlreadyRunningError extends Error { + readonly pid?: number; + + constructor(pid?: number) { + super(`Resin daemon is already running${pid ? ` (PID: ${pid})` : ""}`); + this.name = "DaemonAlreadyRunningError"; + if (pid !== undefined) this.pid = pid; + } +} + const FAILURE_REMEDIATIONS = { AUTHENTICATION: "Run `resin login` to restore cloud access.", CONFIGURATION: "Run `resin doctor`, then `resin repair` if the problem persists.", @@ -1111,9 +1133,7 @@ async function runForeground(options: { const lockResult = await lock.acquire(); if (lockResult.status === "already_running") { - throw new Error( - `Resin daemon is already running${lockResult.pid ? ` (PID: ${lockResult.pid})` : ""}`, - ); + throw new DaemonAlreadyRunningError(lockResult.pid); } if (!configRecoveryWarning) { const configWarningStatePath = path.join( @@ -1425,6 +1445,10 @@ export async function awaitBackgroundDaemonStartup( reject(new Error(parsed.data.message)); return; } + if (parsed.data.type === "already-running") { + reject(new DaemonAlreadyRunningError(parsed.data.pid)); + return; + } resolve(parsed.data.pid); }; const onError = (error: Error): void => { @@ -1490,6 +1514,11 @@ async function runBackground( child.kill(); } child.unref(); + // A daemon already serving this home is what was asked for. + if (err instanceof DaemonAlreadyRunningError) { + console.log(err.message); + return; + } throw err; } } @@ -1572,20 +1601,50 @@ export async function runDaemonCli(options: { home: readHomeArgument(process.argv.slice(2)), }); installCrashHandlers(reporter); - await main(options.entryFile).catch(async (err) => { - await reporter.captureExceptionImmediate(err, { - handled: false, - level: "fatal", - failureClass: "daemon_startup", - }); - const message = sanitizeStartupError(err); + await main(options.entryFile).catch((err) => + endDaemonCliAfterFailure(err, { + reporter, + exit: (code) => process.exit(code), + log: (message, error) => + error === undefined ? console.error(message) : console.error(message, error), + }), + ); +} + +/** + * Ends the daemon command line after `main` failed. A daemon already running is said plainly and + * never reported (the detached parent hears it as such); any other failure is a fatal startup crash, + * reported before the process exits non-zero. + */ +export async function endDaemonCliAfterFailure( + err: unknown, + deps: { + reporter: Pick; + exit: (code: number) => void; + log: (message: string, error?: unknown) => void; + }, +): Promise { + if (err instanceof DaemonAlreadyRunningError) { sendStartupMessage({ - type: "startup-error", - message: message || "Resin daemon startup failed", + type: "already-running", + ...(err.pid === undefined ? {} : { pid: err.pid }), }); - console.error("Fatal error in daemon CLI:", err); - process.exit(1); + deps.log(err.message); + deps.exit(1); + return; + } + await deps.reporter.captureExceptionImmediate(err, { + handled: false, + level: "fatal", + failureClass: "daemon_startup", + }); + const message = sanitizeStartupError(err); + sendStartupMessage({ + type: "startup-error", + message: message || "Resin daemon startup failed", }); + deps.log("Fatal error in daemon CLI:", err); + deps.exit(1); } // Run directly, this module would start a daemon without the packaged entry's modules. diff --git a/apps/observer/src/error-reporting/sanitize.ts b/apps/observer/src/error-reporting/sanitize.ts index ed369713..ac27e9af 100644 --- a/apps/observer/src/error-reporting/sanitize.ts +++ b/apps/observer/src/error-reporting/sanitize.ts @@ -170,13 +170,21 @@ export function sanitizeText( export interface SanitizedStackFrame { readonly platform: "node:javascript"; - readonly function?: string; + /** The frame's function, or {@link UNKNOWN_FRAME_FUNCTION} for a frame without one. */ + readonly function: string; readonly filename?: string; readonly lineno?: number; readonly colno?: number; readonly in_app: boolean; } +/** + * The name of a frame without a function (`at file:line:col`). PostHog's error tracking drops an + * exception any of whose frames lacks `function` ("missing field `function`"), so such a frame is + * named the way PostHog's own stack parser names it. + */ +export const UNKNOWN_FRAME_FUNCTION = "?"; + const FRAME_WITH_FUNCTION = /^\s*at\s+(?:async\s+)?(.*?)\s+\((.*)\)\s*$/; const FRAME_WITHOUT_FUNCTION = /^\s*at\s+(?:async\s+)?(.*)\s*$/; const LOCATION_PATTERN = /^(.*?):(\d+)(?::(\d+))?$/; @@ -223,7 +231,7 @@ export function parseStackFrames( const filename = sanitizeText(location.file, context, 512); frames.push({ platform: "node:javascript", - function: functionName ? sanitizeText(functionName, context, 256) : undefined, + function: functionName ? sanitizeText(functionName, context, 256) : UNKNOWN_FRAME_FUNCTION, filename, lineno: location.line, colno: location.column, diff --git a/apps/observer/tests/error-reporting/sanitize.test.ts b/apps/observer/tests/error-reporting/sanitize.test.ts index 52a8e8bd..cc9a31db 100644 --- a/apps/observer/tests/error-reporting/sanitize.test.ts +++ b/apps/observer/tests/error-reporting/sanitize.test.ts @@ -116,7 +116,7 @@ describe("stack sanitization", () => { expect(frames).toEqual([ { platform: "node:javascript", - function: undefined, + function: "?", filename: "/Users//x.js", lineno: 1, colno: 1, @@ -124,7 +124,7 @@ describe("stack sanitization", () => { }, { platform: "node:javascript", - function: undefined, + function: "?", filename: "node:internal/process/task_queues", lineno: 95, colno: 5, @@ -149,6 +149,14 @@ describe("stack sanitization", () => { ]); }); + it("names every frame, as PostHog's error tracking requires of each frame it processes", () => { + // An exception with a nameless frame (`at file:line:col`) was dropped from error tracking + // ("missing field `function`"); PostHog's own stack parser names such a frame `?`. + for (const frame of parseStackFrames(stack, context)) { + expect(Object.keys(JSON.parse(JSON.stringify(frame)))).toContain("function"); + } + }); + it("caps frames", () => { const long = [ "Error: x", diff --git a/apps/observer/tests/recovery-healing.test.ts b/apps/observer/tests/recovery-healing.test.ts index 1ceb0ccd..80ec6d97 100644 --- a/apps/observer/tests/recovery-healing.test.ts +++ b/apps/observer/tests/recovery-healing.test.ts @@ -11,9 +11,11 @@ import { squatPipeForTesting } from "@resin/windows-security/testing"; import { afterEach, describe, expect, it, vi } from "vitest"; import { z } from "zod"; import { + DaemonAlreadyRunningError, RecoveryAwareDaemonSupervisor, type RecoveryAwareHealthReport, awaitBackgroundDaemonStartup, + endDaemonCliAfterFailure, getRecoverySnapshot, handleIpcCommand, persistAndSurfaceConfigRecoveryWarning, @@ -765,3 +767,53 @@ describe("configuration healing", () => { expect(stderr).toHaveBeenCalledWith(warning.message); }); }); + +describe("starting a daemon that already runs", () => { + function failureFakes() { + const captureExceptionImmediate = vi.fn(async () => undefined); + const exit = vi.fn<(code: number) => void>(); + const log = vi.fn<(message: string, error?: unknown) => void>(); + return { + deps: { reporter: { captureExceptionImmediate }, exit, log }, + captureExceptionImmediate, + }; + } + + it("hears the detached child report a daemon already running, not a startup failure", async () => { + // SAFETY: Mock EventEmitter implements ChildProcess event surface for test. + const fakeChild = new EventEmitter() as ChildProcess; + const startup = awaitBackgroundDaemonStartup(fakeChild, { timeoutMs: 500 }); + + fakeChild.emit("message", { type: "already-running", pid: 4321 }); + + await expect(startup).rejects.toBeInstanceOf(DaemonAlreadyRunningError); + await expect(startup).rejects.toMatchObject({ + pid: 4321, + message: "Resin daemon is already running (PID: 4321)", + }); + }); + + it("ends without a crash report when the lock is held by a running daemon", async () => { + const { deps, captureExceptionImmediate } = failureFakes(); + + await endDaemonCliAfterFailure(new DaemonAlreadyRunningError(4321), deps); + + expect(captureExceptionImmediate).not.toHaveBeenCalled(); + expect(deps.log).toHaveBeenCalledWith("Resin daemon is already running (PID: 4321)"); + expect(deps.exit).toHaveBeenCalledWith(1); + }); + + it("still reports any other startup failure as a fatal crash", async () => { + const { deps, captureExceptionImmediate } = failureFakes(); + const failure = new Error("state directory is not writable"); + + await endDaemonCliAfterFailure(failure, deps); + + expect(captureExceptionImmediate).toHaveBeenCalledWith(failure, { + handled: false, + level: "fatal", + failureClass: "daemon_startup", + }); + expect(deps.exit).toHaveBeenCalledWith(1); + }); +});