fix: daemon lifecycle cleanup and error logging improvements

- Refactor HealthDaemon to use a tracked session (AbortController + awaitable
  promise) instead of fire-and-forget health check loops, preventing health
  checks from running after a service is stopped
- Stop health checks before terminating daemon to avoid false crash reports
  during intentional shutdown
- Guard onExit callbacks with AbortSignal to prevent stale session callbacks
- Add logErrorOnce utility to deduplicate repeated error logging
- Fix SystemForEmbassy.stop() to capture clean promise before deleting ref
- Treat SIGTERM (signal 15) as successful exit in subcontainer sync
- Fix asError to return original Error instead of wrapping in new Error
- Remove unused ExtendedVersion import from Backups.ts
This commit is contained in:
Aiden McClelland
2026-03-20 13:35:24 -06:00
parent b54f10af55
commit 7335e52ab3
10 changed files with 73 additions and 52 deletions

View File

@@ -37,7 +37,7 @@
}, },
"../sdk/dist": { "../sdk/dist": {
"name": "@start9labs/start-sdk", "name": "@start9labs/start-sdk",
"version": "0.4.0-beta.61", "version": "0.4.0-beta.62",
"license": "MIT", "license": "MIT",
"dependencies": { "dependencies": {
"@iarna/toml": "^3.0.0", "@iarna/toml": "^3.0.0",

View File

@@ -445,15 +445,14 @@ export class SystemForEmbassy implements System {
} }
callCallback(_callback: number, _args: any[]): void {} callCallback(_callback: number, _args: any[]): void {}
async stop(): Promise<void> { async stop(): Promise<void> {
const { currentRunning } = this const clean = this.currentRunning?.clean({
this.currentRunning?.clean() timeout: fromDuration(
(this.manifest.main["sigterm-timeout"] as any) || "30s",
),
})
delete this.currentRunning delete this.currentRunning
if (currentRunning) { if (clean) {
await currentRunning.clean({ await clean
timeout: fromDuration(
(this.manifest.main["sigterm-timeout"] as any) || "30s",
),
})
} }
} }

View File

@@ -768,7 +768,7 @@ pub fn exec(
stderr_thread.map(|t| t.join().unwrap()); stderr_thread.map(|t| t.join().unwrap());
if let Some(code) = exit.code() { if let Some(code) = exit.code() {
std::process::exit(code); std::process::exit(code);
} else if exit.success() { } else if exit.success() || exit.signal() == Some(15) {
Ok(()) Ok(())
} else { } else {
Err(Error::new( Err(Error::new(

View File

@@ -8,7 +8,7 @@
*/ */
export const asError = (e: unknown) => { export const asError = (e: unknown) => {
if (e instanceof Error) { if (e instanceof Error) {
return new Error(e as any) return e
} }
if (typeof e === 'string') { if (typeof e === 'string') {
return new Error(`${e}`) return new Error(`${e}`)

View File

@@ -32,3 +32,4 @@ export { deepEqual } from './deepEqual'
export { AbortedError } from './AbortedError' export { AbortedError } from './AbortedError'
export * as regexes from './regexes' export * as regexes from './regexes'
export { stringFromStdErrOut } from './stringFromStdErrOut' export { stringFromStdErrOut } from './stringFromStdErrOut'
export { logErrorOnce } from './logErrorOnce'

View File

@@ -0,0 +1,9 @@
const loggedErrors = new WeakSet<object>()
export function logErrorOnce(err: unknown) {
if (typeof err === 'object' && err !== null) {
if (loggedErrors.has(err)) return
loggedErrors.add(err)
}
console.error(err)
}

View File

@@ -2,7 +2,6 @@ import * as T from '../../../base/lib/types'
import * as child_process from 'child_process' import * as child_process from 'child_process'
import * as fs from 'fs/promises' import * as fs from 'fs/promises'
import { Affine, asError } from '../util' import { Affine, asError } from '../util'
import { ExtendedVersion, VersionRange } from '../../../base/lib'
import { InitKind, InitScript } from '../../../base/lib/inits' import { InitKind, InitScript } from '../../../base/lib/inits'
/** Default rsync options used for backup and restore operations */ /** Default rsync options used for backup and restore operations */

View File

@@ -7,6 +7,7 @@ import { Drop, splitCommand } from '../util'
import * as cp from 'child_process' import * as cp from 'child_process'
import * as fs from 'node:fs/promises' import * as fs from 'node:fs/promises'
import { DaemonCommandType, ExecCommandOptions, ExecFnOptions } from './Daemons' import { DaemonCommandType, ExecCommandOptions, ExecFnOptions } from './Daemons'
import { logErrorOnce } from '../../../base/lib/util/logErrorOnce'
/** /**
* Low-level controller for a single running process inside a subcontainer (or as a JS function). * Low-level controller for a single running process inside a subcontainer (or as a JS function).
@@ -220,6 +221,6 @@ export class CommandController<
} }
} }
onDrop(): void { onDrop(): void {
this.term().catch(console.error) this.term().catch(logErrorOnce)
} }
} }

View File

@@ -1,5 +1,6 @@
import * as T from '../../../base/lib/types' import * as T from '../../../base/lib/types'
import { asError } from '../../../base/lib/util/asError' import { asError } from '../../../base/lib/util/asError'
import { logErrorOnce } from '../../../base/lib/util/logErrorOnce'
import { Drop } from '../util' import { Drop } from '../util'
import { import {
SubContainer, SubContainer,
@@ -64,7 +65,7 @@ export class Daemon<
) )
const res = new Daemon(subc, startCommand) const res = new Daemon(subc, startCommand)
effects.onLeaveContext(() => { effects.onLeaveContext(() => {
res.term({ destroySubcontainer: true }).catch((e) => console.error(e)) res.term({ destroySubcontainer: true }).catch((e) => logErrorOnce(e))
}) })
return res return res
} }
@@ -86,7 +87,7 @@ export class Daemon<
if (this.commandController) if (this.commandController)
await this.commandController await this.commandController
.term({}) .term({})
.catch((err) => console.error(err)) .catch((err) => logErrorOnce(err))
try { try {
this.commandController = await this.startCommand() this.commandController = await this.startCommand()
if (!this.shouldBeRunning) { if (!this.shouldBeRunning) {
@@ -97,7 +98,7 @@ export class Daemon<
const success = await this.commandController.wait().then( const success = await this.commandController.wait().then(
(_) => true, (_) => true,
(err) => { (err) => {
console.error(err) if (this.shouldBeRunning) logErrorOnce(err)
return false return false
}, },
) )
@@ -147,7 +148,7 @@ export class Daemon<
this.onExitFns = [] this.onExitFns = []
} }
if (this.exiting) { if (this.exiting) {
await this.exiting.catch(console.error) await this.exiting.catch(logErrorOnce)
if (termOptions?.destroySubcontainer) { if (termOptions?.destroySubcontainer) {
await this.subcontainer?.destroy() await this.subcontainer?.destroy()
} }
@@ -172,6 +173,6 @@ export class Daemon<
this.onExitFns.push(fn) this.onExitFns.push(fn)
} }
onDrop(): void { onDrop(): void {
this.term().catch((e) => console.error(asError(e))) this.term().catch((e) => logErrorOnce(asError(e)))
} }
} }

View File

@@ -4,14 +4,6 @@ import { Ready } from './Daemons'
import { Daemon } from './Daemon' import { Daemon } from './Daemon'
import { SetHealth, Effects, SDKManifest } from '../../../base/lib/types' import { SetHealth, Effects, SDKManifest } from '../../../base/lib/types'
const oncePromise = <T>() => {
let resolve: (value: T) => void
const promise = new Promise<T>((res) => {
resolve = res
})
return { resolve: resolve!, promise }
}
export const EXIT_SUCCESS = 'EXIT_SUCCESS' as const export const EXIT_SUCCESS = 'EXIT_SUCCESS' as const
/** /**
@@ -29,6 +21,7 @@ export class HealthDaemon<Manifest extends SDKManifest> {
private resolveReady: (() => void) | undefined private resolveReady: (() => void) | undefined
private resolvedReady: boolean = false private resolvedReady: boolean = false
private readyPromise: Promise<void> private readyPromise: Promise<void>
private session: { abort: AbortController; done: Promise<void> } | null = null
constructor( constructor(
readonly daemon: Daemon<Manifest> | null, readonly daemon: Daemon<Manifest> | null,
readonly dependencies: HealthDaemon<Manifest>[], readonly dependencies: HealthDaemon<Manifest>[],
@@ -54,7 +47,7 @@ export class HealthDaemon<Manifest extends SDKManifest> {
}) { }) {
this.healthWatchers = [] this.healthWatchers = []
this.running = false this.running = false
this.healthCheckCleanup?.() await this.stopSession()
await this.daemon?.term({ await this.daemon?.term({
...termOptions, ...termOptions,
@@ -77,20 +70,25 @@ export class HealthDaemon<Manifest extends SDKManifest> {
if (newStatus) { if (newStatus) {
console.debug(`Launching ${this.id}...`) console.debug(`Launching ${this.id}...`)
this.setupHealthCheck() this.startSession()
this.daemon?.start() this.daemon?.start()
this.started = performance.now() this.started = performance.now()
} else { } else {
console.debug(`Stopping ${this.id}...`) console.debug(`Stopping ${this.id}...`)
this.daemon?.term() await this.stopSession()
await this.turnOffHealthCheck() await this.daemon?.term()
} }
} }
private healthCheckCleanup: (() => Promise<null>) | null = null private async stopSession() {
private async turnOffHealthCheck() { if (!this.session) return
await this.healthCheckCleanup?.() this.session.abort.abort()
await this.session.done
this.session = null
this.resetReady()
}
private resetReady() {
this.resolvedReady = false this.resolvedReady = false
this.readyPromise = new Promise( this.readyPromise = new Promise(
(resolve) => (resolve) =>
@@ -100,8 +98,14 @@ export class HealthDaemon<Manifest extends SDKManifest> {
}), }),
) )
} }
private async setupHealthCheck() {
private startSession() {
this.session?.abort.abort()
const abort = new AbortController()
this.daemon?.onExit((success) => { this.daemon?.onExit((success) => {
if (abort.signal.aborted) return
if (success && this.ready === 'EXIT_SUCCESS') { if (success && this.ready === 'EXIT_SUCCESS') {
this.setHealth({ result: 'success', message: null }) this.setHealth({ result: 'success', message: null })
} else if (!success) { } else if (!success) {
@@ -116,42 +120,49 @@ export class HealthDaemon<Manifest extends SDKManifest> {
}) })
} }
}) })
const done =
this.ready === 'EXIT_SUCCESS'
? Promise.resolve()
: this.runHealthCheckLoop(abort.signal)
this.session = { abort, done }
}
private async runHealthCheckLoop(signal: AbortSignal): Promise<void> {
if (this.ready === 'EXIT_SUCCESS') return if (this.ready === 'EXIT_SUCCESS') return
if (this.healthCheckCleanup) return
const trigger = (this.ready.trigger ?? defaultTrigger)(() => ({ const trigger = (this.ready.trigger ?? defaultTrigger)(() => ({
lastResult: this._health.result, lastResult: this._health.result,
})) }))
const { promise: status, resolve: setStatus } = oncePromise<{ const aborted = new Promise<{ done: true }>((resolve) =>
done: true signal.addEventListener('abort', () => resolve({ done: true }), {
}>() once: true,
const { promise: exited, resolve: setExited } = oncePromise<null>() }),
new Promise(async () => { )
if (this.ready === 'EXIT_SUCCESS') return
try {
for ( for (
let res = await Promise.race([status, trigger.next()]); let res = await Promise.race([aborted, trigger.next()]);
!res.done; !res.done;
res = await Promise.race([status, trigger.next()]) res = await Promise.race([aborted, trigger.next()])
) { ) {
const response: HealthCheckResult = await Promise.resolve( const response: HealthCheckResult = await Promise.resolve(
this.ready.fn(), this.ready.fn(),
).catch((err) => { ).catch((err) => {
return { return {
result: 'failure', result: 'failure' as const,
message: 'message' in err ? err.message : String(err), message: 'message' in err ? err.message : String(err),
} }
}) })
if (signal.aborted) break
await this.setHealth(response) await this.setHealth(response)
} }
setExited(null) } catch (err) {
}).catch((err) => console.error(`Daemon ${this.id} failed: ${err}`)) if (!signal.aborted) {
console.error(`Daemon ${this.id} health check failed: ${err}`)
this.healthCheckCleanup = async () => { }
setStatus({ done: true })
await exited
this.healthCheckCleanup = null
return null
} }
} }