mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-06-19 13:21:46 +00:00
fix(server): lengthen idempotency key TTL to survive multi-day offline (H6) (#1182)
The nightly cleanup deleted idempotency keys older than 24h. The TREK client replays queued mutations with their X-Idempotency-Key on reconnect, so a device offline longer than a day had its keys GC'd before it returned — the replayed POST was then treated as new and created a duplicate. - raise the TTL to 30 days (DEFAULT_IDEMPOTENCY_TTL_SECONDS), overridable via IDEMPOTENCY_TTL_SECONDS - extract purgeExpiredIdempotencyKeys(now, ttl, db) (mirrors cleanupOldBackups) with an injectable db, and have the cron job call it - tests: 30-day default eviction, 25-day key retained (was dropped at 24h), env override H7 (exactly-once across the lost-response window) is deferred: a correct fix must store the response in the same DB transaction as the entity write. Doing it in the generic interceptor (reserve-before-handler) cannot store the real response body for the crash case, which would break the client's temp->real id remapping on replay (mutationQueue.flush relies on the entity in the body). It needs a per-service change and is tracked separately.
This commit is contained in:
+32
-7
@@ -291,20 +291,45 @@ function startVersionCheck(): void {
|
||||
}, { timezone: tz });
|
||||
}
|
||||
|
||||
// Idempotency key cleanup: nightly at 3 AM — delete keys older than 24 hours
|
||||
// Idempotency key cleanup: nightly at 3 AM — delete keys past their TTL.
|
||||
// The TTL must exceed any realistic offline window: the TREK client replays
|
||||
// queued mutations with their X-Idempotency-Key when it reconnects, so a key
|
||||
// GC'd before the device comes back online would let the replay create a
|
||||
// duplicate. 24h was far too short for a multi-day offline trip; default 30d,
|
||||
// overridable via IDEMPOTENCY_TTL_SECONDS.
|
||||
const DEFAULT_IDEMPOTENCY_TTL_SECONDS = 30 * 24 * 60 * 60; // 30 days
|
||||
let idempotencyCleanupTask: ScheduledTask | null = null;
|
||||
|
||||
function idempotencyTtlSeconds(): number {
|
||||
const n = Number(process.env.IDEMPOTENCY_TTL_SECONDS);
|
||||
return Number.isFinite(n) && n > 0 ? n : DEFAULT_IDEMPOTENCY_TTL_SECONDS;
|
||||
}
|
||||
|
||||
interface PurgeDb {
|
||||
prepare(sql: string): { run(...args: unknown[]): { changes: number } };
|
||||
}
|
||||
|
||||
/** Delete idempotency keys older than the configured TTL. Returns rows removed.
|
||||
* The db is injectable for testing; the cron job uses the default. */
|
||||
function purgeExpiredIdempotencyKeys(
|
||||
now: number = Date.now(),
|
||||
ttlSeconds: number = idempotencyTtlSeconds(),
|
||||
database: PurgeDb = require('./db/database').db,
|
||||
): number {
|
||||
const cutoff = Math.floor(now / 1000) - ttlSeconds;
|
||||
const result = database.prepare('DELETE FROM idempotency_keys WHERE created_at < ?').run(cutoff);
|
||||
return result.changes;
|
||||
}
|
||||
|
||||
function startIdempotencyCleanup(): void {
|
||||
if (idempotencyCleanupTask) { idempotencyCleanupTask.stop(); idempotencyCleanupTask = null; }
|
||||
|
||||
const tz = process.env.TZ || 'UTC';
|
||||
idempotencyCleanupTask = cron.schedule('0 3 * * *', () => {
|
||||
try {
|
||||
const { db } = require('./db/database');
|
||||
const cutoff = Math.floor(Date.now() / 1000) - 86400;
|
||||
const result = db.prepare('DELETE FROM idempotency_keys WHERE created_at < ?').run(cutoff);
|
||||
if (result.changes > 0) {
|
||||
logInfo(`Idempotency cleanup: removed ${result.changes} expired key(s)`);
|
||||
const removed = purgeExpiredIdempotencyKeys();
|
||||
if (removed > 0) {
|
||||
logInfo(`Idempotency cleanup: removed ${removed} expired key(s)`);
|
||||
}
|
||||
} catch (err: unknown) {
|
||||
logError(`Idempotency cleanup: ${err instanceof Error ? err.message : err}`);
|
||||
@@ -394,4 +419,4 @@ function stop(): void {
|
||||
if (airtrailSyncTask) { airtrailSyncTask.stop(); airtrailSyncTask = null; }
|
||||
}
|
||||
|
||||
export { start, stop, startDemoReset, startTripReminders, startTodoReminders, startVersionCheck, startIdempotencyCleanup, startTrekPhotoCacheCleanup, startPlacePhotoCacheCleanup, startAirTrailSync, loadSettings, saveSettings, VALID_INTERVALS };
|
||||
export { start, stop, startDemoReset, startTripReminders, startTodoReminders, startVersionCheck, startIdempotencyCleanup, purgeExpiredIdempotencyKeys, startTrekPhotoCacheCleanup, startPlacePhotoCacheCleanup, startAirTrailSync, loadSettings, saveSettings, VALID_INTERVALS };
|
||||
|
||||
@@ -0,0 +1,58 @@
|
||||
/**
|
||||
* Idempotency key TTL cleanup (H6).
|
||||
*
|
||||
* The TREK client replays queued mutations with their X-Idempotency-Key on
|
||||
* reconnect, so the server must keep keys long enough to cover a realistic
|
||||
* offline window — otherwise a key GC'd before the device returns lets the
|
||||
* replay create a duplicate. The TTL was raised from 24h to 30d (overridable).
|
||||
*/
|
||||
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
|
||||
import { db } from '../../src/db/database';
|
||||
import { purgeExpiredIdempotencyKeys } from '../../src/scheduler';
|
||||
|
||||
const DAY = 24 * 60 * 60;
|
||||
const NOW = 2_000_000_000_000; // fixed ms so the test is deterministic
|
||||
const NOW_SEC = Math.floor(NOW / 1000);
|
||||
|
||||
function insertKey(key: string, ageSeconds: number): void {
|
||||
db.prepare(
|
||||
`INSERT INTO idempotency_keys (key, user_id, method, path, status_code, response_body, created_at)
|
||||
VALUES (?, 1, 'POST', '/x', 200, '{}', ?)`,
|
||||
).run(key, NOW_SEC - ageSeconds);
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
db.pragma('foreign_keys = OFF'); // fixtures reference a user we don't seed here
|
||||
db.prepare('DELETE FROM idempotency_keys').run();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
db.prepare('DELETE FROM idempotency_keys').run();
|
||||
db.pragma('foreign_keys = ON');
|
||||
delete process.env.IDEMPOTENCY_TTL_SECONDS;
|
||||
});
|
||||
|
||||
describe('purgeExpiredIdempotencyKeys', () => {
|
||||
it('removes keys older than the 30-day default, keeps recent ones', () => {
|
||||
insertKey('old', 31 * DAY);
|
||||
insertKey('fresh', 5 * DAY);
|
||||
|
||||
const removed = purgeExpiredIdempotencyKeys(NOW, undefined, db);
|
||||
|
||||
expect(removed).toBe(1);
|
||||
const keys = db.prepare('SELECT key FROM idempotency_keys').all().map((r: { key: string }) => r.key);
|
||||
expect(keys).toEqual(['fresh']);
|
||||
});
|
||||
|
||||
it('keeps a 25-day-old key that the old 24h TTL would have dropped', () => {
|
||||
insertKey('offline-trip', 25 * DAY);
|
||||
expect(purgeExpiredIdempotencyKeys(NOW, undefined, db)).toBe(0);
|
||||
expect(db.prepare('SELECT COUNT(*) c FROM idempotency_keys').get()).toMatchObject({ c: 1 });
|
||||
});
|
||||
|
||||
it('respects the IDEMPOTENCY_TTL_SECONDS override', () => {
|
||||
process.env.IDEMPOTENCY_TTL_SECONDS = String(DAY);
|
||||
insertKey('twoDays', 2 * DAY);
|
||||
expect(purgeExpiredIdempotencyKeys(NOW, undefined, db)).toBe(1);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user