mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-19 21:46:49 +00:00
fix(server): plan 3a review follow-ups — fail closed without an ORM at every choke point; seam and D5 ratchet tests; dead method and DDL copies removed
This commit is contained in:
@@ -23,8 +23,8 @@ export class AddonsRepository extends EntityRepository<Addons> {
|
||||
* `refresh: true` (Task 0 review, I1 — carried forward for every PK read in
|
||||
* this plan): a primary-key `findOne` answers a repeat call from the
|
||||
* identity map, invisible to a write on the same id inside the same
|
||||
* request (`setEnabled`, below, or `admin.service.ts`'s still-raw
|
||||
* `UPDATE addons SET enabled = ?` on the same connection).
|
||||
* request (`admin.service.ts`'s still-raw `UPDATE addons SET enabled = ?`
|
||||
* on the same connection).
|
||||
*/
|
||||
async isEnabled(id: string): Promise<boolean> {
|
||||
const row = await this.findOne({ id }, { fields: ['enabled'], refresh: true });
|
||||
@@ -43,22 +43,4 @@ export class AddonsRepository extends EntityRepository<Addons> {
|
||||
const rows = await this.find({ enabled: true }, { orderBy: { sort_order: 'asc' } });
|
||||
return rows.map((row) => toRow(row) as AddonRow);
|
||||
}
|
||||
|
||||
/**
|
||||
* `UPDATE addons SET enabled = ? WHERE id = ?` — the write half of the
|
||||
* enablement toggle `admin.service.ts` still issues as raw SQL
|
||||
* (`updateAddon`, outside this plan's six domains). Declared here, on the
|
||||
* repository that owns the `addons` table, so that conversion has a
|
||||
* ready-made method rather than inventing its own; nothing in this domain
|
||||
* calls it yet.
|
||||
*
|
||||
* `enabled` is `p.boolean()` on the entity, so `nativeUpdate` stores the
|
||||
* legacy `1`/`0` through MikroORM's own boolean type mapper — the
|
||||
* repository test asserts the raw stored value directly, and that a
|
||||
* following read (with `refresh: true`) comes back as a JS boolean, not
|
||||
* the raw int.
|
||||
*/
|
||||
async setEnabled(id: string, enabled: boolean): Promise<void> {
|
||||
await this.nativeUpdate({ id }, { enabled });
|
||||
}
|
||||
}
|
||||
|
||||
@@ -20,10 +20,10 @@ export class AppSettingsRepository extends EntityRepository<AppSettings> {
|
||||
* the same key inside the same request, which is exactly the coexistence
|
||||
* case this plan is built on (raw SQL and the ORM sharing one connection
|
||||
* while callers convert one at a time). `refresh` keeps this a single
|
||||
* query either way; only a stale identity-map hit is avoided.
|
||||
* Side effect of `refresh`: an UNFLUSHED in-memory change to the selected
|
||||
* field on that entity is discarded (the entity reverts to the row). No
|
||||
* caller mutates these entities before reading, by design (D4: rows out).
|
||||
* query either way; only a stale identity-map hit is avoided.
|
||||
*/
|
||||
async getValue(key: string): Promise<string | null> {
|
||||
const row = await this.findOne({ key }, { fields: ['value'], refresh: true });
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { Injectable } from '@nestjs/common';
|
||||
import { InjectRepository } from '@mikro-orm/nestjs';
|
||||
import { ValidationError } from '@mikro-orm/core';
|
||||
import { AuditLog } from '../../db/entities/AuditLog.entity';
|
||||
import type { AuditLogRepository } from '../../db/repositories/AuditLog.repository';
|
||||
import { Users } from '../../db/entities/Users.entity';
|
||||
@@ -80,10 +81,31 @@ export class AuditService {
|
||||
try {
|
||||
const email = await this.users.getEmail(userId);
|
||||
return email || `uid:${userId}`;
|
||||
} catch { return `uid:${userId}`; }
|
||||
} catch (e) {
|
||||
// RULING (task-6-fix-brief.md item 4): an audit write must never block
|
||||
// the request it is auditing, so this stays a swallow — the documented
|
||||
// exception to "fail closed on a MikroORM ValidationError" every other
|
||||
// domain in this plan follows. But a ValidationError here (typically
|
||||
// cannotUseGlobalContext) is a wiring bug, not an ordinary DB failure,
|
||||
// and folding it into the generic `uid:${userId}` fallback with no log
|
||||
// line would hide a future unwrapped entrypoint. Log it distinctly so
|
||||
// it is never confused with (or lost among) an actual missing user.
|
||||
if (e instanceof ValidationError) {
|
||||
logError(`Audit email lookup ran with no request context: ${e.message}`);
|
||||
}
|
||||
return `uid:${userId}`;
|
||||
}
|
||||
}
|
||||
|
||||
/** Best-effort; never throws — failures are logged only. */
|
||||
/**
|
||||
* Best-effort; never throws — failures are logged only. RULING
|
||||
* (task-6-fix-brief.md item 4): this is the one documented exception to
|
||||
* "fail closed on a MikroORM ValidationError" — an audit row must never
|
||||
* become a 500 for the mutation it is recording. A ValidationError still
|
||||
* gets a message distinct from every other write failure (below), so an
|
||||
* unwrapped entrypoint stays visible in the log even though it never
|
||||
* throws.
|
||||
*/
|
||||
async writeAudit(entry: {
|
||||
userId: number | null;
|
||||
action: string;
|
||||
@@ -113,7 +135,11 @@ export class AuditService {
|
||||
logDebug(oneLine(`AUDIT ${entry.action} userId=${entry.userId} ${detailsJson}`));
|
||||
}
|
||||
} catch (e) {
|
||||
logError(`Audit write failed: ${e instanceof Error ? e.message : e}`);
|
||||
if (e instanceof ValidationError) {
|
||||
logError(`Audit write ran with no request context (row not written): ${e.message}`);
|
||||
} else {
|
||||
logError(`Audit write failed: ${e instanceof Error ? e.message : e}`);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -33,6 +33,15 @@ import { CategoriesService } from './categories.service';
|
||||
export class CategoriesMcp {
|
||||
constructor(
|
||||
private readonly categories: CategoriesService,
|
||||
// Carve-out (task-6-review-parity.md M3, mirroring addons.service.ts's own
|
||||
// — see that file's docstring): this domain's own reads/writes are all
|
||||
// repository-backed below; `DatabaseService` stays injected purely to
|
||||
// feed `isDemoUser`'s `isDemoUserId(env, db, userId)`, whose
|
||||
// `SELECT email FROM users WHERE id = ?` lives in
|
||||
// `common/demo-write.ts` (nest/auth/nest/users, not this domain's plan).
|
||||
// `UsersRepository.getEmail` already issues that identical statement —
|
||||
// converting `demo-write.ts` to it is a deferred follow-up
|
||||
// (`deferred.md`), not this plan's.
|
||||
private readonly db: DatabaseService,
|
||||
private readonly env: RuntimeEnvService,
|
||||
private readonly guards: McpToolGuardsService,
|
||||
|
||||
@@ -52,7 +52,12 @@ export class DocSyncJob implements OnApplicationBootstrap {
|
||||
|
||||
async onApplicationBootstrap(): Promise<void> {
|
||||
if (!this.registrar.isEnabled()) return;
|
||||
logInfo(`Document sync: polling every ${await this.intervalSeconds()}s`);
|
||||
// Through runOnBoot (task-6-review-parity.md C1: raw SQL today, but the
|
||||
// wrap belongs at the entrypoint so it stays safe if this dependency
|
||||
// graph goes repository-backed later).
|
||||
await this.registrar.runOnBoot('docsync-boot', async () => {
|
||||
logInfo(`Document sync: polling every ${await this.intervalSeconds()}s`);
|
||||
});
|
||||
this.registrar.register('docsync', '* * * * *', () => this.tick());
|
||||
}
|
||||
|
||||
|
||||
@@ -21,10 +21,19 @@ export class AirtrailSyncJob implements OnApplicationBootstrap {
|
||||
|
||||
async onApplicationBootstrap(): Promise<void> {
|
||||
if (!this.registrar.isEnabled()) return;
|
||||
const value = this.db.get<{ value: string }>('SELECT value FROM app_settings WHERE key = ?', 'airtrail_poll_interval_minutes')?.value;
|
||||
const raw = Number.parseInt(value || '5', 10);
|
||||
const minutes = Number.isFinite(raw) && raw >= 1 && raw <= 59 ? raw : 5;
|
||||
logInfo(`AirTrail sync: scheduled every ${minutes}m`);
|
||||
// Through runOnBoot (task-6-review-parity.md C1: raw SQL today, but the
|
||||
// wrap belongs at the entrypoint so it stays safe if this dependency
|
||||
// graph goes repository-backed later). minutes defaults to the same
|
||||
// fallback the interval read itself falls back to, so an absent MikroORM
|
||||
// (logged distinctly by runOnBoot, never silently) still registers the
|
||||
// job at its default cadence rather than not registering at all.
|
||||
let minutes = 5;
|
||||
await this.registrar.runOnBoot('airtrail-sync-boot', async () => {
|
||||
const value = this.db.get<{ value: string }>('SELECT value FROM app_settings WHERE key = ?', 'airtrail_poll_interval_minutes')?.value;
|
||||
const raw = Number.parseInt(value || '5', 10);
|
||||
minutes = Number.isFinite(raw) && raw >= 1 && raw <= 59 ? raw : 5;
|
||||
logInfo(`AirTrail sync: scheduled every ${minutes}m`);
|
||||
});
|
||||
this.registrar.register('airtrail-sync', `*/${minutes} * * * *`, () => this.tick());
|
||||
}
|
||||
|
||||
|
||||
@@ -27,13 +27,22 @@ export class DawarichSyncJob implements OnApplicationBootstrap {
|
||||
|
||||
async onApplicationBootstrap(): Promise<void> {
|
||||
if (!this.registrar.isEnabled()) return;
|
||||
const value = this.db.get<{ value: string }>(
|
||||
'SELECT value FROM app_settings WHERE key = ?',
|
||||
'dawarich_poll_interval_minutes',
|
||||
)?.value;
|
||||
const raw = Number.parseInt(value || '15', 10);
|
||||
const minutes = Number.isFinite(raw) && raw >= 5 && raw <= 59 ? raw : 15;
|
||||
logInfo(`Dawarich sync: scheduled every ${minutes}m`);
|
||||
// Through runOnBoot (task-6-review-parity.md C1: raw SQL today, but the
|
||||
// wrap belongs at the entrypoint so it stays safe if this dependency
|
||||
// graph goes repository-backed later). minutes defaults to the same
|
||||
// fallback the interval read itself falls back to, so an absent MikroORM
|
||||
// (logged distinctly by runOnBoot, never silently) still registers the
|
||||
// job at its default cadence rather than not registering at all.
|
||||
let minutes = 15;
|
||||
await this.registrar.runOnBoot('dawarich-sync-boot', async () => {
|
||||
const value = this.db.get<{ value: string }>(
|
||||
'SELECT value FROM app_settings WHERE key = ?',
|
||||
'dawarich_poll_interval_minutes',
|
||||
)?.value;
|
||||
const raw = Number.parseInt(value || '15', 10);
|
||||
minutes = Number.isFinite(raw) && raw >= 5 && raw <= 59 ? raw : 15;
|
||||
logInfo(`Dawarich sync: scheduled every ${minutes}m`);
|
||||
});
|
||||
this.registrar.register('dawarich-sync', `*/${minutes} * * * *`, () => this.tick());
|
||||
}
|
||||
|
||||
|
||||
@@ -19,8 +19,12 @@ export class JourneyThumbsJob implements OnApplicationBootstrap {
|
||||
|
||||
onApplicationBootstrap(): void {
|
||||
if (!this.registrar.isEnabled()) return;
|
||||
// Run once on startup to reclaim orphans left over from before this sweeper existed.
|
||||
void this.sweep();
|
||||
// Run once on startup to reclaim orphans left over from before this
|
||||
// sweeper existed — through runOnBoot (task-6-review-parity.md C1), not
|
||||
// the bare `void this.sweep()` this used to be: sweepOrphanThumbs reaches
|
||||
// AddonsService.isAddonEnabled, which is repository-backed, and this call
|
||||
// has no HTTP request (or registered-tick wrapper) behind it.
|
||||
void this.registrar.runOnBoot('journey-thumbs-boot', () => this.sweep());
|
||||
this.registrar.register('journey-thumbs', '0 4 * * *', () => {
|
||||
void this.sweep();
|
||||
});
|
||||
|
||||
@@ -18,12 +18,15 @@ export class TrekPhotoCacheJob implements OnApplicationBootstrap {
|
||||
|
||||
onApplicationBootstrap(): void {
|
||||
if (!this.registrar.isEnabled()) return;
|
||||
// Run once immediately on startup to evict any entries left over from a previous run
|
||||
void (async () => {
|
||||
// Run once immediately on startup to evict any entries left over from a
|
||||
// previous run — through runOnBoot (task-6-review-parity.md C1: raw SQL
|
||||
// today, but the wrap belongs at the entrypoint so it stays safe if this
|
||||
// dependency graph goes repository-backed later).
|
||||
void this.registrar.runOnBoot('trek-photo-cache-boot', async () => {
|
||||
try {
|
||||
await this.cache.sweepExpired();
|
||||
} catch { /* cache dir may not exist yet — harmless */ }
|
||||
})();
|
||||
});
|
||||
this.registrar.register('trek-photo-cache', '0 */2 * * *', () => {
|
||||
void this.tick();
|
||||
}, { timezone: 'none' });
|
||||
|
||||
@@ -34,26 +34,30 @@ export class ReminderJobsService implements OnApplicationBootstrap {
|
||||
if (!this.registrar.isEnabled()) return;
|
||||
|
||||
// Boot banners only — the enable gates are read per tick below; these
|
||||
// reflect the state at boot.
|
||||
try {
|
||||
const reminderEnabled = (await this.getSetting('notify_trip_reminder')) !== 'false';
|
||||
const channelsRaw = (await this.getSetting('notification_channels')) || (await this.getSetting('notification_channel')) || 'none';
|
||||
const activeChannels = channelsRaw === 'none' ? [] : channelsRaw.split(',').map(c => c.trim());
|
||||
if (!reminderEnabled) {
|
||||
logInfo('Trip reminders: disabled in settings');
|
||||
} else {
|
||||
const tripCount = this.db.get<{ c: number }>('SELECT COUNT(*) as c FROM trips WHERE reminder_days > 0 AND start_date IS NOT NULL')?.c ?? 0;
|
||||
logInfo(`Trip reminders: enabled via [${activeChannels.join(',')}]${tripCount > 0 ? `, ${tripCount} trip(s) with active reminders` : ''}`);
|
||||
}
|
||||
// reflect the state at boot. Through runOnBoot (task-6-review-parity.md
|
||||
// C1: raw SQL today, but the wrap belongs at the entrypoint so it stays
|
||||
// safe if this dependency graph goes repository-backed later).
|
||||
await this.registrar.runOnBoot('reminder-jobs-boot', async () => {
|
||||
try {
|
||||
const reminderEnabled = (await this.getSetting('notify_trip_reminder')) !== 'false';
|
||||
const channelsRaw = (await this.getSetting('notification_channels')) || (await this.getSetting('notification_channel')) || 'none';
|
||||
const activeChannels = channelsRaw === 'none' ? [] : channelsRaw.split(',').map(c => c.trim());
|
||||
if (!reminderEnabled) {
|
||||
logInfo('Trip reminders: disabled in settings');
|
||||
} else {
|
||||
const tripCount = this.db.get<{ c: number }>('SELECT COUNT(*) as c FROM trips WHERE reminder_days > 0 AND start_date IS NOT NULL')?.c ?? 0;
|
||||
logInfo(`Trip reminders: enabled via [${activeChannels.join(',')}]${tripCount > 0 ? `, ${tripCount} trip(s) with active reminders` : ''}`);
|
||||
}
|
||||
|
||||
if ((await this.getSetting('notify_todo_due')) !== 'false') {
|
||||
logInfo(`Todo due reminders: enabled (lead ${TODO_REMINDER_LEAD_DAYS}d)`);
|
||||
} else {
|
||||
logInfo('Todo due reminders: disabled in settings');
|
||||
if ((await this.getSetting('notify_todo_due')) !== 'false') {
|
||||
logInfo(`Todo due reminders: enabled (lead ${TODO_REMINDER_LEAD_DAYS}d)`);
|
||||
} else {
|
||||
logInfo('Todo due reminders: disabled in settings');
|
||||
}
|
||||
} catch {
|
||||
/* banners are best-effort */
|
||||
}
|
||||
} catch {
|
||||
/* banners are best-effort */
|
||||
}
|
||||
});
|
||||
|
||||
this.registrar.register('trip-reminders', '0 9 * * *', () => this.tripTick());
|
||||
this.registrar.register('todo-reminders', '0 9 * * *', () => this.todoTick());
|
||||
|
||||
@@ -19,8 +19,12 @@ export class PlacePhotoCacheJob implements OnApplicationBootstrap {
|
||||
|
||||
onApplicationBootstrap(): void {
|
||||
if (!this.registrar.isEnabled()) return;
|
||||
// Run once on startup to reclaim orphans left over from before this sweeper existed.
|
||||
void this.sweep();
|
||||
// Run once on startup to reclaim orphans left over from before this
|
||||
// sweeper existed — through runOnBoot (task-6-review-parity.md C1's
|
||||
// "scanned for siblings" list: sweepOrphans is raw SQL today, safe, but
|
||||
// the wrap belongs at the entrypoint so it stays safe once this
|
||||
// dependency graph goes repository-backed too, same as journey-thumbs).
|
||||
void this.registrar.runOnBoot('place-photo-cache-boot', () => this.sweep());
|
||||
this.registrar.register('place-photo-cache', '30 3 * * *', () => {
|
||||
void this.sweep();
|
||||
});
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { Injectable, type OnApplicationBootstrap, type OnModuleDestroy } from '@nestjs/common';
|
||||
import { Injectable, Optional, type OnApplicationBootstrap, type OnModuleDestroy } from '@nestjs/common';
|
||||
import semver from 'semver';
|
||||
import { MikroORM } from '@mikro-orm/core';
|
||||
import { DatabaseService } from '../database/database.service';
|
||||
@@ -195,8 +195,17 @@ export class PluginRuntimeService implements OnApplicationBootstrap, OnModuleDes
|
||||
// request context built from this. Nest always injects it (MikroOrmCoreModule is
|
||||
// global); a hand-built test instance that dispatches through a real
|
||||
// PermissionsService/repository must pass one or those reads fail closed with
|
||||
// cannotUseGlobalContext instead of running.
|
||||
private readonly orm?: MikroORM,
|
||||
// cannotUseGlobalContext instead of running (task-6-fix-brief.md item 1: the
|
||||
// supervisor now THROWS rather than dispatching unwrapped when it is absent).
|
||||
// `@Optional()` (task-6-review-template.md Minor 3 — harmonised with
|
||||
// `CronRegistrarService`'s own `@Optional()` orm param; a plain `?` is
|
||||
// TS-only and does not tell Nest's DI a provider may be absent): safe
|
||||
// today because nothing constructs this service through a partial
|
||||
// `Test.createTestingModule` graph, only `new PluginRuntimeService(...)`
|
||||
// by hand or the full `buildApp()` — but the first partial harness that
|
||||
// does needs this, the same lesson `CronRegistrarService` already learned
|
||||
// (task-2-fix-report.md's "Concerns").
|
||||
@Optional() private readonly orm?: MikroORM,
|
||||
) {}
|
||||
|
||||
private get db() {
|
||||
|
||||
@@ -120,9 +120,13 @@ export class PluginSupervisor {
|
||||
// PluginRuntimeService's own `orm` field is not assigned yet when its `supervisor`
|
||||
// field initializer runs (constructor params are assigned after field
|
||||
// initializers — see that file's comment), so this is read lazily, once per
|
||||
// dispatch. Optional only so the hand-built doubles in
|
||||
// supervisor-lifecycle.test.ts / provider-hook-grant.test.ts /
|
||||
// event-subscriptions.test.ts (none of which reach a repository) don't need one;
|
||||
// dispatch. Optional at the type level only for the hand-built doubles in
|
||||
// provider-hook-grant.test.ts / event-subscriptions.test.ts, neither of
|
||||
// which ever dispatches a `'req'` message through onMessage — a double
|
||||
// that DOES dispatch (supervisor-lifecycle.test.ts's rate-limit case,
|
||||
// tests/integration/plugins/supervisor.test.ts's real child IPC) must
|
||||
// pass one, because onMessage now THROWS when this returns undefined
|
||||
// (task-6-fix-brief.md item 1 — no more silent unwrapped dispatch).
|
||||
// PluginRuntimeService always passes the real MikroORM (OrmModule is global).
|
||||
private readonly resolveOrm?: () => { em: EntityManager } | undefined,
|
||||
) {
|
||||
@@ -568,10 +572,22 @@ export class PluginSupervisor {
|
||||
// dispatch's own audit write. Without this, PermissionsService.checkPermission
|
||||
// ran outside any context, threw on a cold cache, and the catch served
|
||||
// defaults — fail-open on a tightened flag (task-2-review.md C3).
|
||||
//
|
||||
// Fail closed (task-6-fix-brief.md item 1, task-6-review-template.md
|
||||
// Important 1 / M-B): an absent `resolveOrm` used to fall through to
|
||||
// an UNWRAPPED dispatch — silent degrade, the exact shape D6 forbids
|
||||
// at a choke point. It only failed closed downstream, and only
|
||||
// because two OTHER switches happened to hold (production's
|
||||
// `allowGlobalContext: false` and `loadPermissions` rethrowing a
|
||||
// MikroORM `ValidationError`) — nothing here said so. Throw instead:
|
||||
// the dispatch never runs without a context, full stop, regardless of
|
||||
// what those other switches do. The three hand-built test doubles
|
||||
// that dispatch through this path now pass a thunk (`sharedTestOrm`).
|
||||
const orm = this.resolveOrm?.();
|
||||
const res = orm
|
||||
? await withRequestContext(orm, () => sup.rpcHost.dispatch(req, actingUserId))
|
||||
: await sup.rpcHost.dispatch(req, actingUserId);
|
||||
if (!orm) {
|
||||
throw new Error('PluginSupervisor: no ORM available to build a request context for this RPC dispatch');
|
||||
}
|
||||
const res = await withRequestContext(orm, () => sup.rpcHost.dispatch(req, actingUserId));
|
||||
sup.child?.send(res);
|
||||
} finally {
|
||||
sup.rpcLimiter.release();
|
||||
|
||||
@@ -104,9 +104,11 @@ export class TrekWsAdapter extends WsAdapter {
|
||||
httpServer: HttpServer,
|
||||
// D6 (task-2-review.md's controller ruling): a WS message handler has no HTTP
|
||||
// request behind it, so every `@SubscribeMessage` dispatch is wrapped here —
|
||||
// ONE wrapper at bindMessageHandlers below, not one per handler. Optional only
|
||||
// for trek-ws-adapter.test.ts, which never reaches a repository; bootstrap.ts
|
||||
// always passes the real MikroORM.
|
||||
// ONE wrapper at bindMessageHandlers below, not one per handler. Optional at
|
||||
// the type level only for hand-built test doubles — bootstrap.ts always
|
||||
// passes the real MikroORM, and any double that dispatches a matched
|
||||
// message through bindMessageHandlers (task-6-fix-brief.md item 1) must
|
||||
// pass one too, or the dispatch throws rather than running unwrapped.
|
||||
private readonly orm?: { em: EntityManager },
|
||||
) {
|
||||
super(httpServer as unknown as INestApplicationContext);
|
||||
@@ -219,9 +221,16 @@ export class TrekWsAdapter extends WsAdapter {
|
||||
// invoked (before transform()/subscribe() ever run), and RequestContext.create
|
||||
// is AsyncLocalStorage.run — it only covers what executes inside this
|
||||
// synchronous frame, after which the async chain carries it on its own.
|
||||
const result = this.orm
|
||||
? withRequestContext(this.orm, () => handler.callback(message, socket))
|
||||
: handler.callback(message, socket);
|
||||
//
|
||||
// Fail closed (task-6-fix-brief.md item 1): a missing `orm` used to fall
|
||||
// through to an unwrapped call — silent degrade. `bootstrap.ts` always
|
||||
// passes the real MikroORM, so this only ever throws in a hand-built
|
||||
// test double that dispatches through bindMessageHandlers without one;
|
||||
// that double now passes an ORM from `sharedTestOrm`.
|
||||
if (!this.orm) {
|
||||
throw new Error('TrekWsAdapter: no MikroORM available to build a request context for this message handler');
|
||||
}
|
||||
const result = withRequestContext(this.orm, () => handler.callback(message, socket));
|
||||
transform(result).subscribe({
|
||||
next: (response) => {
|
||||
if (response !== undefined && socket.readyState === 1) {
|
||||
|
||||
@@ -5,6 +5,7 @@ import { MikroORM } from '@mikro-orm/core';
|
||||
import { readEnv } from '../../app-config';
|
||||
import { RuntimeEnvService } from '../app-config/runtime-env.service';
|
||||
import { withRequestContext } from '../database/request-context';
|
||||
import { logError } from '../audit/audit-log.logger';
|
||||
|
||||
/**
|
||||
* The one way TREK code schedules a cron. Job providers register here from
|
||||
@@ -71,13 +72,58 @@ export class CronRegistrarService implements OnApplicationShutdown {
|
||||
if (!this.isEnabled()) return false;
|
||||
const timeZone = opts?.timezone === 'none' ? undefined : readEnv().app.tz || 'UTC';
|
||||
const orm = this.orm;
|
||||
const wrappedTick = orm ? () => withRequestContext(orm, () => onTick()) : onTick;
|
||||
// Fail closed (task-6-fix-brief.md item 1): registration itself never
|
||||
// requires an ORM (CRONREG-002..009 register jobs whose onTick never
|
||||
// touches a repository, and the e2e partial harnesses that construct this
|
||||
// service via @Optional() with no MikroORM never tick at all — isEnabled()
|
||||
// is false under NODE_ENV=test). But once a tick DOES fire, it must run
|
||||
// inside a request context or not at all: dispatching it unwrapped used to
|
||||
// let a repository read inside onTick either silently succeed against the
|
||||
// wrong (global) EntityManager or throw MikroORM's own generic
|
||||
// "global EntityManager" error deep inside the job's own try/catch. Throw
|
||||
// OUR OWN clear error instead, so a hand-built double that registers a
|
||||
// job without an ORM but whose tick reaches a repository fails loudly and
|
||||
// distinctly rather than silently degrading.
|
||||
const wrappedTick = async () => {
|
||||
if (!orm) {
|
||||
throw new Error(`CronRegistrarService: no MikroORM available to build a request context for job "${name}"`);
|
||||
}
|
||||
return withRequestContext(orm, () => onTick());
|
||||
};
|
||||
const job = CronJob.from({ cronTime: expression, onTick: wrappedTick, start: true, timeZone });
|
||||
this.registry.addCronJob(name, job);
|
||||
this.names.add(name);
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Runs a boot-time (`onApplicationBootstrap`) one-off sweep inside the same
|
||||
* request context a registered tick gets from `register()` above — the one
|
||||
* choke point for the whole class of bug task-6-review-parity.md's C1
|
||||
* found: a job's "run once at startup" call is not a scheduled tick, so it
|
||||
* bypassed `register()`'s wrapper entirely; once its dependency graph went
|
||||
* repository-backed (`AddonsService.isAddonEnabled`), the read threw
|
||||
* `cannotUseGlobalContext` on EVERY production boot — silently, because the
|
||||
* job's own try/catch swallowed it into a log line about the sweep's own
|
||||
* domain, never mentioning the missing context. Every
|
||||
* `onApplicationBootstrap` boot sweep goes through this one method now,
|
||||
* whether or not it reaches a repository TODAY, so a job's dependency graph
|
||||
* can grow one later without silently regressing to the same bug.
|
||||
*
|
||||
* Fails closed like every other D6 choke point: no MikroORM means `fn`
|
||||
* never runs at all, and the failure is logged with a message naming THIS
|
||||
* method — never MikroORM's own "global EntityManager" wording — so a
|
||||
* missing-context boot failure is never confused with (or swallowed by) an
|
||||
* ordinary sweep failure the job's own catch already logs.
|
||||
*/
|
||||
async runOnBoot(name: string, fn: () => Promise<void> | void): Promise<void> {
|
||||
if (!this.orm) {
|
||||
logError(`CronRegistrarService.runOnBoot: no MikroORM available — boot sweep "${name}" did not run`);
|
||||
return;
|
||||
}
|
||||
await withRequestContext(this.orm, fn);
|
||||
}
|
||||
|
||||
/** Stop and drop a job by name. A name that was never registered is a no-op. */
|
||||
unregister(name: string): void {
|
||||
if (!this.names.has(name)) return;
|
||||
|
||||
Reference in New Issue
Block a user