From 44a1457baaf6ae6dbaa58b1a95c15ae9058d729b Mon Sep 17 00:00:00 2001 From: TIANYAO ZHANG <88520881+ZHANGTIANYAO1@users.noreply.github.com> Date: Sat, 3 Oct 2026 21:03:05 +0800 Subject: [PATCH] fix: isolate embedded music API credential logs --- src/music/api-server-child.ts | 43 ++++ src/music/api-server-runtime.test.ts | 86 +++++++ src/music/api-server-runtime.ts | 92 ++++++++ src/music/api-server.test.ts | 282 +++++++++++++--------- src/music/api-server.ts | 338 +++++++++++++-------------- 5 files changed, 551 insertions(+), 290 deletions(-) create mode 100644 src/music/api-server-child.ts create mode 100644 src/music/api-server-runtime.test.ts create mode 100644 src/music/api-server-runtime.ts diff --git a/src/music/api-server-child.ts b/src/music/api-server-child.ts new file mode 100644 index 0000000..65bc1b7 --- /dev/null +++ b/src/music/api-server-child.ts @@ -0,0 +1,43 @@ +import type { Server } from "node:http"; +import { closeEmbeddedApi, getSafeApiStartupError, startEmbeddedApi, type ApiChildMessage, type ApiProvider } from "./api-server-runtime.js"; + +const providerArg = process.argv[2]; +const port = Number(process.argv[3]); +if ((providerArg !== "netease" && providerArg !== "qq") || !Number.isInteger(port) || port < 1 || port > 65535 || !process.send) process.exit(1); +const provider = providerArg as ApiProvider; +let server: Server | null = null; +let stopping = false; + +function shutdown(exitCode = 0): void { + if (stopping) return; + stopping = true; + // Also covers a legacy auto-start listener and an import still in flight. + const deadline = setTimeout(() => process.exit(exitCode), 1000); + closeEmbeddedApi(server).finally(() => { clearTimeout(deadline); process.exit(exitCode); }); +} + +function fail(error: unknown): void { + if (stopping) return; + const message: ApiChildMessage = { type: "error", provider, port, ...getSafeApiStartupError(error) }; + if (!process.connected) { shutdown(1); return; } + try { process.send!(message, () => shutdown(1)); } + catch { shutdown(1); } +} + +process.on("message", (message: unknown) => { + if (message && typeof message === "object" && (message as { type?: unknown }).type === "stop") shutdown(); +}); +process.on("disconnect", () => shutdown()); +process.on("SIGTERM", () => shutdown()); +process.on("SIGINT", () => shutdown()); +process.on("uncaughtException", fail); +process.on("unhandledRejection", fail); + +startEmbeddedApi(provider, port).then((runtime) => { + server = runtime.server; + if (stopping || !process.connected) { shutdown(); return; } + server?.on("error", fail); + const message: ApiChildMessage = { type: "ready", provider, port }; + try { process.send!(message, (error) => { if (error) shutdown(1); }); } + catch { shutdown(1); } +}).catch(fail); diff --git a/src/music/api-server-runtime.test.ts b/src/music/api-server-runtime.test.ts new file mode 100644 index 0000000..5d90f92 --- /dev/null +++ b/src/music/api-server-runtime.test.ts @@ -0,0 +1,86 @@ +import { EventEmitter } from "node:events"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { closeEmbeddedApi, getSafeApiStartupError, startEmbeddedApi } from "./api-server-runtime.js"; + +const state = vi.hoisted(() => ({ serveNcmApi: vi.fn(), qqExport: null as any, portFree: true })); +vi.mock("NeteaseCloudMusicApi", () => ({ server: { serveNcmApi: state.serveNcmApi } })); +vi.mock("@sansenjian/qq-music-api", () => ({ get default() { return state.qqExport; } })); +vi.mock("node:net", async () => { + const { EventEmitter } = await import("node:events"); + return { default: { createServer: () => { + const probe = new EventEmitter() as EventEmitter & { close(done: () => void): void; listen(): void }; + probe.close = (done) => queueMicrotask(done); + probe.listen = () => queueMicrotask(() => probe.emit(state.portFree ? "listening" : "error")); + return probe; + } } }; +}); + +class FakeServer extends EventEmitter { + listening = false; + close = vi.fn((done: () => void) => { this.listening = false; queueMicrotask(done); return this; }); + closeAllConnections = vi.fn(); +} + +describe("embedded API runtime", () => { + let server: FakeServer; + let listen: ReturnType; + let previousPort: string | undefined; + beforeEach(() => { + previousPort = process.env.PORT; + server = new FakeServer(); + state.portFree = true; + state.serveNcmApi.mockReset(); + state.serveNcmApi.mockResolvedValue({ server }); + listen = vi.fn(() => { queueMicrotask(() => { server.listening = true; server.emit("listening"); }); return server; }); + state.qqExport = { listen }; + }); + afterEach(() => { if (previousPort === undefined) delete process.env.PORT; else process.env.PORT = previousPort; }); + + it("binds NetEase to loopback/configured port without version checks and waits for listening", async () => { + let ready = false; + const starting = startEmbeddedApi("netease", 39218).then((result) => { ready = true; return result; }); + await vi.waitFor(() => expect(server.listenerCount("listening")).toBe(1)); + expect(state.serveNcmApi).toHaveBeenCalledWith({ port: 39218, host: "127.0.0.1", checkVersion: false }); + expect(ready).toBe(false); + server.listening = true; server.emit("listening"); + expect((await starting).server).toBe(server); + }); + it("closes the HTTP server returned by NetEase rather than the Express app", async () => { + server.listening = true; + const runtime = await startEmbeddedApi("netease", 39218); + await closeEmbeddedApi(runtime.server); + expect(server.close).toHaveBeenCalledTimes(1); + expect(server.closeAllConnections).toHaveBeenCalledTimes(1); + }); + it("rejects failed listening and cleans up the startup handle", async () => { + const starting = startEmbeddedApi("netease", 39218); + const rejected = expect(starting).rejects.toMatchObject({ code: "EADDRINUSE" }); + await vi.waitFor(() => expect(server.listenerCount("error")).toBe(1)); + server.emit("error", Object.assign(new Error("bind failure"), { code: "EADDRINUSE" })); + await rejected; + expect(server.close).toHaveBeenCalledTimes(1); + }); + it("binds QQ to its configured loopback port and restores injected PORT", async () => { + process.env.PORT = "39999"; + expect((await startEmbeddedApi("qq", 39217)).server).toBe(server); + expect(listen).toHaveBeenCalledWith(39217, "127.0.0.1"); + expect(process.env.PORT).toBe("39999"); + }); + it("restores an absent PORT and supports the legacy nested export", async () => { + delete process.env.PORT; state.qqExport = { default: { listen } }; + await startEmbeddedApi("qq", 39217); + expect(process.env.PORT).toBeUndefined(); + expect(listen).toHaveBeenCalledWith(39217, "127.0.0.1"); + }); + it("reuses a legacy module that auto-started on import without a duplicate listen", async () => { + state.portFree = false; + expect(await startEmbeddedApi("qq", 39217)).toEqual({ server: null }); + expect(listen).not.toHaveBeenCalled(); + }); + it("never reflects arbitrary startup message, stack or code values", () => { + expect(getSafeApiStartupError({ message: "synthetic-credential", stack: "synthetic-credential", code: "synthetic-credential" })).toEqual({ category: "startup" }); + expect(getSafeApiStartupError({ code: "ERR_REQUIRE_ESM", message: "synthetic-credential" })).toEqual({ category: "esm", code: "ERR_REQUIRE_ESM" }); + expect(getSafeApiStartupError({ code: "EBADENGINE" })).toEqual({ category: "node-engine", code: "EBADENGINE" }); + expect(getSafeApiStartupError({ code: "EADDRINUSE" })).toEqual({ category: "port-in-use", code: "EADDRINUSE" }); + }); +}); diff --git a/src/music/api-server-runtime.ts b/src/music/api-server-runtime.ts new file mode 100644 index 0000000..351cb78 --- /dev/null +++ b/src/music/api-server-runtime.ts @@ -0,0 +1,92 @@ +import net from "node:net"; +import type { Server } from "node:http"; + +export type ApiProvider = "netease" | "qq"; +export type ApiStartupCategory = "esm" | "node-engine" | "port-in-use" | "startup" | "timeout" | "cancelled"; +export interface SafeApiStartupError { category: ApiStartupCategory; code?: string } +export type ApiChildMessage = + | { type: "ready"; provider: ApiProvider; port: number } + | ({ type: "error"; provider: ApiProvider; port: number } & SafeApiStartupError); + +const SAFE_ERROR_CODES = new Set(["ERR_REQUIRE_ESM", "EBADENGINE", "EADDRINUSE", "EACCES", "ENOENT", "MODULE_NOT_FOUND", "ERR_MODULE_NOT_FOUND"]); +export function safeApiErrorCode(code: unknown): string | undefined { + return typeof code === "string" && SAFE_ERROR_CODES.has(code) ? code : undefined; +} + +/** Classification may inspect a message locally, but IPC never contains it. */ +export function getSafeApiStartupError(err: unknown): SafeApiStartupError { + const error = (err ?? {}) as { code?: unknown; message?: unknown }; + const code = safeApiErrorCode(error.code); + const message = typeof error.message === "string" ? error.message : ""; + let category: ApiStartupCategory = "startup"; + if (code === "ERR_REQUIRE_ESM" || /ERR_REQUIRE_ESM|require\(\) of ES ?Module/i.test(message)) category = "esm"; + else if (code === "EBADENGINE" || /Unsupported engine|EBADENGINE|requires Node|Node\.js version/i.test(message)) category = "node-engine"; + else if (code === "EADDRINUSE") category = "port-in-use"; + return code ? { category, code } : { category }; +} + +export function isApiPortFree(port: number): Promise { + return new Promise((resolve) => { + const server = net.createServer(); + server.once("error", () => server.close(() => resolve(false))); + server.once("listening", () => server.close(() => resolve(true))); + server.listen(port, "127.0.0.1"); + }); +} + +function waitForListening(server: Server): Promise { + if (server.listening) return Promise.resolve(); + return new Promise((resolve, reject) => { + const ready = () => { cleanup(); resolve(); }; + const failed = (error: Error) => { cleanup(); reject(error); }; + const cleanup = () => { server.off("listening", ready); server.off("error", failed); }; + server.once("listening", ready); + server.once("error", failed); + }); +} + +export async function closeEmbeddedApi(server: Server | null): Promise { + if (!server) return; + await new Promise((resolve) => { + try { + server.close(() => resolve()); + server.closeAllConnections?.(); + } catch { resolve(); } + }); +} + +/** Only call in the isolated child: these dependencies write raw request URLs + * and response cookies directly to console, outside the bot's logger. */ +export async function startEmbeddedApi(provider: ApiProvider, port: number): Promise<{ server: Server | null }> { + let server: Server | null = null; + try { + if (provider === "netease") { + const imported = await import("NeteaseCloudMusicApi") as any; + const api = imported.server ?? imported.default?.server; + const app = await api.serveNcmApi({ port, host: "127.0.0.1", checkVersion: false }); + server = app.server; + if (!server) throw new Error("NetEase API did not expose its HTTP server"); + } else { + const previousPort = process.env.PORT; + process.env.PORT = String(port); + let imported: any; + try { imported = await import("@sansenjian/qq-music-api"); } + finally { + if (previousPort === undefined) delete process.env.PORT; + else process.env.PORT = previousPort; + } + const candidate = imported.default ?? imported; + const app = typeof candidate.listen === "function" ? candidate : candidate.default; + if (!app || typeof app.listen !== "function") throw new Error("QQ API did not expose a Koa app"); + // Historical packages listened during import. Their listener remains + // owned by this child and closes when the child exits. + if (!(await isApiPortFree(port))) return { server: null }; + server = app.listen(port, "127.0.0.1"); + } + await waitForListening(server!); + return { server }; + } catch (error) { + await closeEmbeddedApi(server); + throw error; + } +} diff --git a/src/music/api-server.test.ts b/src/music/api-server.test.ts index 6b8067d..3f7b038 100644 --- a/src/music/api-server.test.ts +++ b/src/music/api-server.test.ts @@ -1,133 +1,191 @@ -import { describe, it, expect, vi, beforeEach } from "vitest"; +import { EventEmitter } from "node:events"; +import type { ChildProcess } from "node:child_process"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { createApiServerManager, describeQqApiStartupError } from "./api-server.js"; import type { Logger } from "../logger.js"; -// Record every listen() the QQ sidecar makes so we can assert it is always -// pinned to the configured port (regression coverage for issue #122). -const mockState = vi.hoisted(() => ({ - listenCalls: [] as Array<{ port: number; host: string }>, -})); - -vi.mock("@sansenjian/qq-music-api", () => { - const app = { - listen(port: number, host: string, cb?: () => void) { - mockState.listenCalls.push({ port, host }); - const server = { - address: () => ({ port, address: host, family: "IPv4" as const }), - on() { - return server; - }, - close(done?: () => void) { - done?.(); - }, - }; - // Real net/Koa fire the listening callback on a later tick, after the - // caller has captured the returned server handle. - if (cb) setImmediate(cb); - return server; - }, - }; - return { default: app }; +const state = vi.hoisted(() => ({ fork: vi.fn(), probes: [] as EventEmitter[], probeAutomatically: true, portFree: true, directImports: 0 })); +vi.mock("node:child_process", () => ({ fork: state.fork })); +vi.mock("node:net", async () => { + const { EventEmitter } = await import("node:events"); + return { default: { createServer: () => { + const probe = new EventEmitter() as EventEmitter & { close(done: () => void): void; listen(): void }; + probe.close = (done) => queueMicrotask(done); + probe.listen = () => { + state.probes.push(probe); + if (state.probeAutomatically) queueMicrotask(() => probe.emit(state.portFree ? "listening" : "error")); + }; + return probe; + } } }; }); +vi.mock("@sansenjian/qq-music-api", () => { + state.directImports++; + return { default: { listen: () => { throw new Error("sidecar imported in parent"); } } }; +}); +vi.mock("NeteaseCloudMusicApi", () => { + state.directImports++; + return { server: { serveNcmApi: () => { throw new Error("sidecar imported in parent"); } } }; +}); + +class FakeChild extends EventEmitter { + connected = true; + exitOnStop = true; + send = vi.fn((message: { type: string }) => { + if (message.type === "stop" && this.exitOnStop) queueMicrotask(() => this.finish(0, null)); + return true; + }); + kill = vi.fn((signal: string = "SIGTERM") => { queueMicrotask(() => this.finish(null, signal)); return true; }); + finish(code: number | null, signal: string | null) { this.connected = false; this.emit("exit", code, signal); } +} describe("describeQqApiStartupError", () => { - it("flags ERR_REQUIRE_ESM by error code with version-pin guidance", () => { - const hint = describeQqApiStartupError({ code: "ERR_REQUIRE_ESM", message: "..." }); - expect(hint).toMatch(/ERR_REQUIRE_ESM/); - expect(hint).toMatch(/~2\.4\.0/); - expect(hint).toMatch(/~2\.2\.10/); + it("retains ESM diagnostics by code and message", () => { + expect(describeQqApiStartupError({ code: "ERR_REQUIRE_ESM" })).toMatch(/~2\.4\.0/); + expect(describeQqApiStartupError(new Error("require() of ES Module is unsupported"))).toMatch(/ERR_REQUIRE_ESM/); }); - - it("flags ERR_REQUIRE_ESM by message when the code is absent", () => { - const hint = describeQqApiStartupError( - new Error("require() of ES Module .../@sansenjian/qq-music-api/dist/index.js not supported") - ); - expect(hint).toMatch(/incompatible @sansenjian\/qq-music-api/); - }); - - it("flags a Node engine mismatch with a Node-upgrade hint", () => { - const hint = describeQqApiStartupError(new Error("Unsupported engine: requires Node >=20.17")); - expect(hint).toMatch(/Node >=20\.17/); - expect(hint).toMatch(/~2\.2\.10/); - }); - - it("returns null for an unrelated startup error (falls back to the generic warning)", () => { - expect(describeQqApiStartupError(new Error("EADDRINUSE: port in use"))).toBeNull(); - expect(describeQqApiStartupError(undefined)).toBeNull(); - expect(describeQqApiStartupError(null)).toBeNull(); + it("retains engine diagnostics and ignores unrelated failures", () => { + expect(describeQqApiStartupError(new Error("Unsupported engine: requires Node >=20.17"))).toMatch(/Node >=20\.17/); + expect(describeQqApiStartupError(new Error("EADDRINUSE"))).toBeNull(); }); }); -// Regression coverage for issue #122: the QQ Music API sidecar must listen on -// the same port the client base URL targets (config.qqMusicApiPort). A stale -// build once bound 3300 while the client requested 3200, silently breaking the -// QQ login QR / search flow with ECONNREFUSED on 127.0.0.1:3200. -describe("createApiServerManager — QQ sidecar port binding", () => { - const noopLogger = { - info() {}, - warn() {}, - error() {}, - debug() {}, - trace() {}, - fatal() {}, - } as unknown as Logger; - +describe("embedded API child lifecycle", () => { + let children: FakeChild[]; + let logger: Logger; + let manager: ReturnType; + let automaticReady: boolean; + const options = { neteasePort: 39218, qqMusicPort: 39217, neteaseEnabled: true, qqEnabled: true }; + const flush = async () => { for (let i = 0; i < 12; i++) await Promise.resolve(); }; beforeEach(() => { - mockState.listenCalls = []; + vi.useRealTimers(); children = []; automaticReady = true; + state.probes = []; state.portFree = true; state.probeAutomatically = true; state.fork.mockReset(); + state.fork.mockImplementation((_entry: string, args: string[]) => { + const child = new FakeChild(); children.push(child); + if (automaticReady) queueMicrotask(() => child.emit("message", { type: "ready", provider: args[0], port: Number(args[1]) })); + return child as unknown as ChildProcess; + }); + logger = { info: vi.fn(), warn: vi.fn(), error: vi.fn() } as unknown as Logger; + manager = createApiServerManager(options, logger); }); + afterEach(async () => { manager.stop(); await flush(); vi.useRealTimers(); }); - it("listens on the configured qqMusicPort and exposes a matching base URL", async () => { - const port = 39217; // uncommon port to avoid clashing with a real instance - const manager = createApiServerManager( - { neteasePort: 39218, qqMusicPort: port, neteaseEnabled: false, qqEnabled: true }, - noopLogger - ); + it("isolates both APIs with ignored stdio, configured ports and IPC", async () => { await manager.start(); - manager.stop(); - - expect(manager.getQQMusicBaseUrl()).toBe(`http://127.0.0.1:${port}`); - expect(mockState.listenCalls).toEqual([{ port, host: "127.0.0.1" }]); + expect(state.fork).toHaveBeenCalledTimes(2); + expect(state.fork.mock.calls.map((call) => call[1])).toEqual([["netease", "39218"], ["qq", "39217"]]); + for (const call of state.fork.mock.calls) { + expect(String(call[0])).toMatch(/api-server-child\.ts$/); + expect(call[2].stdio).toEqual(["ignore", "ignore", "ignore", "ipc"]); + expect(call[2].execArgv).not.toContain("--eval"); + expect(call[2].execArgv).not.toContain("--input-type=module"); + } + expect(state.directImports).toBe(0); + expect(manager.getNeteaseBaseUrl()).toBe("http://127.0.0.1:39218"); + expect(manager.getQQMusicBaseUrl()).toBe("http://127.0.0.1:39217"); }); - - it("follows qqMusicPort — not an injected PORT — and restores PORT afterwards", async () => { - const port = 39219; - const previous = process.env.PORT; - // Simulate a hosting platform / compose file injecting a stray PORT that - // must NOT leak into the QQ sidecar's chosen port. - process.env.PORT = "39999"; - const manager = createApiServerManager( - { neteasePort: 39220, qqMusicPort: port, neteaseEnabled: false, qqEnabled: true }, - noopLogger - ); + it("preserves provider gating and externally bound port reuse", async () => { + manager = createApiServerManager({ ...options, neteaseEnabled: false, qqEnabled: false }, logger); + await manager.start(); expect(state.probes).toHaveLength(0); expect(state.fork).not.toHaveBeenCalled(); + state.portFree = false; + manager = createApiServerManager({ ...options, neteaseEnabled: false }, logger); + await manager.start(); expect(state.fork).not.toHaveBeenCalled(); + expect(logger.info).toHaveBeenCalledWith({ port: 39217 }, expect.stringContaining("reusing")); + }); + it("inherits tsx loader arguments without unrelated parent runner flags", async () => { + const previous = process.execArgv; + process.execArgv = ["--require", "C:\\app\\node_modules\\tsx\\dist\\preflight.cjs", "--import", "file:///app/node_modules/tsx/dist/loader.mjs", "--eval", "synthetic-evaluation", "--conditions", "vitest", "--input-type=module", "--inspect"]; try { await manager.start(); - // The sidecar follows qqMusicPort, never the injected PORT. - expect(mockState.listenCalls).toEqual([{ port, host: "127.0.0.1" }]); - // The injected PORT is restored so nothing else in the process is affected. - expect(process.env.PORT).toBe("39999"); - } finally { - manager.stop(); - if (previous === undefined) delete process.env.PORT; - else process.env.PORT = previous; - } + expect(state.fork.mock.calls[0][2].execArgv).toEqual(process.execArgv.slice(0, 4)); + } finally { process.execArgv = previous; } }); - - it("leaves an absent PORT env unset after importing the sidecar", async () => { - const port = 39221; - const previous = process.env.PORT; - delete process.env.PORT; - const manager = createApiServerManager( - { neteasePort: 39222, qqMusicPort: port, neteaseEnabled: false, qqEnabled: true }, - noopLogger - ); - try { - await manager.start(); - // Was unset before importing — must be unset again, no leaked override. - expect(process.env.PORT).toBeUndefined(); - } finally { - manager.stop(); - if (previous === undefined) delete process.env.PORT; - else process.env.PORT = previous; + it("does not duplicate concurrent or repeated starts", async () => { + await Promise.all([manager.start(), manager.start()]); await manager.start(); + expect(state.fork).toHaveBeenCalledTimes(2); + }); + it("fences a stop during pending port preflight", async () => { + state.probeAutomatically = false; + const starting = manager.start(); await flush(); manager.stop(); + state.probes[0].emit("listening"); await starting; + expect(state.fork).not.toHaveBeenCalled(); + }); + it("cancels a pending handshake and ignores its late ready", async () => { + automaticReady = false; + const starting = manager.start(); await flush(); expect(children).toHaveLength(1); + manager.stop(); children[0].emit("message", { type: "ready", provider: "netease", port: 39218 }); await starting; + expect(children[0].send).toHaveBeenCalledWith({ type: "stop" }, expect.any(Function)); + expect(state.fork).toHaveBeenCalledTimes(1); + expect(logger.info).not.toHaveBeenCalledWith({ port: 39218 }, "NetEase Cloud Music API started"); + }); + it("waits for a cancelled preflight to release its probe before restart", async () => { + state.probeAutomatically = false; + const first = manager.start(); await flush(); manager.stop(); + const restarting = manager.start(); await flush(); + expect(state.probes).toHaveLength(1); + state.probeAutomatically = true; state.probes[0].emit("listening"); + await Promise.all([first, restarting]); + expect(state.fork).toHaveBeenCalledTimes(2); + }); + it("waits for old children to exit before restart", async () => { + await manager.start(); children.forEach((child) => { child.exitOnStop = false; }); manager.stop(); + const restarting = manager.start(); await flush(); expect(state.fork).toHaveBeenCalledTimes(2); + children.slice(0, 2).forEach((child) => child.finish(0, null)); await restarting; + expect(state.fork).toHaveBeenCalledTimes(4); + }); + it("reports unexpected post-ready exits with safe fields", async () => { + await manager.start(); children[1].finish(7, "SIGTERM"); + expect(logger.error).toHaveBeenCalledWith({ provider: "qq", port: 39217, code: 7, signal: "SIGTERM" }, expect.stringContaining("exited unexpectedly")); + }); + it("retains static QQ diagnostics and discards arbitrary IPC fields", async () => { + automaticReady = false; manager = createApiServerManager({ ...options, neteaseEnabled: false }, logger); + const starting = manager.start(); await flush(); + children[0].emit("message", { type: "error", provider: "qq", port: 39217, category: "esm", code: "ERR_REQUIRE_ESM", message: "synthetic-credential", stack: "synthetic-credential" }); await starting; + expect(logger.error).toHaveBeenCalledWith({ provider: "qq", port: 39217, category: "esm", code: "ERR_REQUIRE_ESM" }, expect.stringContaining("ERR_REQUIRE_ESM")); + expect(JSON.stringify([...(logger.error as ReturnType).mock.calls, ...(logger.warn as ReturnType).mock.calls])).not.toContain("synthetic-credential"); + expect(children[0].send).toHaveBeenCalledWith({ type: "stop" }, expect.any(Function)); + }); + it("ignores a ready message for a different provider or port", async () => { + automaticReady = false; manager = createApiServerManager({ ...options, neteaseEnabled: false }, logger); + const starting = manager.start(); await flush(); + children[0].emit("message", { type: "ready", provider: "netease", port: 39217 }); + children[0].emit("message", { type: "ready", provider: "qq", port: 39999 }); + await flush(); + expect(logger.info).not.toHaveBeenCalledWith({ port: 39217 }, "QQ Music API started"); + children[0].emit("message", { type: "ready", provider: "qq", port: 39217 }); await starting; + expect(logger.info).toHaveBeenCalledWith({ port: 39217 }, "QQ Music API started"); + }); + it("cleans up a failed fork that closes without an exit event", async () => { + automaticReady = false; manager = createApiServerManager({ ...options, neteaseEnabled: false }, logger); + const starting = manager.start(); await flush(); + children[0].emit("error", Object.assign(new Error("synthetic-credential"), { code: "ENOENT" })); + children[0].emit("close", null, null); await starting; + expect(logger.error).toHaveBeenCalledWith({ provider: "qq", port: 39217, category: "startup", code: "ENOENT" }, expect.stringContaining("start")); + automaticReady = true; await manager.start(); + expect(state.fork).toHaveBeenCalledTimes(2); + }); + it("times out and terminates a silent child", async () => { + vi.useFakeTimers(); automaticReady = false; manager = createApiServerManager({ ...options, neteaseEnabled: false }, logger); + const starting = manager.start(); await flush(); await vi.advanceTimersByTimeAsync(30000); await starting; + expect(logger.error).toHaveBeenCalledWith({ provider: "qq", port: 39217, category: "timeout" }, expect.stringContaining("start")); + expect(children[0].send).toHaveBeenCalledWith({ type: "stop" }, expect.any(Function)); + }); + it("forces shutdown if a child ignores stop", async () => { + vi.useFakeTimers(); await manager.start(); children.forEach((child) => { child.exitOnStop = false; }); manager.stop(); + await vi.advanceTimersByTimeAsync(2000); + expect(children.every((child) => child.kill.mock.calls.length > 0)).toBe(true); + expect(logger.error).not.toHaveBeenCalled(); + }); + it("escalates to SIGKILL if stop and SIGTERM are ignored", async () => { + vi.useFakeTimers(); await manager.start(); + for (const child of children) { + child.exitOnStop = false; + child.kill.mockImplementation((signal: string = "SIGTERM") => { + if (signal === "SIGKILL") queueMicrotask(() => child.finish(null, signal)); + return true; + }); } + manager.stop(); await vi.advanceTimersByTimeAsync(2000); + expect(children.every((child) => child.kill.mock.calls.some(([signal]) => signal === "SIGKILL"))).toBe(true); + expect(logger.error).not.toHaveBeenCalled(); }); }); diff --git a/src/music/api-server.ts b/src/music/api-server.ts index 1374268..c8a6cc8 100644 --- a/src/music/api-server.ts +++ b/src/music/api-server.ts @@ -1,16 +1,13 @@ -import net from "node:net"; +import { fork, type ChildProcess } from "node:child_process"; import type { Logger } from "../logger.js"; -import type { Server } from "node:http"; +import { getSafeApiStartupError, isApiPortFree, safeApiErrorCode, type ApiProvider, type SafeApiStartupError } from "./api-server-runtime.js"; export interface ApiServerOptions { neteasePort: number; qqMusicPort: number; - /** Provider gating (#enabledProviders): when false, the corresponding - * embedded sidecar API server is never started and its port never bound. */ neteaseEnabled?: boolean; qqEnabled?: boolean; } - export interface ApiServerManager { start(): Promise; stop(): void; @@ -18,196 +15,181 @@ export interface ApiServerManager { getQQMusicBaseUrl(): string; } -/** - * Classify a QQ Music API (@sansenjian/qq-music-api) startup failure into - * actionable operator guidance, or null when it isn't a recognised - * dependency/runtime mismatch. Exported for testing. - * - * Background: the package became ESM in 2.3.x. A loose `^` range could pull an - * ESM-only build (2.3.0/2.3.1) that throws ERR_REQUIRE_ESM, or a 2.4.x build - * that needs Node >=20.17 — either way the embedded server never binds, so - * every QQ request fails downstream with ECONNREFUSED on the API port. - */ export function describeQqApiStartupError(err: unknown): string | null { - const e = (err ?? {}) as { code?: string; message?: string }; - const code = String(e.code ?? ""); - const msg = String(e.message ?? ""); - if (code === "ERR_REQUIRE_ESM" || /ERR_REQUIRE_ESM|require\(\) of ES ?Module/i.test(msg)) { - return ( - "an incompatible @sansenjian/qq-music-api build is installed (ERR_REQUIRE_ESM). " + - "Pin it to ~2.4.0 (needs Node >=20.17) or ~2.2.10 in package.json, then reinstall" - ); - } - if (/Unsupported engine|EBADENGINE|requires Node|Node\.js version/i.test(msg)) { - return "@sansenjian/qq-music-api 2.4.x requires Node >=20.17 (or >=22.9) — upgrade Node, or pin the package to ~2.2.10"; - } + const { category } = getSafeApiStartupError(err); + if (category === "esm") return "an incompatible @sansenjian/qq-music-api build is installed (ERR_REQUIRE_ESM). Pin it to ~2.4.0 (needs Node >=20.17) or ~2.2.10 in package.json, then reinstall"; + if (category === "node-engine") return "@sansenjian/qq-music-api 2.4.x requires Node >=20.17 (or >=22.9) — upgrade Node, or pin the package to ~2.2.10"; return null; } -function isPortFree(port: number): Promise { - return new Promise((resolve) => { - const server = net.createServer(); - server.once("error", () => { - server.close(() => resolve(false)); - }); - server.once("listening", () => { - server.close(() => resolve(true)); - }); - server.listen(port, "127.0.0.1"); - }); +/** Carry only tsx loader arguments into a source child. CLI evaluation, + * inspector and test-runner flags have unrelated meanings in a fork. */ +function childExecArgv(source: boolean): string[] { + if (!source) return []; + const args: string[] = []; + for (let i = 0; i < process.execArgv.length; i++) { + const arg = process.execArgv[i]; + if (arg === "--import" || arg === "--require" || arg === "-r") { + const value = process.execArgv[++i]; + if (value && (value === "tsx" || /[/\\]tsx[/\\]/.test(value))) args.push(arg, value); + } else if (arg.startsWith("--import=") && (arg === "--import=tsx" || /[/\\]tsx[/\\]/.test(arg))) args.push(arg); + } + return args.length ? args : ["--import", "tsx"]; } -export function createApiServerManager( - options: ApiServerOptions, - logger: Logger -): ApiServerManager { - let neteaseServer: Server | null = null; - let qqMusicServer: Server | null = null; +class StartupFailure extends Error { + constructor(readonly details: SafeApiStartupError) { super("Embedded music API startup failed"); } +} +interface ManagedChild { + child: ChildProcess; + stop(): Promise; +} +const STARTUP_TIMEOUT_MS = 15000; +const ERROR_CATEGORIES = new Set(["esm", "node-engine", "port-in-use", "startup"]); - const neteaseBaseUrl = `http://127.0.0.1:${options.neteasePort}`; - const qqMusicBaseUrl = `http://127.0.0.1:${options.qqMusicPort}`; +export function createApiServerManager(options: ApiServerOptions, logger: Logger): ApiServerManager { + const children = new Map(); + let generation = 0; + let starting: Promise | null = null; + let stopping: Promise = Promise.resolve(); + + function launch(provider: ApiProvider, port: number, launchGeneration: number): Promise { + const source = import.meta.url.endsWith(".ts"); + const entry = new URL(source ? "./api-server-child.ts" : "./api-server-child.js", import.meta.url); + const child = fork(entry, [provider, String(port)], { + stdio: ["ignore", "ignore", "ignore", "ipc"], + execArgv: childExecArgv(source), + }); + let ready = false; + let expectedExit = false; + let settled = false; + let hasExited = false; + let startupTimer: ReturnType; + let terminateTimer: ReturnType | undefined; + let killTimer: ReturnType | undefined; + let resolveExit!: () => void; + const exited = new Promise((resolve) => { resolveExit = resolve; }); + let resolveStart!: () => void; + let rejectStart!: (error: StartupFailure) => void; + const started = new Promise((resolve, reject) => { resolveStart = resolve; rejectStart = reject; }); + const settle = (error?: SafeApiStartupError) => { + if (settled) return; + settled = true; + clearTimeout(startupTimer); + if (error) rejectStart(new StartupFailure(error)); else resolveStart(); + }; + const record: ManagedChild = { + child, + stop() { + if (expectedExit) return exited; + expectedExit = true; + settle({ category: "cancelled" }); + if (children.get(provider) === record) children.delete(provider); + stopping = Promise.all([stopping, exited]).then(() => {}); + if (hasExited) return exited; + try { + if (child.connected) child.send({ type: "stop" }, (error) => { if (error) child.kill("SIGTERM"); }); + else child.kill("SIGTERM"); + } catch { child.kill("SIGTERM"); } + terminateTimer = setTimeout(() => child.kill("SIGTERM"), 1000); + killTimer = setTimeout(() => child.kill("SIGKILL"), 2000); + terminateTimer.unref(); killTimer.unref(); + return exited; + }, + }; + children.set(provider, record); + child.on("message", (message: unknown) => { + if (!message || typeof message !== "object" || expectedExit || launchGeneration !== generation) return; + const data = message as Record; + if (data.provider !== provider || data.port !== port) return; + if (data.type === "ready") { ready = true; settle(); } + else if (data.type === "error" && typeof data.category === "string" && ERROR_CATEGORIES.has(data.category)) { + const code = safeApiErrorCode(data.code); + const details: SafeApiStartupError = { category: data.category as SafeApiStartupError["category"], ...(code ? { code } : {}) }; + if (!ready) settle(details); + else logger.error({ provider, port, ...details }, "Embedded music API reported a runtime failure"); + void record.stop(); + } + }); + child.on("error", (error) => { + if (expectedExit) return; + const details = getSafeApiStartupError(error); + if (!ready) settle(details); + else logger.error({ provider, port, ...details }, "Embedded music API child failed"); + void record.stop(); + }); + const onExit = (code: number | null, signal: NodeJS.Signals | null) => { + if (hasExited) return; + hasExited = true; + clearTimeout(startupTimer); clearTimeout(terminateTimer); clearTimeout(killTimer); + if (children.get(provider) === record) children.delete(provider); + if (!expectedExit) { + if (ready) logger.error({ provider, port, code, signal }, "Embedded music API exited unexpectedly"); + else settle({ category: "startup" }); + } + resolveExit(); + }; + child.once("exit", onExit); + // A failed fork emits close without exit. + child.once("close", onExit); + startupTimer = setTimeout(() => { settle({ category: "timeout" }); void record.stop(); }, STARTUP_TIMEOUT_MS); + return started; + } return { - async start(): Promise { - // Provider gating: with the jellyfin-only default config neither legacy - // sidecar starts, so ports 3001/3200 are never opened. - if (options.neteaseEnabled === false && options.qqEnabled === false) { - logger.info("NetEase/QQ providers disabled — embedded music API servers not started"); - return; - } - logger.info("Starting embedded music API servers..."); - - // Start NetEase Cloud Music API - if (options.neteaseEnabled !== false) { - try { - const portFree = await isPortFree(options.neteasePort); - if (!portFree) { - logger.info( - { port: options.neteasePort }, - "NetEase API port already in use — reusing existing instance" - ); - } else { - const ncmModule = await import("NeteaseCloudMusicApi") as any; - const serverObj = ncmModule.server ?? ncmModule.default?.server; - const app = await serverObj.serveNcmApi({ port: options.neteasePort }); - neteaseServer = app; - logger.info( - { port: options.neteasePort }, - "NetEase Cloud Music API started" - ); - } - } catch (err) { - logger.error({ err }, "Failed to start NetEase Cloud Music API"); + start(): Promise { + if (starting) return starting; + const startGeneration = generation; + const run = async () => { + await stopping; + if (startGeneration !== generation) return; + if (options.neteaseEnabled === false && options.qqEnabled === false) { + logger.info("NetEase/QQ providers disabled — embedded music API servers not started"); return; } - } - - // Start QQ Music API. Older versions auto-started on import; the - // current fork (2.2.11+) only listens when run as `require.main`, - // so we explicitly call .listen() on the imported Koa app and keep - // the server handle for clean shutdown. - if (options.qqEnabled === false) return; - try { - const portFree = await isPortFree(options.qqMusicPort); - if (!portFree) { - logger.info( - { port: options.qqMusicPort }, - "QQ Music API port already in use — reusing existing instance" - ); - } else { - // Pin the upstream server to the configured port before importing. - // The package derives its default port from process.env.PORT (falling - // back to 3200) and, in some historical versions, auto-started that - // server as an import side effect. Aligning PORT with qqMusicApiPort - // guarantees the sidecar can never bind a different port than the one - // the client base URL (getQQMusicBaseUrl) targets — the root cause of - // issue #122, where an old build listened on 3300 while the client - // requested 3200. Restore the previous value right after import so we - // never leak the override into the rest of the process (e.g. the web - // server or the NetEase sidecar, which also read PORT as a fallback). - const prevPortEnv = process.env.PORT; - process.env.PORT = String(options.qqMusicPort); - let qqModule: any; + logger.info("Starting embedded music API servers..."); + const providers: Array<{ provider: ApiProvider; port: number; enabled: boolean; name: string }> = [ + { provider: "netease", port: options.neteasePort, enabled: options.neteaseEnabled !== false, name: "NetEase Cloud Music" }, + { provider: "qq", port: options.qqMusicPort, enabled: options.qqEnabled !== false, name: "QQ Music" }, + ]; + for (const { provider, port, enabled, name } of providers) { + if (startGeneration !== generation) return; + if (!enabled || children.has(provider)) continue; try { - qqModule = (await import("@sansenjian/qq-music-api")) as any; - } finally { - if (prevPortEnv === undefined) delete process.env.PORT; - else process.env.PORT = prevPortEnv; - } - // The module's export structure varies between versions: - // 2.2.11+: default → Koa app (has .listen) - // 2.2.10: default → wrapper object whose .default is the Koa app - // older: module itself may be the Koa app - const candidate = qqModule.default ?? qqModule; - const koaApp = typeof candidate.listen === "function" - ? candidate - : candidate.default ?? null; - if (koaApp && typeof koaApp.listen === "function") { - // A version that auto-started on import has already bound the - // configured port (thanks to the PORT alignment above); reuse it - // rather than racing a second listen that would fail EADDRINUSE. - const stillFree = await isPortFree(options.qqMusicPort); - if (!stillFree) { - logger.info( - { port: options.qqMusicPort }, - "QQ Music API already listening on the configured port (auto-started on import) — reusing embedded instance" - ); - } else { - qqMusicServer = await new Promise((resolve, reject) => { - const srv = koaApp.listen(options.qqMusicPort, "127.0.0.1", () => - resolve(srv) - ); - srv.on("error", reject); - }); - // Log the port actually bound (read from the socket) rather than - // the requested one, so operators can spot a mismatch in the logs. - const addr = qqMusicServer.address(); - const boundPort = - addr && typeof addr === "object" && addr !== null - ? addr.port - : options.qqMusicPort; - logger.info( - { port: boundPort }, - "QQ Music API started" - ); + const free = await isApiPortFree(port); + if (startGeneration !== generation) return; + if (!free) { + logger.info({ port }, `${provider === "netease" ? "NetEase" : "QQ Music"} API port already in use — reusing existing instance`); + continue; } - } else { - logger.warn("QQ Music API module does not expose a Koa app"); + await launch(provider, port, startGeneration); + if (startGeneration !== generation) return; + logger.info({ port }, `${name} API started`); + } catch (error) { + if (startGeneration !== generation) return; + const details = error instanceof StartupFailure ? error.details : getSafeApiStartupError(error); + if (details.category === "cancelled") return; + const hint = provider === "qq" ? describeQqApiStartupError(details.category === "esm" ? { code: "ERR_REQUIRE_ESM" } : details.category === "node-engine" ? { code: "EBADENGINE" } : {}) : null; + logger.error({ provider, port, ...details }, hint ? `QQ Music API failed to start — ${hint}. QQ features (search/play/login) will be unavailable until fixed; port ${port} is down.` : `Failed to start ${name} API`); } } - } catch (err) { - const hint = describeQqApiStartupError(err); - if (hint) { - logger.error( - { err }, - `QQ Music API failed to start — ${hint}. QQ features (search/play/login) will be unavailable until fixed; port ${options.qqMusicPort} is down.` - ); - } else { - logger.warn( - { err }, - "QQ Music API not available — QQ Music features may be limited" - ); - } - } + }; + const promise = run(); + starting = promise; + void promise.finally(() => { if (starting === promise) starting = null; }); + return promise; }, - stop(): void { + generation++; + const pendingStart = starting; + starting = null; logger.info("Stopping music API servers"); - if (neteaseServer && typeof (neteaseServer as any).close === "function") { - (neteaseServer as any).close(); - } - neteaseServer = null; - if (qqMusicServer && typeof (qqMusicServer as any).close === "function") { - (qqMusicServer as any).close(); - } - qqMusicServer = null; - }, - - getNeteaseBaseUrl(): string { - return neteaseBaseUrl; - }, - - getQQMusicBaseUrl(): string { - return qqMusicBaseUrl; + const retiring = [...children.values()]; + children.clear(); + // A cancelled preflight still owns a temporary listening socket until + // its callback closes it. Restart must wait for that work as well. + stopping = Promise.all([stopping, pendingStart, ...retiring.map((record) => record.stop())]).then(() => {}); }, + getNeteaseBaseUrl: () => `http://127.0.0.1:${options.neteasePort}`, + getQQMusicBaseUrl: () => `http://127.0.0.1:${options.qqMusicPort}`, }; }