mirror of
https://github.com/ksyasuda/SubMiner.git
synced 2026-07-27 16:49:51 -07:00
fix(sync): harden detached launch, bounded --check probes, and pipe-clos
- `sync --ui` now launches detached; closing standalone Sync window exits app - `--check` uses BatchMode + ConnectTimeout + 15s timeoutMs on all SSH calls - `runSyncLauncher` settles on `exit` (not just `close`) for Electron pipe inheritance - Added `timeoutMs` to `runSyncLauncher`; check-host passes 30s deadline - `handleSyncCliAtEntry` wraps exit deps for testability; fixes Linux AppImage GUI fallthrough - `config-settings-window` gains optional `onClosed` hook - Added preload-syncui bundle to `build:syncui` script
This commit is contained in:
@@ -45,13 +45,14 @@ test('createOpenConfigSettingsWindowHandler creates window and clears closed sta
|
||||
createSettingsWindow: () => created,
|
||||
settingsHtmlPath: '/tmp/settings.html',
|
||||
promoteSettingsWindowAboveOverlay: () => calls.push('promote'),
|
||||
onClosed: () => calls.push('on-closed'),
|
||||
});
|
||||
|
||||
assert.equal(open(), true);
|
||||
assert.deepEqual(calls, ['load:/tmp/settings.html', 'set:window', 'show', 'focus', 'promote']);
|
||||
assert.ok(handlers.closed);
|
||||
handlers.closed();
|
||||
assert.equal(calls.at(-1), 'set:null');
|
||||
assert.deepEqual(calls.slice(-2), ['set:null', 'on-closed']);
|
||||
});
|
||||
|
||||
test('createOpenConfigSettingsWindowHandler clears failed load window state', async () => {
|
||||
|
||||
@@ -13,6 +13,7 @@ export interface OpenConfigSettingsWindowDeps<TWindow extends ConfigSettingsWind
|
||||
createSettingsWindow(): TWindow;
|
||||
settingsHtmlPath: string;
|
||||
promoteSettingsWindowAboveOverlay?: (window: TWindow) => void;
|
||||
onClosed?: () => void;
|
||||
log?: (message: string) => void;
|
||||
}
|
||||
|
||||
@@ -42,6 +43,7 @@ export function createOpenConfigSettingsWindowHandler<TWindow extends ConfigSett
|
||||
deps.setSettingsWindow(window);
|
||||
window.on('closed', () => {
|
||||
deps.setSettingsWindow(null);
|
||||
deps.onClosed?.();
|
||||
});
|
||||
showAndFocus(window);
|
||||
return true;
|
||||
|
||||
@@ -89,6 +89,23 @@ test('runSyncLauncher falls back to stderr when no result event arrives', async
|
||||
assert.match(result.error ?? '', /boom/);
|
||||
});
|
||||
|
||||
test('runSyncLauncher settles when the child exits before its stdio pipes close', async () => {
|
||||
const { spawn, children } = makeSpawn();
|
||||
const handle = runSyncLauncher({
|
||||
command: ['subminer'],
|
||||
args: ['sync', 'media-box', '--check', '--json'],
|
||||
onEvent: () => {},
|
||||
spawn,
|
||||
});
|
||||
children[0]!.emit('exit', 0, null);
|
||||
|
||||
const result = await Promise.race([
|
||||
handle.done,
|
||||
new Promise<null>((resolve) => setTimeout(() => resolve(null), 25)),
|
||||
]);
|
||||
assert.deepEqual(result, { ok: true, error: null });
|
||||
});
|
||||
|
||||
test('runSyncLauncher cancel kills the child and resolves as cancelled', async () => {
|
||||
const { spawn, children } = makeSpawn();
|
||||
const handle = runSyncLauncher({
|
||||
@@ -105,6 +122,24 @@ test('runSyncLauncher cancel kills the child and resolves as cancelled', async (
|
||||
assert.match(result.error ?? '', /cancel/i);
|
||||
});
|
||||
|
||||
test('runSyncLauncher times out a child that never completes', async () => {
|
||||
const { spawn, children } = makeSpawn();
|
||||
const handle = runSyncLauncher({
|
||||
command: ['subminer'],
|
||||
args: ['sync', 'media-box', '--check', '--json'],
|
||||
onEvent: () => {},
|
||||
spawn,
|
||||
timeoutMs: 1,
|
||||
});
|
||||
|
||||
const result = await Promise.race([
|
||||
handle.done,
|
||||
new Promise<null>((resolve) => setTimeout(() => resolve(null), 25)),
|
||||
]);
|
||||
assert.equal(children[0]!.killed, true);
|
||||
assert.deepEqual(result, { ok: false, error: 'Sync operation timed out.' });
|
||||
});
|
||||
|
||||
test('runSyncLauncher surfaces spawn errors', async () => {
|
||||
const { spawn, children } = makeSpawn();
|
||||
const handle = runSyncLauncher({
|
||||
|
||||
@@ -9,6 +9,7 @@ export interface SyncLauncherChildLike {
|
||||
stdout: { on(event: 'data', listener: (chunk: Buffer | string) => void): unknown } | null;
|
||||
stderr: { on(event: 'data', listener: (chunk: Buffer | string) => void): unknown } | null;
|
||||
on(event: 'close', listener: (code: number | null, signal: string | null) => void): unknown;
|
||||
on(event: 'exit', listener: (code: number | null, signal: string | null) => void): unknown;
|
||||
on(event: 'error', listener: (error: Error) => void): unknown;
|
||||
kill(signal?: NodeJS.Signals): boolean;
|
||||
}
|
||||
@@ -54,6 +55,7 @@ export function runSyncLauncher(options: {
|
||||
onEvent: (event: SyncProgressEvent) => void;
|
||||
onStderr?: (text: string) => void;
|
||||
spawn?: SyncLauncherSpawn;
|
||||
timeoutMs?: number;
|
||||
}): SyncLauncherRunHandle {
|
||||
const spawn =
|
||||
options.spawn ??
|
||||
@@ -70,7 +72,7 @@ export function runSyncLauncher(options: {
|
||||
let stdoutBuffer = '';
|
||||
let stderrTail = '';
|
||||
let resultEvent: Extract<SyncProgressEvent, { type: 'result' }> | null = null;
|
||||
let cancelled = false;
|
||||
let terminationError: string | null = null;
|
||||
|
||||
child.stdout?.on('data', (chunk) => {
|
||||
stdoutBuffer += chunk.toString();
|
||||
@@ -92,19 +94,29 @@ export function runSyncLauncher(options: {
|
||||
options.onStderr?.(text);
|
||||
});
|
||||
|
||||
let killTimer: ReturnType<typeof setTimeout> | null = null;
|
||||
let operationTimer: ReturnType<typeof setTimeout> | null = null;
|
||||
const clearTimers = (): void => {
|
||||
if (killTimer !== null) clearTimeout(killTimer);
|
||||
if (operationTimer !== null) clearTimeout(operationTimer);
|
||||
killTimer = null;
|
||||
operationTimer = null;
|
||||
};
|
||||
|
||||
const done = new Promise<SyncLauncherRunResult>((resolve) => {
|
||||
let settled = false;
|
||||
const settle = (result: SyncLauncherRunResult): void => {
|
||||
if (settled) return;
|
||||
settled = true;
|
||||
clearTimers();
|
||||
resolve(result);
|
||||
};
|
||||
child.on('error', (error) => {
|
||||
settle({ ok: false, error: error.message });
|
||||
});
|
||||
child.on('close', (code) => {
|
||||
if (cancelled) {
|
||||
settle({ ok: false, error: 'Sync cancelled.' });
|
||||
const settleFromExit = (code: number | null) => {
|
||||
if (terminationError) {
|
||||
settle({ ok: false, error: terminationError });
|
||||
return;
|
||||
}
|
||||
if (code === 0) {
|
||||
@@ -114,39 +126,42 @@ export function runSyncLauncher(options: {
|
||||
const error =
|
||||
resultEvent?.error ?? (stderrTail.trim() || `Launcher exited with code ${code ?? 'null'}.`);
|
||||
settle({ ok: false, error });
|
||||
});
|
||||
};
|
||||
// Electron descendants can retain inherited stdio pipes after the main
|
||||
// child exits, delaying `close` indefinitely. `exit` is authoritative for
|
||||
// process completion; keep `close` as a fallback for test/process shims.
|
||||
child.on('exit', settleFromExit);
|
||||
child.on('close', settleFromExit);
|
||||
});
|
||||
|
||||
// A sync child blocked on an ssh password prompt ignores SIGTERM, so escalate
|
||||
// to SIGKILL if it is still alive after a grace period. The timer is cleared
|
||||
// on close so a cancelled run cannot hold the process open.
|
||||
let killTimer: ReturnType<typeof setTimeout> | null = null;
|
||||
const clearKillTimer = (): void => {
|
||||
if (killTimer === null) return;
|
||||
clearTimeout(killTimer);
|
||||
killTimer = null;
|
||||
};
|
||||
child.on('close', clearKillTimer);
|
||||
|
||||
return {
|
||||
cancel: () => {
|
||||
if (cancelled) return;
|
||||
cancelled = true;
|
||||
// A sync child blocked on an ssh password prompt may ignore SIGTERM, so
|
||||
// escalate to SIGKILL after a grace period.
|
||||
const terminate = (error: string): void => {
|
||||
if (terminationError) return;
|
||||
terminationError = error;
|
||||
killTimer = setTimeout(() => {
|
||||
killTimer = null;
|
||||
try {
|
||||
child.kill('SIGTERM');
|
||||
child.kill('SIGKILL');
|
||||
} catch {
|
||||
// process may already be gone
|
||||
}
|
||||
killTimer = setTimeout(() => {
|
||||
killTimer = null;
|
||||
try {
|
||||
child.kill('SIGKILL');
|
||||
} catch {
|
||||
// process may already be gone
|
||||
}
|
||||
}, CANCEL_GRACE_MS);
|
||||
killTimer.unref?.();
|
||||
},
|
||||
}, CANCEL_GRACE_MS);
|
||||
killTimer.unref?.();
|
||||
try {
|
||||
child.kill('SIGTERM');
|
||||
} catch {
|
||||
// process may already be gone
|
||||
}
|
||||
};
|
||||
|
||||
if (options.timeoutMs !== undefined) {
|
||||
operationTimer = setTimeout(() => terminate('Sync operation timed out.'), options.timeoutMs);
|
||||
operationTimer.unref?.();
|
||||
}
|
||||
|
||||
return {
|
||||
cancel: () => terminate('Sync cancelled.'),
|
||||
done,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -9,6 +9,7 @@ import { createSyncUiRuntime, type SyncUiRuntimeDeps } from './sync-ui-runtime';
|
||||
|
||||
interface LauncherCall {
|
||||
args: string[];
|
||||
timeoutMs: number | undefined;
|
||||
onEvent: (event: SyncProgressEvent) => void;
|
||||
finish: (result: { ok: boolean; error: string | null }) => void;
|
||||
cancelled: boolean;
|
||||
@@ -42,6 +43,7 @@ function makeTestRig(root: string, overrides: Partial<SyncUiRuntimeDeps> = {}) {
|
||||
});
|
||||
const call: LauncherCall = {
|
||||
args: options.args,
|
||||
timeoutMs: options.timeoutMs,
|
||||
onEvent: options.onEvent,
|
||||
finish,
|
||||
cancelled: false,
|
||||
@@ -216,6 +218,7 @@ test('check-host resolves with the check-result event', () =>
|
||||
}>;
|
||||
await new Promise((resolve) => setImmediate(resolve));
|
||||
assert.deepEqual(launcherCalls[0]!.args, ['sync', 'media-box', '--check', '--json']);
|
||||
assert.equal(launcherCalls[0]!.timeoutMs, 30_000);
|
||||
launcherCalls[0]!.onEvent({
|
||||
type: 'check-result',
|
||||
host: 'media-box',
|
||||
|
||||
@@ -252,6 +252,7 @@ export function createSyncUiRuntime(deps: SyncUiRuntimeDeps) {
|
||||
const handle = deps.runLauncher({
|
||||
command: resolution.command!,
|
||||
args: ['sync', trimmed, '--check', '--json'],
|
||||
timeoutMs: 30_000,
|
||||
onEvent: (event) => {
|
||||
if (event.type === 'check-result') {
|
||||
checkResult = {
|
||||
|
||||
@@ -0,0 +1,34 @@
|
||||
import test from 'node:test';
|
||||
import assert from 'node:assert/strict';
|
||||
import { handleSyncCliAtEntry } from './sync-cli';
|
||||
|
||||
test('handleSyncCliAtEntry exits through the process exit dependency', async () => {
|
||||
const exits: number[] = [];
|
||||
const handled = await handleSyncCliAtEntry(
|
||||
['SubMiner', '--sync-cli', 'sync', '--make-temp'],
|
||||
{},
|
||||
'0.18.0',
|
||||
{
|
||||
run: async () => 7,
|
||||
exit: (code) => {
|
||||
exits.push(code);
|
||||
},
|
||||
},
|
||||
);
|
||||
|
||||
assert.equal(handled, true);
|
||||
assert.deepEqual(exits, [7]);
|
||||
});
|
||||
|
||||
test('handleSyncCliAtEntry ignores normal GUI startup', async () => {
|
||||
const handled = await handleSyncCliAtEntry(['SubMiner', '--start'], {}, '0.18.0', {
|
||||
run: async () => {
|
||||
throw new Error('should not run');
|
||||
},
|
||||
exit: () => {
|
||||
throw new Error('should not exit');
|
||||
},
|
||||
});
|
||||
|
||||
assert.equal(handled, false);
|
||||
});
|
||||
+31
-6
@@ -10,10 +10,7 @@ import {
|
||||
parseSyncCliTokens,
|
||||
syncCliUsage,
|
||||
} from '../core/services/stats-sync/cli-args';
|
||||
import {
|
||||
createDbSnapshot,
|
||||
findLiveStatsDaemonPid,
|
||||
} from '../core/services/stats-sync/shared';
|
||||
import { createDbSnapshot, findLiveStatsDaemonPid } from '../core/services/stats-sync/shared';
|
||||
import { formatMergeSummary, mergeSnapshotIntoDb } from '../core/services/stats-sync/merge';
|
||||
import {
|
||||
assertSafeSshHost,
|
||||
@@ -28,9 +25,17 @@ import {
|
||||
runSyncFlow,
|
||||
type SyncFlowDeps,
|
||||
} from '../core/services/stats-sync/sync-flow';
|
||||
import { recordSyncResult, readSyncHostsState, writeSyncHostsState, getSyncHostsPath } from '../shared/sync/sync-hosts-store';
|
||||
import {
|
||||
recordSyncResult,
|
||||
readSyncHostsState,
|
||||
writeSyncHostsState,
|
||||
getSyncHostsPath,
|
||||
} from '../shared/sync/sync-hosts-store';
|
||||
|
||||
export function shouldHandleSyncCliAtEntry(argv: readonly string[], env: NodeJS.ProcessEnv): boolean {
|
||||
export function shouldHandleSyncCliAtEntry(
|
||||
argv: readonly string[],
|
||||
env: NodeJS.ProcessEnv,
|
||||
): boolean {
|
||||
if (env.ELECTRON_RUN_AS_NODE === '1') return false;
|
||||
return extractSyncCliTokens(argv) !== null;
|
||||
}
|
||||
@@ -188,3 +193,23 @@ export async function runSyncCliFromProcess(
|
||||
return 1;
|
||||
}
|
||||
}
|
||||
|
||||
export async function handleSyncCliAtEntry(
|
||||
argv: readonly string[],
|
||||
env: NodeJS.ProcessEnv,
|
||||
appVersion: string,
|
||||
deps: {
|
||||
run: typeof runSyncCliFromProcess;
|
||||
exit: (code: number) => void;
|
||||
} = {
|
||||
run: runSyncCliFromProcess,
|
||||
exit: (code) => process.exit(code),
|
||||
},
|
||||
): Promise<boolean> {
|
||||
if (!shouldHandleSyncCliAtEntry(argv, env)) return false;
|
||||
const exitCode = await deps.run(argv, appVersion);
|
||||
// This path runs before app readiness and must not call app.exit(): on Linux
|
||||
// that can throw and make the entrypoint fall back to full GUI startup.
|
||||
deps.exit(exitCode);
|
||||
return true;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user