mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
refactor(server): migrate plugin jobs off node-cron
The plugin host's job scheduler now uses the cron package (the one @nestjs/schedule wraps), so node-cron and @types/node-cron leave the dependency tree entirely. Verified against both real libraries that the accepted cron grammar is identical — no plugin's declared schedule tightens — and pinned that with an unmocked compat test.
This commit is contained in:
Generated
-18
@@ -7384,13 +7384,6 @@
|
||||
"undici-types": ">=7.24.0 <7.24.7"
|
||||
}
|
||||
},
|
||||
"node_modules/@types/node-cron": {
|
||||
"version": "3.0.11",
|
||||
"resolved": "https://registry.npmjs.org/@types/node-cron/-/node-cron-3.0.11.tgz",
|
||||
"integrity": "sha512-0ikrnug3/IyneSHqCBeslAhlK2aBfYek1fGo4bP4QnZPmiqSGRK+Oy7ZMisLWkesffJvQ1cqAcBnJC+8+nxIAg==",
|
||||
"dev": true,
|
||||
"license": "MIT"
|
||||
},
|
||||
"node_modules/@types/nodemailer": {
|
||||
"version": "8.0.1",
|
||||
"resolved": "https://registry.npmjs.org/@types/nodemailer/-/nodemailer-8.0.1.tgz",
|
||||
@@ -15231,15 +15224,6 @@
|
||||
"node": ">=10"
|
||||
}
|
||||
},
|
||||
"node_modules/node-cron": {
|
||||
"version": "4.4.1",
|
||||
"resolved": "https://registry.npmjs.org/node-cron/-/node-cron-4.4.1.tgz",
|
||||
"integrity": "sha512-1e4Gku1qeKx1ywNnMoA0o5hiZ8iA4klH84TIszTHZAME68xVIjLkiBo5KrtowbcOJn8MQqJG/yWHjC/dFdMWyQ==",
|
||||
"license": "ISC",
|
||||
"engines": {
|
||||
"node": ">=20"
|
||||
}
|
||||
},
|
||||
"node_modules/node-int64": {
|
||||
"version": "0.4.0",
|
||||
"resolved": "https://registry.npmjs.org/node-int64/-/node-int64-0.4.0.tgz",
|
||||
@@ -21187,7 +21171,6 @@
|
||||
"json5": "^2.2.3",
|
||||
"jsonwebtoken": "^9.0.2",
|
||||
"nestjs-zod": "^5.5.0",
|
||||
"node-cron": "^4.2.1",
|
||||
"nodemailer": "^9.0.1",
|
||||
"otplib": "^12.0.1",
|
||||
"pdf-parse": "^2.4.5",
|
||||
@@ -21220,7 +21203,6 @@
|
||||
"@types/jsonwebtoken": "^9.0.10",
|
||||
"@types/multer": "^2.1.0",
|
||||
"@types/node": "^25.5.0",
|
||||
"@types/node-cron": "^3.0.11",
|
||||
"@types/nodemailer": "^8.0.1",
|
||||
"@types/qrcode": "^1.5.5",
|
||||
"@types/semver": "^7.7.1",
|
||||
|
||||
@@ -50,7 +50,6 @@
|
||||
"json5": "^2.2.3",
|
||||
"jsonwebtoken": "^9.0.2",
|
||||
"nestjs-zod": "^5.5.0",
|
||||
"node-cron": "^4.2.1",
|
||||
"nodemailer": "^9.0.1",
|
||||
"otplib": "^12.0.1",
|
||||
"pdf-parse": "^2.4.5",
|
||||
@@ -93,7 +92,6 @@
|
||||
"@types/jsonwebtoken": "^9.0.10",
|
||||
"@types/multer": "^2.1.0",
|
||||
"@types/node": "^25.5.0",
|
||||
"@types/node-cron": "^3.0.11",
|
||||
"@types/nodemailer": "^8.0.1",
|
||||
"@types/qrcode": "^1.5.5",
|
||||
"@types/semver": "^7.7.1",
|
||||
|
||||
@@ -1,11 +1,11 @@
|
||||
import cron, { type ScheduledTask } from 'node-cron';
|
||||
import { CronJob, validateCronExpression } from 'cron';
|
||||
import { JOBS_PERMISSION } from '../protocol/envelope';
|
||||
|
||||
/**
|
||||
* Host-side scheduler for a plugin's declared background jobs (#plugins).
|
||||
*
|
||||
* A plugin declares `jobs` (id + cron `schedule`) in its code; the child reports
|
||||
* them at load. This wires each to node-cron in the HOST and fires
|
||||
* them at load. This wires each to a `cron` CronJob in the HOST and fires
|
||||
* `invoke(id, 'invoke.job', { jobId })` on the tick. Kept deliberately small and
|
||||
* pure so it is unit-testable in isolation of the supervisor.
|
||||
*
|
||||
@@ -16,6 +16,10 @@ import { JOBS_PERMISSION } from '../protocol/envelope';
|
||||
* egress. (3) An invalid cron is skipped (never runs). (4) The tick callback and
|
||||
* `stop()` are wrapped so a throwing job or a stray task can never break the host
|
||||
* or leak across a deactivation.
|
||||
*
|
||||
* The library is `cron` (the one @nestjs/schedule wraps), migrated from
|
||||
* node-cron when the core scheduler retired; tests/unit/plugins/
|
||||
* plugin-jobs.cron-compat.test.ts pins that the accepted grammar did not narrow.
|
||||
*/
|
||||
|
||||
export interface ScheduledJob {
|
||||
@@ -27,25 +31,29 @@ export interface ScheduledJob {
|
||||
export { JOBS_PERMISSION as JOBS_RUN_PERMISSION } from '../protocol/envelope';
|
||||
|
||||
/**
|
||||
* Schedule every valid-cron job (returns the node-cron tasks to keep for teardown).
|
||||
* Schedule every valid-cron job (returns the CronJobs to keep for teardown).
|
||||
* Empty when the plugin lacks `jobs:run`, so a plugin without the grant runs nothing.
|
||||
*/
|
||||
export function scheduleJobs(
|
||||
granted: ReadonlySet<string>,
|
||||
jobs: readonly ScheduledJob[],
|
||||
run: (jobId: string) => void,
|
||||
): ScheduledTask[] {
|
||||
): CronJob[] {
|
||||
if (!granted.has(JOBS_PERMISSION)) return [];
|
||||
const tasks: ScheduledTask[] = [];
|
||||
const tasks: CronJob[] = [];
|
||||
for (const job of jobs) {
|
||||
if (!job.schedule || !cron.validate(job.schedule)) continue; // invalid cron -> never runs
|
||||
if (!job.schedule || !validateCronExpression(job.schedule).valid) continue; // invalid cron -> never runs
|
||||
tasks.push(
|
||||
cron.schedule(job.schedule, () => {
|
||||
try {
|
||||
run(job.id);
|
||||
} catch {
|
||||
/* a job must never break the host loop */
|
||||
}
|
||||
CronJob.from({
|
||||
cronTime: job.schedule,
|
||||
start: true,
|
||||
onTick: () => {
|
||||
try {
|
||||
run(job.id);
|
||||
} catch {
|
||||
/* a job must never break the host loop */
|
||||
}
|
||||
},
|
||||
}),
|
||||
);
|
||||
}
|
||||
@@ -53,7 +61,7 @@ export function scheduleJobs(
|
||||
}
|
||||
|
||||
/** Stop + drop the given tasks (called on deactivation/kill so nothing leaks). */
|
||||
export function stopJobs(tasks: ScheduledTask[] | undefined): void {
|
||||
export function stopJobs(tasks: CronJob[] | undefined): void {
|
||||
for (const t of tasks ?? []) {
|
||||
try {
|
||||
t.stop();
|
||||
|
||||
@@ -53,7 +53,7 @@ interface Supervised {
|
||||
lastRss: number; // last reported resident set size (bytes)
|
||||
routes: PluginRouteInfo[];
|
||||
jobs: ScheduledJob[]; // declared background jobs (id + cron schedule)
|
||||
jobTasks?: ReturnType<typeof scheduleJobs>; // live node-cron tasks (only when jobs:run granted)
|
||||
jobTasks?: ReturnType<typeof scheduleJobs>; // live cron jobs (only when jobs:run granted)
|
||||
hooks: string[]; // provider hooks the plugin implements (e.g. 'placeDetailProvider')
|
||||
events: string[]; // core events the plugin subscribes to (names or '*')
|
||||
exports: string[]; // functions the plugin exposes to dependents (ctx.plugins.call)
|
||||
@@ -638,7 +638,7 @@ export class PluginSupervisor {
|
||||
sup.child = null;
|
||||
// In-flight host→child invokes can never complete now.
|
||||
this.rejectPending(sup, 'plugin exited');
|
||||
// The dead child's node-cron tasks keep ticking (node-cron holds them, not the
|
||||
// The dead child's cron jobs keep ticking (the host holds them, not the
|
||||
// child). Stop them here — otherwise every crash-restart cycle leaks a task-set
|
||||
// AND re-schedules a fresh one, so the job fires N+1 times per tick after N
|
||||
// crashes (duplicate egress/db side effects). kill() already does this for the
|
||||
|
||||
@@ -229,6 +229,56 @@ describe('auto-backup scheduling (AutoBackupJob.start)', () => {
|
||||
expect(registrar.unregister).not.toHaveBeenCalled();
|
||||
expect(logMock.logInfo).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('onApplicationBootstrap arms the cron when the registrar is enabled', () => {
|
||||
fsMock.existsSync.mockImplementation((p: PathLike) => String(p).endsWith('backup-settings.json'));
|
||||
fsMock.readFileSync.mockReturnValue(JSON.stringify({ enabled: true, interval: 'hourly' }));
|
||||
const { job, registered } = makeJob();
|
||||
job.onApplicationBootstrap();
|
||||
expect(registered.at(-1)?.name).toBe('auto-backup');
|
||||
expect(registered.at(-1)?.expr).toBe('0 * * * *');
|
||||
});
|
||||
|
||||
it('keep_days 0 banners "retention: forever" and skips retention after a run', async () => {
|
||||
fsMock.existsSync.mockImplementation((p: PathLike) => String(p).endsWith('backup-settings.json'));
|
||||
fsMock.readFileSync.mockReturnValue(JSON.stringify({ enabled: true, interval: 'daily', keep_days: 0 }));
|
||||
stubArchiver();
|
||||
const { job, registered } = makeJob();
|
||||
job.start();
|
||||
expect(logMock.logInfo).toHaveBeenCalledWith(expect.stringContaining('retention: forever'));
|
||||
|
||||
await (registered.at(-1)?.onTick as () => Promise<void>)();
|
||||
expect(logMock.logInfo).toHaveBeenCalledWith(expect.stringContaining('Auto-Backup created'));
|
||||
expect(fsMock.readdirSync).not.toHaveBeenCalled(); // cleanupOldBackups never ran
|
||||
});
|
||||
|
||||
it('a non-Error rejection from createBackup is stringified into the failure log', async () => {
|
||||
const broken = { createBackup: vi.fn().mockRejectedValue('plain string') } as unknown as BackupService;
|
||||
const registrar = { isEnabled: vi.fn(() => true), register: vi.fn(() => true), unregister: vi.fn() };
|
||||
const job = new AutoBackupJob(broken, registrar as unknown as CronRegistrarService);
|
||||
await job.runBackup();
|
||||
expect(logMock.logError).toHaveBeenCalledWith('Auto-Backup: plain string');
|
||||
});
|
||||
|
||||
it('getAutoSettings returns the loaded settings with a resolved timezone', () => {
|
||||
const prevTz = process.env.TZ;
|
||||
try {
|
||||
process.env.TZ = 'Europe/Zurich';
|
||||
const { job } = makeJob();
|
||||
expect(job.getAutoSettings()).toEqual({
|
||||
settings: { enabled: false, interval: 'daily', keep_days: 7, hour: 2, day_of_week: 0, day_of_month: 1 },
|
||||
timezone: 'Europe/Zurich',
|
||||
});
|
||||
|
||||
delete process.env.TZ;
|
||||
// Falls back to the host zone (or UTC when the host reports none).
|
||||
expect(typeof job.getAutoSettings().timezone).toBe('string');
|
||||
expect(job.getAutoSettings().timezone.length).toBeGreaterThan(0);
|
||||
} finally {
|
||||
if (prevTz === undefined) delete process.env.TZ;
|
||||
else process.env.TZ = prevTz;
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
// Moved from backup.impl.test.ts when updateAutoSettings moved onto the job
|
||||
|
||||
@@ -76,6 +76,11 @@ describe('IdempotencyCleanupJob', () => {
|
||||
on.job.onApplicationBootstrap();
|
||||
expect(on.registrar.register).toHaveBeenCalledWith('idempotency-cleanup', '0 3 * * *', expect.any(Function));
|
||||
|
||||
// The registered callback IS the tick — drive it once over an empty table
|
||||
// (nothing to purge → no log, no throw).
|
||||
const onTick = on.registrar.register.mock.calls[0][2] as () => void;
|
||||
expect(() => onTick()).not.toThrow();
|
||||
|
||||
const off = makeJob(false);
|
||||
off.job.onApplicationBootstrap();
|
||||
expect(off.registrar.register).not.toHaveBeenCalled();
|
||||
@@ -103,5 +108,10 @@ describe('IdempotencyCleanupJob', () => {
|
||||
const broken = { prepare: () => { throw new Error('db gone'); } } as unknown as DatabaseService;
|
||||
const job = new IdempotencyCleanupJob(broken, { isEnabled: () => true } as unknown as CronRegistrarService);
|
||||
expect(() => job.tick()).not.toThrow();
|
||||
|
||||
// Non-Error throws are stringified rather than crashing the catch itself.
|
||||
const brokenString = { prepare: () => { throw 'db string'; } } as unknown as DatabaseService;
|
||||
const job2 = new IdempotencyCleanupJob(brokenString, { isEnabled: () => true } as unknown as CronRegistrarService);
|
||||
expect(() => job2.tick()).not.toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -263,4 +263,9 @@ describe('cleanupOldBackups', () => {
|
||||
readdirSyncMock.mockImplementation(() => { throw new Error('ENOENT'); });
|
||||
expect(() => cleanupOldBackups(7, NOW)).not.toThrow();
|
||||
});
|
||||
|
||||
it('swallows non-Error throws without throwing (string rejection path)', () => {
|
||||
readdirSyncMock.mockImplementation(() => { throw 'nope'; });
|
||||
expect(() => cleanupOldBackups(7, NOW)).not.toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,43 @@
|
||||
/**
|
||||
* Grammar-compat pin for the plugin job scheduler's node-cron → cron migration.
|
||||
*
|
||||
* A validation *tightening* would silently disable a published plugin's job
|
||||
* (an invalid cron is skipped, never scheduled — deliberately, so a bad
|
||||
* schedule can't take the host down). This runs the REAL `cron` validator
|
||||
* (unmocked) over the manifest shapes plugins realistically declare and pins
|
||||
* that everything node-cron accepted still schedules, and garbage still
|
||||
* doesn't.
|
||||
*/
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { validateCronExpression } from 'cron';
|
||||
|
||||
const ACCEPTED = [
|
||||
'0 0 * * *', // nightly
|
||||
'*/5 * * * *', // step minutes
|
||||
'0 9 * * MON', // day-of-week name
|
||||
'0 9 * * sun', // lowercase name
|
||||
'0 9 * * 7', // 7-as-Sunday
|
||||
'0 0 1 JAN *', // month name
|
||||
'30 */2 * * 0-5', // range + step combined
|
||||
'1-5 * * * *', // minute range
|
||||
'*/30 * * * * *', // 6-field with seconds first
|
||||
'0 0 29 2 *', // leap-day
|
||||
];
|
||||
|
||||
const REJECTED = [
|
||||
'not-a-cron',
|
||||
'',
|
||||
'* * *', // too few fields
|
||||
'61 * * * *', // minute out of range
|
||||
'0 24 * * *', // hour out of range
|
||||
];
|
||||
|
||||
describe('plugin job cron grammar (real validator)', () => {
|
||||
it.each(ACCEPTED)('PJOB-COMPAT accepts %j', (expr) => {
|
||||
expect(validateCronExpression(expr).valid).toBe(true);
|
||||
});
|
||||
|
||||
it.each(REJECTED)('PJOB-COMPAT rejects %j', (expr) => {
|
||||
expect(validateCronExpression(expr).valid).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -6,11 +6,11 @@
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
|
||||
const h = vi.hoisted(() => ({ tasks: [] as Array<{ expr: string; fn: () => void; stopped: boolean; stop(): void }> }));
|
||||
vi.mock('node-cron', () => ({
|
||||
default: {
|
||||
validate: (expr: string) => typeof expr === 'string' && expr.trim() !== '' && expr !== 'not-a-cron',
|
||||
schedule: (expr: string, fn: () => void) => {
|
||||
const t = { expr, fn, stopped: false, stop() { this.stopped = true; } };
|
||||
vi.mock('cron', () => ({
|
||||
validateCronExpression: (expr: string) => ({ valid: typeof expr === 'string' && expr.trim() !== '' && expr !== 'not-a-cron' }),
|
||||
CronJob: {
|
||||
from: ({ cronTime, onTick }: { cronTime: string; onTick: () => void }) => {
|
||||
const t = { expr: cronTime, fn: onTick, stopped: false, stop() { this.stopped = true; } };
|
||||
h.tasks.push(t);
|
||||
return t;
|
||||
},
|
||||
|
||||
Reference in New Issue
Block a user