mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-01 12:52:11 +03:00
* fix(plugins): delete the host script synchronously and stop two tests leaking child processes
Three related leaks in the plugin child-process lifecycle, found while tracing 27 node
processes on a developer machine.
1. loader.ts removed the generated omniroute-plugin-host-*.mjs with a fire-and-forget
`rm(...).catch(() => {})`. That unlink loses the race against process exit: test:unit
runs with --test-force-exit, which tears the process down before the promise settles,
so every plugin load leaked one temp .mjs into TMPDIR. Measured at 6 files per
full-suite run, 40 accumulated over a handful of local runs. rmSync closes the race;
the throw stays swallowed because an exception raised from a child "exit" handler
would take the server down, and a leftover temp script would not.
2. plugins-manager-lifecycle.test.ts "activates an installed plugin" called activate() --
which spawns the plugin's child process -- but never deactivate(). deactivate() is the
only path that reaches the loader's cleanup(), so the child outlived the test and its
IPC channel kept the test process's event loop alive.
3. plugins-manager-restart-reload-7806.test.ts simulateRestart() deleted the entry from
loadedPlugins without calling cleanup(), dropping the only handle that can kill child
#1. The reload then spawned child #2, and the finally block's deactivate() could reach
only child #2 -- one dangling child per test. A real restart takes the whole process
tree down, so calling cleanup() here is both the faithful simulation and the fix.
Combined effect: a run without --test-force-exit deadlocks. The test process cannot exit
while its child holds the IPC channel open, and the child waits for messages that never
come. Observed as three plugin hosts alive for 4h51m under a runner that never finished.
Validation (Hard Rule #18, TDD): the new plugins-loader.test.ts case fails against the old
async unlink ("must delete the host script synchronously, not on a later tick") and passes
with rmSync. It redirects TMPDIR/TEMP/TMP to a private directory before counting, because
test:unit runs at --test-concurrency=20 and a concurrent file's host scripts would
otherwise land in the counted directory and flake the assertion.
After: 19/19 pass across the three files, 0 temp scripts created, 0 orphan processes.
tests/unit/build/** 334/334; typecheck:core and eslint clean.
* docs(changelog): add fragment for plugin host script sync delete
138 lines
4.8 KiB
TypeScript
138 lines
4.8 KiB
TypeScript
import { describe, it, beforeEach } from "node:test";
|
|
import assert from "node:assert/strict";
|
|
import { mkdtempSync, writeFileSync, mkdirSync, rmSync } from "node:fs";
|
|
import { join } from "node:path";
|
|
import { tmpdir } from "node:os";
|
|
|
|
const mod = await import("../../src/lib/plugins/manager.ts");
|
|
const db = await import("../../src/lib/db/plugins.ts");
|
|
const { getDbInstance } = await import("../../src/lib/db/core.ts");
|
|
|
|
function makeTmpPlugin(name: string, manifest: Record<string, unknown> = {}) {
|
|
const tmp = mkdtempSync(join(tmpdir(), "mgr-test-"));
|
|
const pluginDir = join(tmp, name);
|
|
mkdirSync(pluginDir, { recursive: true });
|
|
writeFileSync(
|
|
join(pluginDir, "plugin.json"),
|
|
JSON.stringify({ name, version: "1.0.0", ...manifest })
|
|
);
|
|
writeFileSync(
|
|
join(pluginDir, "index.js"),
|
|
`module.exports = { onRequest: async (ctx) => ({ metadata: { banner: "hello" } }) };`
|
|
);
|
|
return pluginDir;
|
|
}
|
|
|
|
function cleanup(name: string) {
|
|
try {
|
|
db.deletePlugin(name);
|
|
} catch {}
|
|
}
|
|
|
|
describe("pluginManager lifecycle", () => {
|
|
const testPlugins: string[] = [];
|
|
|
|
beforeEach(() => {
|
|
// Ensure migrations ran (creates the `plugins` table via migration 076)
|
|
// before any lifecycle call touches it — uses the real migration, not an
|
|
// inline CREATE TABLE, so a missing/renumbered migration fails loudly.
|
|
getDbInstance();
|
|
// Clean up test plugins
|
|
for (const name of testPlugins) {
|
|
try {
|
|
db.deletePlugin(name);
|
|
} catch {}
|
|
}
|
|
testPlugins.length = 0;
|
|
});
|
|
|
|
describe("install", () => {
|
|
it("installs a valid plugin from directory", async () => {
|
|
const dir = makeTmpPlugin("install-test");
|
|
testPlugins.push("install-test");
|
|
try {
|
|
const result = await mod.pluginManager.install(dir);
|
|
assert.ok(result);
|
|
assert.equal(result.name, "install-test");
|
|
const dbRow = db.getPluginByName("install-test");
|
|
assert.ok(dbRow);
|
|
assert.equal(dbRow!.status, "installed");
|
|
} finally {
|
|
rmSync(dir.split("/").slice(0, -1).join("/"), { recursive: true, force: true });
|
|
}
|
|
});
|
|
|
|
it("rejects invalid directory", async () => {
|
|
await assert.rejects(() => mod.pluginManager.install("/nonexistent/path"));
|
|
});
|
|
});
|
|
|
|
describe("activate / deactivate", () => {
|
|
it("activates an installed plugin", async () => {
|
|
const dir = makeTmpPlugin("activate-test");
|
|
testPlugins.push("activate-test");
|
|
try {
|
|
await mod.pluginManager.install(dir);
|
|
await mod.pluginManager.activate("activate-test");
|
|
const dbRow = db.getPluginByName("activate-test");
|
|
assert.equal(dbRow!.status, "active");
|
|
} finally {
|
|
// deactivate() is the only path that reaches the loader's cleanup() and kills
|
|
// the plugin's child process — without it the child outlives the test and its
|
|
// IPC channel keeps this process's event loop alive after the suite finishes.
|
|
await mod.pluginManager.deactivate("activate-test").catch(() => {});
|
|
rmSync(dir.split("/").slice(0, -1).join("/"), { recursive: true, force: true });
|
|
}
|
|
});
|
|
|
|
it("deactivates an active plugin", async () => {
|
|
const dir = makeTmpPlugin("deactivate-test");
|
|
testPlugins.push("deactivate-test");
|
|
try {
|
|
await mod.pluginManager.install(dir);
|
|
await mod.pluginManager.activate("deactivate-test");
|
|
await mod.pluginManager.deactivate("deactivate-test");
|
|
const dbRow = db.getPluginByName("deactivate-test");
|
|
assert.equal(dbRow!.status, "inactive");
|
|
} finally {
|
|
rmSync(dir.split("/").slice(0, -1).join("/"), { recursive: true, force: true });
|
|
}
|
|
});
|
|
|
|
it("throws for unknown plugin", async () => {
|
|
await assert.rejects(() => mod.pluginManager.activate("nonexistent"));
|
|
});
|
|
});
|
|
|
|
describe("uninstall", () => {
|
|
it("removes plugin completely", async () => {
|
|
const dir = makeTmpPlugin("uninstall-test");
|
|
testPlugins.push("uninstall-test");
|
|
try {
|
|
await mod.pluginManager.install(dir);
|
|
await mod.pluginManager.uninstall("uninstall-test");
|
|
const dbRow = db.getPluginByName("uninstall-test");
|
|
assert.equal(dbRow, null);
|
|
} finally {
|
|
rmSync(dir.split("/").slice(0, -1).join("/"), { recursive: true, force: true });
|
|
}
|
|
});
|
|
});
|
|
|
|
describe("getLoaded / listAll / getPlugin", () => {
|
|
it("getLoaded returns undefined for unloaded plugin", () => {
|
|
assert.equal(mod.pluginManager.getLoaded("not-loaded"), undefined);
|
|
});
|
|
|
|
it("listAll returns array", async () => {
|
|
const list = await mod.pluginManager.listAll();
|
|
assert.ok(Array.isArray(list));
|
|
});
|
|
|
|
it("getPlugin returns null for unknown", async () => {
|
|
const result = await mod.pluginManager.getPlugin("nonexistent");
|
|
assert.equal(result, null);
|
|
});
|
|
});
|
|
});
|