mirror of
https://github.com/ksyasuda/SubMiner.git
synced 2026-03-20 12:11:28 -07:00
fix(launcher): address newest PR review feedback
This commit is contained in:
@@ -409,3 +409,61 @@ test('stats cleanup command fails if attached app exits before startup response'
|
|||||||
});
|
});
|
||||||
}, /Stats app exited before startup response \(status 2\)\./);
|
}, /Stats app exited before startup response \(status 2\)\./);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('stats command aborts pending response wait when app exits before startup response', async () => {
|
||||||
|
const context = createContext();
|
||||||
|
context.args.stats = true;
|
||||||
|
let aborted = false;
|
||||||
|
|
||||||
|
await assert.rejects(async () => {
|
||||||
|
await runStatsCommand(context, {
|
||||||
|
createTempDir: () => '/tmp/subminer-stats-test',
|
||||||
|
joinPath: (...parts) => parts.join('/'),
|
||||||
|
runAppCommandAttached: async () => 2,
|
||||||
|
waitForStatsResponse: async (_responsePath, signal) =>
|
||||||
|
await new Promise((resolve) => {
|
||||||
|
signal?.addEventListener(
|
||||||
|
'abort',
|
||||||
|
() => {
|
||||||
|
aborted = true;
|
||||||
|
resolve({ ok: false, error: 'aborted' });
|
||||||
|
},
|
||||||
|
{ once: true },
|
||||||
|
);
|
||||||
|
}),
|
||||||
|
removeDir: () => {},
|
||||||
|
});
|
||||||
|
}, /Stats app exited before startup response \(status 2\)\./);
|
||||||
|
|
||||||
|
assert.equal(aborted, true);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('stats cleanup command aborts pending response wait when app exits before startup response', async () => {
|
||||||
|
const context = createContext();
|
||||||
|
context.args.stats = true;
|
||||||
|
context.args.statsCleanup = true;
|
||||||
|
context.args.statsCleanupVocab = true;
|
||||||
|
let aborted = false;
|
||||||
|
|
||||||
|
await assert.rejects(async () => {
|
||||||
|
await runStatsCommand(context, {
|
||||||
|
createTempDir: () => '/tmp/subminer-stats-test',
|
||||||
|
joinPath: (...parts) => parts.join('/'),
|
||||||
|
runAppCommandAttached: async () => 2,
|
||||||
|
waitForStatsResponse: async (_responsePath, signal) =>
|
||||||
|
await new Promise((resolve) => {
|
||||||
|
signal?.addEventListener(
|
||||||
|
'abort',
|
||||||
|
() => {
|
||||||
|
aborted = true;
|
||||||
|
resolve({ ok: false, error: 'aborted' });
|
||||||
|
},
|
||||||
|
{ once: true },
|
||||||
|
);
|
||||||
|
}),
|
||||||
|
removeDir: () => {},
|
||||||
|
});
|
||||||
|
}, /Stats app exited before startup response \(status 2\)\./);
|
||||||
|
|
||||||
|
assert.equal(aborted, true);
|
||||||
|
});
|
||||||
|
|||||||
@@ -20,7 +20,10 @@ type StatsCommandDeps = {
|
|||||||
logLevel: LauncherCommandContext['args']['logLevel'],
|
logLevel: LauncherCommandContext['args']['logLevel'],
|
||||||
label: string,
|
label: string,
|
||||||
) => Promise<number>;
|
) => Promise<number>;
|
||||||
waitForStatsResponse: (responsePath: string) => Promise<StatsCommandResponse>;
|
waitForStatsResponse: (
|
||||||
|
responsePath: string,
|
||||||
|
signal?: AbortSignal,
|
||||||
|
) => Promise<StatsCommandResponse>;
|
||||||
removeDir: (targetPath: string) => void;
|
removeDir: (targetPath: string) => void;
|
||||||
};
|
};
|
||||||
|
|
||||||
@@ -31,9 +34,15 @@ const defaultDeps: StatsCommandDeps = {
|
|||||||
joinPath: (...parts) => path.join(...parts),
|
joinPath: (...parts) => path.join(...parts),
|
||||||
runAppCommandAttached: (appPath, appArgs, logLevel, label) =>
|
runAppCommandAttached: (appPath, appArgs, logLevel, label) =>
|
||||||
runAppCommandAttached(appPath, appArgs, logLevel, label),
|
runAppCommandAttached(appPath, appArgs, logLevel, label),
|
||||||
waitForStatsResponse: async (responsePath) => {
|
waitForStatsResponse: async (responsePath, signal) => {
|
||||||
const deadline = Date.now() + STATS_STARTUP_RESPONSE_TIMEOUT_MS;
|
const deadline = Date.now() + STATS_STARTUP_RESPONSE_TIMEOUT_MS;
|
||||||
while (Date.now() < deadline) {
|
while (Date.now() < deadline) {
|
||||||
|
if (signal?.aborted) {
|
||||||
|
return {
|
||||||
|
ok: false,
|
||||||
|
error: 'Cancelled waiting for stats dashboard startup response.',
|
||||||
|
};
|
||||||
|
}
|
||||||
try {
|
try {
|
||||||
if (fs.existsSync(responsePath)) {
|
if (fs.existsSync(responsePath)) {
|
||||||
return JSON.parse(fs.readFileSync(responsePath, 'utf8')) as StatsCommandResponse;
|
return JSON.parse(fs.readFileSync(responsePath, 'utf8')) as StatsCommandResponse;
|
||||||
@@ -66,6 +75,16 @@ export async function runStatsCommand(
|
|||||||
const tempDir = resolvedDeps.createTempDir('subminer-stats-');
|
const tempDir = resolvedDeps.createTempDir('subminer-stats-');
|
||||||
const responsePath = resolvedDeps.joinPath(tempDir, 'response.json');
|
const responsePath = resolvedDeps.joinPath(tempDir, 'response.json');
|
||||||
|
|
||||||
|
const createResponseWait = () => {
|
||||||
|
const controller = new AbortController();
|
||||||
|
return {
|
||||||
|
controller,
|
||||||
|
promise: resolvedDeps
|
||||||
|
.waitForStatsResponse(responsePath, controller.signal)
|
||||||
|
.then((response) => ({ kind: 'response' as const, response })),
|
||||||
|
};
|
||||||
|
};
|
||||||
|
|
||||||
try {
|
try {
|
||||||
const forwarded = args.statsCleanup
|
const forwarded = args.statsCleanup
|
||||||
? ['--stats', '--stats-response-path', responsePath]
|
? ['--stats', '--stats-response-path', responsePath]
|
||||||
@@ -102,19 +121,19 @@ export async function runStatsCommand(
|
|||||||
}
|
}
|
||||||
|
|
||||||
if (!args.statsCleanup && !args.statsStop) {
|
if (!args.statsCleanup && !args.statsStop) {
|
||||||
|
const responseWait = createResponseWait();
|
||||||
const startupResult = await Promise.race([
|
const startupResult = await Promise.race([
|
||||||
resolvedDeps
|
responseWait.promise,
|
||||||
.waitForStatsResponse(responsePath)
|
|
||||||
.then((response) => ({ kind: 'response' as const, response })),
|
|
||||||
attachedExitPromise.then((status) => ({ kind: 'exit' as const, status })),
|
attachedExitPromise.then((status) => ({ kind: 'exit' as const, status })),
|
||||||
]);
|
]);
|
||||||
if (startupResult.kind === 'exit') {
|
if (startupResult.kind === 'exit') {
|
||||||
if (startupResult.status !== 0) {
|
if (startupResult.status !== 0) {
|
||||||
|
responseWait.controller.abort();
|
||||||
throw new Error(
|
throw new Error(
|
||||||
`Stats app exited before startup response (status ${startupResult.status}).`,
|
`Stats app exited before startup response (status ${startupResult.status}).`,
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
const response = await resolvedDeps.waitForStatsResponse(responsePath);
|
const response = await responseWait.promise.then((result) => result.response);
|
||||||
if (!response.ok) {
|
if (!response.ok) {
|
||||||
throw new Error(response.error || 'Stats dashboard failed to start.');
|
throw new Error(response.error || 'Stats dashboard failed to start.');
|
||||||
}
|
}
|
||||||
@@ -130,20 +149,20 @@ export async function runStatsCommand(
|
|||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
const attachedExitPromiseCleanup = attachedExitPromise;
|
const attachedExitPromiseCleanup = attachedExitPromise;
|
||||||
|
const responseWait = createResponseWait();
|
||||||
|
|
||||||
const startupResult = await Promise.race([
|
const startupResult = await Promise.race([
|
||||||
resolvedDeps
|
responseWait.promise,
|
||||||
.waitForStatsResponse(responsePath)
|
|
||||||
.then((response) => ({ kind: 'response' as const, response })),
|
|
||||||
attachedExitPromiseCleanup.then((status) => ({ kind: 'exit' as const, status })),
|
attachedExitPromiseCleanup.then((status) => ({ kind: 'exit' as const, status })),
|
||||||
]);
|
]);
|
||||||
if (startupResult.kind === 'exit') {
|
if (startupResult.kind === 'exit') {
|
||||||
if (startupResult.status !== 0) {
|
if (startupResult.status !== 0) {
|
||||||
|
responseWait.controller.abort();
|
||||||
throw new Error(
|
throw new Error(
|
||||||
`Stats app exited before startup response (status ${startupResult.status}).`,
|
`Stats app exited before startup response (status ${startupResult.status}).`,
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
const response = await resolvedDeps.waitForStatsResponse(responsePath);
|
const response = await responseWait.promise.then((result) => result.response);
|
||||||
if (!response.ok) {
|
if (!response.ok) {
|
||||||
throw new Error(response.error || 'Stats dashboard failed to start.');
|
throw new Error(response.error || 'Stats dashboard failed to start.');
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -9,15 +9,45 @@ import type { Args } from './types';
|
|||||||
import {
|
import {
|
||||||
cleanupPlaybackSession,
|
cleanupPlaybackSession,
|
||||||
findAppBinary,
|
findAppBinary,
|
||||||
|
launchTexthookerOnly,
|
||||||
parseMpvArgString,
|
parseMpvArgString,
|
||||||
runAppCommandCaptureOutput,
|
runAppCommandCaptureOutput,
|
||||||
shouldResolveAniSkipMetadata,
|
shouldResolveAniSkipMetadata,
|
||||||
|
stopOverlay,
|
||||||
startOverlay,
|
startOverlay,
|
||||||
state,
|
state,
|
||||||
waitForUnixSocketReady,
|
waitForUnixSocketReady,
|
||||||
} from './mpv';
|
} from './mpv';
|
||||||
import * as mpvModule from './mpv';
|
import * as mpvModule from './mpv';
|
||||||
|
|
||||||
|
class ExitSignal extends Error {
|
||||||
|
code: number;
|
||||||
|
|
||||||
|
constructor(code: number) {
|
||||||
|
super(`exit:${code}`);
|
||||||
|
this.code = code;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
function withProcessExitIntercept(callback: () => void): ExitSignal {
|
||||||
|
const originalExit = process.exit;
|
||||||
|
try {
|
||||||
|
process.exit = ((code?: number) => {
|
||||||
|
throw new ExitSignal(code ?? 0);
|
||||||
|
}) as typeof process.exit;
|
||||||
|
callback();
|
||||||
|
} catch (error) {
|
||||||
|
if (error instanceof ExitSignal) {
|
||||||
|
return error;
|
||||||
|
}
|
||||||
|
throw error;
|
||||||
|
} finally {
|
||||||
|
process.exit = originalExit;
|
||||||
|
}
|
||||||
|
|
||||||
|
throw new Error('expected process.exit');
|
||||||
|
}
|
||||||
|
|
||||||
function createTempSocketPath(): { dir: string; socketPath: string } {
|
function createTempSocketPath(): { dir: string; socketPath: string } {
|
||||||
const baseDir = path.join(process.cwd(), '.tmp', 'launcher-mpv-tests');
|
const baseDir = path.join(process.cwd(), '.tmp', 'launcher-mpv-tests');
|
||||||
fs.mkdirSync(baseDir, { recursive: true });
|
fs.mkdirSync(baseDir, { recursive: true });
|
||||||
@@ -71,6 +101,44 @@ test('parseMpvArgString preserves empty quoted tokens', () => {
|
|||||||
]);
|
]);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('launchTexthookerOnly exits non-zero when app binary cannot be spawned', () => {
|
||||||
|
const error = withProcessExitIntercept(() => {
|
||||||
|
launchTexthookerOnly('/definitely-missing-subminer-binary', makeArgs());
|
||||||
|
});
|
||||||
|
|
||||||
|
assert.equal(error.code, 1);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('stopOverlay logs a warning when stop command cannot be spawned', () => {
|
||||||
|
const originalWrite = process.stdout.write;
|
||||||
|
const writes: string[] = [];
|
||||||
|
const overlayProc = {
|
||||||
|
killed: false,
|
||||||
|
kill: () => true,
|
||||||
|
} as unknown as NonNullable<typeof state.overlayProc>;
|
||||||
|
|
||||||
|
try {
|
||||||
|
process.stdout.write = ((chunk: string | Uint8Array) => {
|
||||||
|
writes.push(Buffer.isBuffer(chunk) ? chunk.toString('utf8') : String(chunk));
|
||||||
|
return true;
|
||||||
|
}) as typeof process.stdout.write;
|
||||||
|
state.stopRequested = false;
|
||||||
|
state.overlayManagedByLauncher = true;
|
||||||
|
state.appPath = '/definitely-missing-subminer-binary';
|
||||||
|
state.overlayProc = overlayProc;
|
||||||
|
|
||||||
|
stopOverlay(makeArgs({ logLevel: 'warn' }));
|
||||||
|
|
||||||
|
assert.ok(writes.some((text) => text.includes('Failed to stop SubMiner overlay')));
|
||||||
|
} finally {
|
||||||
|
process.stdout.write = originalWrite;
|
||||||
|
state.stopRequested = false;
|
||||||
|
state.overlayManagedByLauncher = false;
|
||||||
|
state.appPath = '';
|
||||||
|
state.overlayProc = null;
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
test('waitForUnixSocketReady returns false when socket never appears', async () => {
|
test('waitForUnixSocketReady returns false when socket never appears', async () => {
|
||||||
const { dir, socketPath } = createTempSocketPath();
|
const { dir, socketPath } = createTempSocketPath();
|
||||||
try {
|
try {
|
||||||
|
|||||||
@@ -703,6 +703,9 @@ export function launchTexthookerOnly(appPath: string, args: Args): never {
|
|||||||
stdio: 'inherit',
|
stdio: 'inherit',
|
||||||
env: buildAppEnv(),
|
env: buildAppEnv(),
|
||||||
});
|
});
|
||||||
|
if (result.error) {
|
||||||
|
fail(`Failed to launch texthooker mode: ${result.error.message}`);
|
||||||
|
}
|
||||||
process.exit(result.status ?? 0);
|
process.exit(result.status ?? 0);
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -716,10 +719,15 @@ export function stopOverlay(args: Args): void {
|
|||||||
const stopArgs = ['--stop'];
|
const stopArgs = ['--stop'];
|
||||||
if (args.logLevel !== 'info') stopArgs.push('--log-level', args.logLevel);
|
if (args.logLevel !== 'info') stopArgs.push('--log-level', args.logLevel);
|
||||||
|
|
||||||
spawnSync(state.appPath, stopArgs, {
|
const result = spawnSync(state.appPath, stopArgs, {
|
||||||
stdio: 'ignore',
|
stdio: 'ignore',
|
||||||
env: buildAppEnv(),
|
env: buildAppEnv(),
|
||||||
});
|
});
|
||||||
|
if (result.error) {
|
||||||
|
log('warn', args.logLevel, `Failed to stop SubMiner overlay: ${result.error.message}`);
|
||||||
|
} else if (typeof result.status === 'number' && result.status !== 0) {
|
||||||
|
log('warn', args.logLevel, `SubMiner overlay stop command exited with status ${result.status}`);
|
||||||
|
}
|
||||||
|
|
||||||
if (state.overlayProc && !state.overlayProc.killed) {
|
if (state.overlayProc && !state.overlayProc.killed) {
|
||||||
try {
|
try {
|
||||||
|
|||||||
Reference in New Issue
Block a user