test(SessionStore): drive expiry timers with a fake clock

The SessionStore spec scheduled sessions to expire a few hundred ms in
the future and then slept on the real clock. Under CPU load the DB
round trips between "set" and the cleanup/cancel step could take longer
than the expiry window, so the store's own expiry timer fired mid-setup
and purged the record (e.g. "shutdown cancels timeouts" and "destroy
cancels the timeout" failing with the record missing). Reproduced 4/20
runs with the CPU oversubscribed.

Give SessionStore a `_timers` seam (defaults to globalThis) for the
Date/setTimeout/clearTimeout used by its expiration timers, and have the
spec install a sinon fake clock on it. The DB keeps real timers, so
time only moves when a test advances it, and the spec awaits the DB
reads started by any timer that fired. The "timer must not fire" tests
now also assert the timer callback never ran, and the expiry tests
check the record survives until the exact expiry instant.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01V4cYEvi9Y4BTPntnkV9tzh

Author: JohnMcLear

src/node/db/SessionStore.ts

@@ -48,6 +48,10 @@ class SessionStore extends expressSession.Store {
     this._expirations = new Map();
     this._cleanupTimer = null;
     this._cleanupRunning = false;
+    // Source of `Date`, `setTimeout` and `clearTimeout` for the session expiration timers. Only
+    // replaced by tests, which swap in a fake clock so expiry can be checked without depending on
+    // wall-clock timing.
+    this._timers = globalThis;
   }
 
   /**
@@ -73,7 +77,7 @@ class SessionStore extends expressSession.Store {
   }
 
   shutdown() {
-    for (const {timeout} of this._expirations.values()) clearTimeout(timeout);
+    for (const {timeout} of this._expirations.values()) this._timers.clearTimeout(timeout);
     if (this._cleanupTimer) {
       clearTimeout(this._cleanupTimer);
       this._cleanupTimer = null;
@@ -157,14 +161,14 @@ class SessionStore extends expressSession.Store {
 
   async _updateExpirations(sid: string, sess: any, updateDbExp = true) {
     const exp = this._expirations.get(sid) || {};
-    clearTimeout(exp.timeout);
+    this._timers.clearTimeout(exp.timeout);
     // @ts-ignore
     const {cookie: {expires} = {}} = sess || {};
     if (expires) {
       const sessExp = new Date(expires).getTime();
       if (updateDbExp) exp.db = sessExp;
       exp.real = Math.max(exp.real || 0, exp.db || 0, sessExp);
-      const now = Date.now();
+      const now = this._timers.Date.now();
       if (exp.real <= now) return await this._destroy(sid);
       // If reading from the database, update the expiration with the latest value from touch() so
       // that touch() appears to write to the database every time even though it doesn't.
@@ -173,11 +177,12 @@ class SessionStore extends expressSession.Store {
       // the in-memory expiry first — touch() may have extended it without rescheduling the timeout
       // (e.g., if touch's clearTimeout raced with the timer on a slow system). If the session was
       // extended, reschedule instead of reading from the DB which may return stale cached data.
-      exp.timeout = setTimeout(() => {
+      exp.timeout = this._timers.setTimeout(() => {
         const currentExp = this._expirations.get(sid);
-        if (currentExp && currentExp.real > Date.now()) {
+        if (currentExp && currentExp.real > this._timers.Date.now()) {
           // Expiry was extended (e.g., by touch). Reschedule.
-          currentExp.timeout = setTimeout(() => this._get(sid), currentExp.real - Date.now());
+          currentExp.timeout = this._timers.setTimeout(
+              () => this._get(sid), currentExp.real - this._timers.Date.now());
           return;
         }
         // Use this._get(), not this._destroy(), to query the DB for the latest expiration in case
@@ -210,7 +215,7 @@ class SessionStore extends expressSession.Store {
 
   async _destroy(sid:string) {
     logger.debug(`DESTROY ${sid}`);
-    clearTimeout((this._expirations.get(sid) || {}).timeout);
+    this._timers.clearTimeout((this._expirations.get(sid) || {}).timeout);
     this._expirations.delete(sid);
     await DB.remove(`sessionstorage:${sid}`);
   }

src/tests/backend/specs/SessionStore.ts

@@ -5,6 +5,7 @@ import {strict as assert} from 'assert';
 const common = require('../common');
 const db = require('../../../node/db/DB');
 import util from 'util';
+import sinon from 'sinon';
 
 type Session = {
   set: (sid: string|null,sess:any, sess2:any) => void;
@@ -15,36 +16,55 @@ type Session = {
   startCleanup: () => void;
   _cleanup: () => Promise<void>;
   _cleanupTimer: any;
+  _timers: any;
+  _get: (sid: string) => Promise<any>;
 }
 
 describe(__filename, function () {
   let ss: Session|null;
   let sid: string|null;
+  // Fake clock driving the store's expiration timers and its notion of "now". The DB keeps using
+  // real timers, so awaiting DB calls never moves the store's clock: expiry is only reached when a
+  // test explicitly advances time, no matter how slow the machine is.
+  let clock: sinon.SinonFakeTimers|null = null;
+  // Records the DB reads kicked off by the store's expiration timers (the timer callbacks call
+  // `this._get()`; the public callback API does not go through the instance property).
+  let timerGets: sinon.SinonSpy;
 
   const set = async (sess: string|null) => await util.promisify(ss!.set).call(ss, sid, sess);
   const get = async () => await util.promisify(ss!.get).call(ss, sid);
   const destroy = async () => await util.promisify(ss!.destroy).call(ss, sid);
   const touch = async (sess: Session) => await util.promisify(ss!.touch).call(ss, sid, sess);
 
-  // Poll until `cond` is true. Used in place of fixed sleeps for "the cleanup timer should have
-  // fired by now" assertions — passes immediately when cleanup completes so tests stay fast,
-  // but tolerates slow CI runners where the event loop may be delayed by hundreds of ms.
-  const eventually = async (cond: () => Promise<boolean>, maxMs = 2000, intervalMs = 25) => {
-    const deadline = Date.now() + maxMs;
-    // First check is immediate so the helper doesn't add a fixed delay.
-    while (true) {
-      if (await cond()) return;
-      if (Date.now() >= deadline) throw new Error(`condition not met within ${maxMs}ms`);
-      await new Promise((r) => setTimeout(r, intervalMs));
-    }
+  const now = () => clock!.now;
+
+  // Replaces `ss` with a new store whose timers run on a fresh fake clock.
+  const newStore = (refresh?: number) => {
+    if (ss != null) ss.shutdown();
+    if (clock != null) clock.uninstall();
+    ss = new SessionStore(refresh);
+    const timers = {Date, setTimeout, clearTimeout};
+    // `global` is supported by sinon (it fakes the given object instead of globalThis) but is
+    // missing from @types/sinon, hence the cast.
+    clock = sinon.useFakeTimers(
+        {global: timers, now: Date.now(), toFake: ['Date', 'setTimeout', 'clearTimeout']} as any);
+    ss!._timers = timers;
+    timerGets = sinon.spy(ss!, '_get');
+  };
+
+  // Advances the store's clock, firing any due expiration timers, and waits for the DB work those
+  // timers started to finish.
+  const advance = async (ms: number) => {
+    clock!.tick(ms);
+    await Promise.all(timerGets.returnValues);
   };
 
   before(async function () {
     await common.init();
   });
 
   beforeEach(async function () {
-    ss = new SessionStore();
+    newStore();
     sid = common.randomString();
   });
 
@@ -53,6 +73,8 @@ describe(__filename, function () {
       if (sid != null) await destroy();
       ss.shutdown();
     }
+    if (clock != null) clock.uninstall();
+    clock = null;
     sid = null;
     ss = null;
   });
@@ -70,11 +92,14 @@ describe(__filename, function () {
     });
 
     it('set of session that expires', async function () {
-      const sess:any  = {foo: 'bar', cookie: {expires: new Date(Date.now() + 300)}};
+      const sess:any  = {foo: 'bar', cookie: {expires: new Date(now() + 300)}};
       await set(sess);
       assert.equal(JSON.stringify(await db.get(`sessionstorage:${sid}`)), JSON.stringify(sess));
       // Writing should start a timeout that purges the record once expiry passes.
-      await eventually(async () => await db.get(`sessionstorage:${sid}`) == null);
+      await advance(299);
+      assert.equal(JSON.stringify(await db.get(`sessionstorage:${sid}`)), JSON.stringify(sess));
+      await advance(1);
+      assert(await db.get(`sessionstorage:${sid}`) == null);
     });
 
     it('set of already expired session', async function () {
@@ -87,17 +112,19 @@ describe(__filename, function () {
     it('switch from non-expiring to expiring', async function () {
       const sess:any  = {foo: 'bar'};
       await set(sess);
-      const sess2:any  = {foo: 'bar', cookie: {expires: new Date(Date.now() + 300)}};
+      const sess2:any  = {foo: 'bar', cookie: {expires: new Date(now() + 300)}};
       await set(sess2);
-      await eventually(async () => await db.get(`sessionstorage:${sid}`) == null);
+      await advance(300);
+      assert(await db.get(`sessionstorage:${sid}`) == null);
     });
 
     it('switch from expiring to non-expiring', async function () {
-      const sess:any  = {foo: 'bar', cookie: {expires: new Date(Date.now() + 300)}};
+      const sess:any  = {foo: 'bar', cookie: {expires: new Date(now() + 300)}};
       await set(sess);
       const sess2:any  = {foo: 'bar'};
       await set(sess2);
-      await new Promise((resolve) => setTimeout(resolve, 330));
+      await advance(330);
+      assert(timerGets.notCalled);
       assert.equal(JSON.stringify(await db.get(`sessionstorage:${sid}`)), JSON.stringify(sess2));
     });
   });
@@ -120,11 +147,12 @@ describe(__filename, function () {
     });
 
     it('get of record from previous run (not yet expired)', async function () {
-      const sess = {foo: 'bar', cookie: {expires: new Date(Date.now() + 300)}};
+      const sess = {foo: 'bar', cookie: {expires: new Date(now() + 300)}};
       await db.set(`sessionstorage:${sid}`, sess);
       assert.equal(JSON.stringify(await get()), JSON.stringify(sess));
       // Reading should start a timeout that purges the record once expiry passes.
-      await eventually(async () => await db.get(`sessionstorage:${sid}`) == null);
+      await advance(300);
+      assert(await db.get(`sessionstorage:${sid}`) == null);
     });
 
     it('get of record from previous run (already expired)', async function () {
@@ -135,47 +163,47 @@ describe(__filename, function () {
     });
 
     it('external expiration update is picked up', async function () {
-      const sess:any  = {foo: 'bar', cookie: {expires: new Date(Date.now() + 300)}};
+      const sess:any  = {foo: 'bar', cookie: {expires: new Date(now() + 300)}};
       await set(sess);
       assert.equal(JSON.stringify(await get()), JSON.stringify(sess));
-      const sess2 = {...sess, cookie: {expires: new Date(Date.now() + 600)}};
+      const sess2 = {...sess, cookie: {expires: new Date(now() + 600)}};
       await db.set(`sessionstorage:${sid}`, sess2);
       assert.equal(JSON.stringify(await get()), JSON.stringify(sess2));
-      await new Promise((resolve) => setTimeout(resolve, 330));
+      await advance(330);
       // The original timeout should not have fired.
+      assert(timerGets.notCalled);
       assert.equal(JSON.stringify(await get()), JSON.stringify(sess2));
     });
   });
 
   describe('shutdown', function () {
     it('shutdown cancels timeouts', async function () {
-      // expires=500 / wait=700 keeps comfortable headroom on slow CI: setup
-      // (set+get+shutdown) must finish before the timer would fire (500ms is plenty), and the
-      // 700ms wait is past the original expiry so a cancelled timer would have fired by then.
-      const sess:any  = {foo: 'bar', cookie: {expires: new Date(Date.now() + 500)}};
+      const sess:any  = {foo: 'bar', cookie: {expires: new Date(now() + 300)}};
       await set(sess);
       assert.equal(JSON.stringify(await get()), JSON.stringify(sess));
       ss!.shutdown();
-      await new Promise((resolve) => setTimeout(resolve, 700));
+      await advance(330);
+      assert(timerGets.notCalled);
       // The record should not have been automatically purged.
       assert.equal(JSON.stringify(await db.get(`sessionstorage:${sid}`)), JSON.stringify(sess));
     });
   });
 
   describe('destroy', function () {
     it('destroy deletes the database record', async function () {
-      const sess:any  = {cookie: {expires: new Date(Date.now() + 300)}};
+      const sess:any  = {cookie: {expires: new Date(now() + 300)}};
       await set(sess);
       await destroy();
       assert(await db.get(`sessionstorage:${sid}`) == null);
     });
 
     it('destroy cancels the timeout', async function () {
-      const sess:any  = {cookie: {expires: new Date(Date.now() + 300)}};
+      const sess:any  = {cookie: {expires: new Date(now() + 300)}};
       await set(sess);
       await destroy();
       await db.set(`sessionstorage:${sid}`, sess);
-      await new Promise((resolve) => setTimeout(resolve, 330));
+      await advance(330);
+      assert(timerGets.notCalled);
       assert.equal(JSON.stringify(await db.get(`sessionstorage:${sid}`)), JSON.stringify(sess));
     });
 
@@ -186,13 +214,13 @@ describe(__filename, function () {
 
   describe('touch without refresh', function () {
     it('touch before set is equivalent to set if session expires', async function () {
-      const sess:any  = {cookie: {expires: new Date(Date.now() + 1000)}};
+      const sess:any  = {cookie: {expires: new Date(now() + 1000)}};
       await touch(sess);
       assert.equal(JSON.stringify(await get()), JSON.stringify(sess));
     });
 
     it('touch updates observed expiration but not database', async function () {
-      const start = Date.now();
+      const start = now();
       const sess:any  = {cookie: {expires: new Date(start + 200)}};
       await set(sess);
       const sess2:any  = {cookie: {expires: new Date(start + 12000)}};
@@ -204,52 +232,57 @@ describe(__filename, function () {
 
   describe('touch with refresh', function () {
     beforeEach(async function () {
-      ss = new SessionStore(200);
+      newStore(200);
     });
 
     it('touch before set is equivalent to set if session expires', async function () {
-      const sess:any  = {cookie: {expires: new Date(Date.now() + 1000)}};
+      const sess:any  = {cookie: {expires: new Date(now() + 1000)}};
       await touch(sess);
       assert.equal(JSON.stringify(await get()), JSON.stringify(sess));
     });
 
     it('touch before eligible for refresh updates expiration but not DB', async function () {
-      const now = Date.now();
-      const sess:any  = {foo: 'bar', cookie: {expires: new Date(now + 1000)}};
+      const start = now();
+      const sess:any  = {foo: 'bar', cookie: {expires: new Date(start + 1000)}};
       await set(sess);
-      const sess2:any  = {foo: 'bar', cookie: {expires: new Date(now + 1001)}};
+      const sess2:any  = {foo: 'bar', cookie: {expires: new Date(start + 1001)}};
       await touch(sess2);
       assert.equal(JSON.stringify(await db.get(`sessionstorage:${sid}`)), JSON.stringify(sess));
       assert.equal(JSON.stringify(await get()), JSON.stringify(sess2));
     });
 
     it('touch before eligible for refresh updates timeout', async function () {
-      const start = Date.now();
+      const start = now();
       const sess:any  = {foo: 'bar', cookie: {expires: new Date(start + 200)}};
       await set(sess);
-      await new Promise((resolve) => setTimeout(resolve, 100));
+      await advance(100);
       const sess2:any  = {foo: 'bar', cookie: {expires: new Date(start + 399)}};
       await touch(sess2);
-      await new Promise((resolve) => setTimeout(resolve, 110));
+      await advance(110);
+      // Past the original expiry, but the touch moved the timeout out.
+      assert(timerGets.notCalled);
       assert.equal(JSON.stringify(await db.get(`sessionstorage:${sid}`)), JSON.stringify(sess));
       assert.equal(JSON.stringify(await get()), JSON.stringify(sess2));
+      // The rescheduled timeout purges the record once the touched expiry passes.
+      await advance(189);
+      assert(await db.get(`sessionstorage:${sid}`) == null);
     });
 
     it('touch after eligible for refresh updates db', async function () {
-      const start = Date.now();
+      const start = now();
       const sess:any  = {foo: 'bar', cookie: {expires: new Date(start + 2000)}};
       await set(sess);
-      await new Promise((resolve) => setTimeout(resolve, 100));
+      await advance(100);
       const sess2:any  = {foo: 'bar', cookie: {expires: new Date(start + 4000)}};
       await touch(sess2);
-      await new Promise((resolve) => setTimeout(resolve, 110));
+      await advance(110);
       assert.equal(JSON.stringify(await db.get(`sessionstorage:${sid}`)), JSON.stringify(sess2));
       assert.equal(JSON.stringify(await get()), JSON.stringify(sess2));
     });
 
     it('refresh=0 updates db every time', async function () {
-      ss = new SessionStore(0);
-      const sess:any  = {foo: 'bar', cookie: {expires: new Date(Date.now() + 1000)}};
+      newStore(0);
+      const sess:any  = {foo: 'bar', cookie: {expires: new Date(now() + 1000)}};
       await set(sess);
       await db.remove(`sessionstorage:${sid}`);
       await touch(sess); // No change in expiration time.