diff --git a/.oxlintrc.json b/.oxlintrc.json index f64cf1585..33d149dba 100644 --- a/.oxlintrc.json +++ b/.oxlintrc.json @@ -191,6 +191,8 @@ // a live actor handle: subscription and mailbox state xstate mutates internally, no // readonly form { "from": "package", "name": "ActorRef", "package": "xstate" }, + // an actor clock handle: its setTimeout/clearTimeout methods are behaviour, not data + { "from": "package", "name": "Clock", "package": "xstate" }, { "from": "package", "name": ["CryptoKey", "Request", "Response", "AbortSignal"], diff --git a/libs/game/idle-client/src/submission/checkpoint-activity-machine.test.ts b/libs/game/idle-client/src/submission/checkpoint-activity-machine.test.ts index 1a138ab3f..4c82f8ecd 100644 --- a/libs/game/idle-client/src/submission/checkpoint-activity-machine.test.ts +++ b/libs/game/idle-client/src/submission/checkpoint-activity-machine.test.ts @@ -5,11 +5,12 @@ import { resolveServiceURL } from '@vers/mock-services'; import { mockActivityService } from '@vers/mock-services/activity'; import { waitFor } from '@vers/test-utils'; import { HttpResponse } from 'msw'; -import { createActor } from 'xstate'; +import { SimulatedClock, createActor } from 'xstate'; import { server } from '../mocks/node'; import { createMockCheckpointBatchEntry } from '../test-utils/factories/create-mock-checkpoint-batch-entry'; import type { CheckpointActivityEmittedEvent } from './checkpoint-activity-machine'; import { checkpointActivityMachine } from './checkpoint-activity-machine'; +import { PROGRESS_FLUSH_INTERVAL_MS, RETRY_BACKOFF_CAP_MS } from './constants'; import type { ActivityServiceClient } from './types'; import { writeQueuedCheckpoint } from './write-queued-checkpoint'; @@ -18,11 +19,11 @@ function setupTest( activityID?: string; latestQueuedVersion?: number; onAcked?: (activityID: string, appendedHead: number) => void; - retryTimings?: Readonly<{ maxTimeout: number; minTimeout: number }>; signal?: AbortSignal; terminalQueued?: boolean; }> = {}, ) { + const clock = new SimulatedClock(); const link = new RPCLink({ url: `${resolveServiceURL('activity')}/rpc` }); const client: ActivityServiceClient = createORPCClient(link); @@ -32,6 +33,7 @@ function setupTest( const emitted: Array = []; const actor = createActor(checkpointActivityMachine, { + clock, input: { activityID: config.activityID ?? 'activity-machine-test', client, @@ -42,7 +44,7 @@ function setupTest( onEvicted: undefined, onInvalid, onServerContact: undefined, - retryTimings: config.retryTimings ?? { maxTimeout: 300_000, minTimeout: 10_000 }, + retryTimings: { maxTimeout: RETRY_BACKOFF_CAP_MS, minTimeout: PROGRESS_FLUSH_INTERVAL_MS }, scheduleProgressFlush, signal: config.signal, terminalQueued: config.terminalQueued ?? false, @@ -55,7 +57,7 @@ function setupTest( actor.start(); - return { actor, emitted, onAcked, onInvalid, scheduleProgressFlush }; + return { actor, clock, emitted, onAcked, onInvalid, scheduleProgressFlush }; } test('it arms the shared progress window and waits for an explicit flush-due event, not a timer', async () => { @@ -116,14 +118,10 @@ test('it moves to retrying and reports the batch held on a transport failure', a ]); }); -test('it retries on a real backoff timer, driven by tiny retryTimings, until the batch lands', async () => { +test('it retries once the backoff delay elapses, until the batch lands', async () => { let shouldFail = true; const track = mock<() => void>(); - - const ctx = setupTest({ - activityID: 'backoff-machine-activity', - retryTimings: { maxTimeout: 20, minTimeout: 5 }, - }); + const ctx = setupTest({ activityID: 'backoff-machine-activity' }); server.use( mockActivityService.trackActivityProgress.handler(() => { @@ -150,20 +148,18 @@ test('it retries on a real backoff timer, driven by tiny retryTimings, until the shouldFail = false; + ctx.clock.increment(RETRY_BACKOFF_CAP_MS); + await waitFor(() => { expect(ctx.actor.getSnapshot().matches('evicted')).toBeTrue(); }); - expect(track.mock.calls.length).toBeGreaterThan(1); + expect(track).toHaveBeenCalledTimes(2); }); test('it runs an immediate attempt from retrying without advancing the backoff attempt counter', async () => { const track = mock<() => void>(); - - const ctx = setupTest({ - activityID: 'flush-now-retrying-activity', - retryTimings: { maxTimeout: 100_000, minTimeout: 50_000 }, - }); + const ctx = setupTest({ activityID: 'flush-now-retrying-activity' }); server.use( mockActivityService.trackActivityProgress.handler(() => { @@ -198,11 +194,7 @@ test('it runs an immediate attempt from retrying without advancing the backoff a test('it resets the backoff attempt counter and re-flushes when flushHeld arrives while retrying', async () => { let shouldFail = true; - - const ctx = setupTest({ - activityID: 'flush-held-retrying-activity', - retryTimings: { maxTimeout: 20, minTimeout: 5 }, - }); + const ctx = setupTest({ activityID: 'flush-held-retrying-activity' }); server.use( mockActivityService.trackActivityProgress.handler(() => { @@ -222,7 +214,14 @@ test('it resets the backoff attempt counter and re-flushes when flushHeld arrive ctx.actor.send({ isTerminal: true, type: 'QUEUED', version: 1 }); await waitFor(() => { - expect(ctx.actor.getSnapshot().context.retryAttempt).toBeGreaterThan(0); + expect(ctx.actor.getSnapshot().matches('retrying')).toBeTrue(); + }); + + ctx.clock.increment(RETRY_BACKOFF_CAP_MS); + + await waitFor(() => { + expect(ctx.actor.getSnapshot().matches('retrying')).toBeTrue(); + expect(ctx.actor.getSnapshot().context.retryAttempt).toBe(1); }); shouldFail = false; @@ -283,7 +282,6 @@ test('it exits retrying with no re-entry once the shutdown signal aborts', async const ctx = setupTest({ activityID: 'shutdown-abort-activity', - retryTimings: { maxTimeout: 100_000, minTimeout: 50_000 }, signal: shutdownController.signal, }); @@ -314,8 +312,11 @@ test('it exits retrying with no re-entry once the shutdown signal aborts', async const callsAtAbort = track.mock.calls.length; + // an abort that failed to cancel the backoff would fire an attempt as soon as it elapses + ctx.clock.increment(RETRY_BACKOFF_CAP_MS); + await new Promise((resolve) => { - setTimeout(resolve, 40); + setTimeout(resolve, 0); }); expect(track.mock.calls.length).toBe(callsAtAbort); @@ -404,11 +405,7 @@ test('it reports a callback failure and holds the batch without starting a retry test('it resets the backoff attempt counter once a retried batch lands, so a later outage starts at the base window', async () => { let shouldFail = true; - - const ctx = setupTest({ - activityID: 'backoff-reset-machine-activity', - retryTimings: { maxTimeout: 20, minTimeout: 5 }, - }); + const ctx = setupTest({ activityID: 'backoff-reset-machine-activity' }); server.use( mockActivityService.trackActivityProgress.handler(() => { @@ -429,11 +426,20 @@ test('it resets the backoff attempt counter once a retried batch lands, so a lat ctx.actor.send({ type: 'FLUSH_DUE' }); await waitFor(() => { - expect(ctx.actor.getSnapshot().context.retryAttempt).toBeGreaterThan(0); + expect(ctx.actor.getSnapshot().matches('retrying')).toBeTrue(); + }); + + ctx.clock.increment(RETRY_BACKOFF_CAP_MS); + + await waitFor(() => { + expect(ctx.actor.getSnapshot().matches('retrying')).toBeTrue(); + expect(ctx.actor.getSnapshot().context.retryAttempt).toBe(1); }); shouldFail = false; + ctx.clock.increment(RETRY_BACKOFF_CAP_MS); + await waitFor(() => { expect(ctx.actor.getSnapshot().matches('idle')).toBeTrue(); }); diff --git a/libs/game/idle-client/src/submission/create-checkpoint-submitter.test.ts b/libs/game/idle-client/src/submission/create-checkpoint-submitter.test.ts index 24bb17e21..eeea9ecb5 100644 --- a/libs/game/idle-client/src/submission/create-checkpoint-submitter.test.ts +++ b/libs/game/idle-client/src/submission/create-checkpoint-submitter.test.ts @@ -6,11 +6,13 @@ import { mockActivityService } from '@vers/mock-services/activity'; import { waitFor } from '@vers/test-utils'; import { HttpResponse, http } from 'msw'; import invariant from 'tiny-invariant'; +import { SimulatedClock } from 'xstate'; import { server } from '../mocks/node'; import { createMockCheckpointBatchEntry } from '../test-utils/factories/create-mock-checkpoint-batch-entry'; import { createMockCompletedCheckpoint } from '../test-utils/factories/create-mock-completed-checkpoint'; import { createMockProgressCheckpoint } from '../test-utils/factories/create-mock-progress-checkpoint'; import { createMockStartedCheckpoint } from '../test-utils/factories/create-mock-started-checkpoint'; +import { RETRY_BACKOFF_CAP_MS } from './constants'; import { createCheckpointSubmitter } from './create-checkpoint-submitter'; import { readQueuedCheckpoints } from './read-queued-checkpoints'; import type { ActivityServiceClient } from './types'; @@ -20,11 +22,12 @@ function setupTest( config: Readonly<{ onAcked?: (activityID: string, appendedHead: number) => void; onServerContact?: () => void; - retryTimings?: Readonly<{ maxTimeout: number; minTimeout: number }>; scheduleFlush?: (flush: () => Promise) => void; signal?: AbortSignal; }> = {}, ) { + const clock = new SimulatedClock(); + const link = new RPCLink<{ traceparent?: string }>({ headers: (options) => options.context?.traceparent === undefined @@ -45,6 +48,7 @@ function setupTest( const submitter = createCheckpointSubmitter({ client, + clock, onAcked, onCapped, onEvicted, @@ -58,6 +62,7 @@ function setupTest( return { client, + clock, onAcked, onCapped, onEvicted, @@ -575,7 +580,7 @@ test('it drops a checkpoint for an activity that was never registered', async () test('it holds the queue on a transport failure and retries it in the background until it lands', async () => { let shouldFail = true; const track = mock<() => void>(); - const ctx = setupTest({ retryTimings: { maxTimeout: 20, minTimeout: 5 } }); + const ctx = setupTest(); server.use( mockActivityService.trackActivityProgress.handler(() => { @@ -606,18 +611,20 @@ test('it holds the queue on a transport failure and retries it in the background shouldFail = false; + ctx.clock.increment(RETRY_BACKOFF_CAP_MS); + await waitFor(async () => { const remaining = await readQueuedCheckpoints('transport-failure-activity'); expect(remaining).toStrictEqual([]); }); - expect(track.mock.calls.length).toBeGreaterThan(1); + expect(track).toHaveBeenCalledTimes(2); }); test('it holds the queue and retries in the background identically on UNAUTHORIZED', async () => { let shouldFail = true; - const ctx = setupTest({ retryTimings: { maxTimeout: 20, minTimeout: 5 } }); + const ctx = setupTest(); server.use( mockActivityService.trackActivityProgress.handler((opts) => { @@ -642,6 +649,8 @@ test('it holds the queue and retries in the background identically on UNAUTHORIZ shouldFail = false; + ctx.clock.increment(RETRY_BACKOFF_CAP_MS); + await waitFor(async () => { const remaining = await readQueuedCheckpoints('unauthorized-backoff-activity'); @@ -658,13 +667,12 @@ test('it reports an unexpected retry-loop failure and starts a fresh loop on the const ctx = setupTest({ // scripts the unexpected failure: the first acknowledged flush throws from inside the retry - // loop's attempt, which p-retry treats as terminal rather than another backoff step + // attempt's ack callback, which kills the loop rather than taking another backoff step onAcked: () => { if (ackShouldThrow) { throw ackFailure; } }, - retryTimings: { maxTimeout: 20, minTimeout: 5 }, }); server.use( @@ -690,6 +698,8 @@ test('it reports an unexpected retry-loop failure and starts a fresh loop on the transportShouldFail = false; + ctx.clock.increment(RETRY_BACKOFF_CAP_MS); + await waitFor(() => { expect(ctx.onRetryFailed).toHaveBeenCalledExactlyOnceWith( 'retry-loop-failure-activity', @@ -709,6 +719,8 @@ test('it reports an unexpected retry-loop failure and starts a fresh loop on the transportShouldFail = false; + ctx.clock.increment(RETRY_BACKOFF_CAP_MS); + await waitFor(async () => { const remaining = await readQueuedCheckpoints('retry-loop-failure-activity'); @@ -717,7 +729,7 @@ test('it reports an unexpected retry-loop failure and starts a fresh loop on the }); test('it keeps a single retry loop per activity across repeated failures', async () => { - const ctx = setupTest({ retryTimings: { maxTimeout: 20, minTimeout: 5 } }); + const ctx = setupTest(); server.use(mockActivityService.trackActivityProgress.handler(() => HttpResponse.error())); @@ -730,7 +742,7 @@ test('it keeps a single retry loop per activity across repeated failures', async // each terminal submission flushes and fails immediately, but only the first failure may start // a loop — the second reports held again but defers to the running loop instead of starting a - // second one + // second one; the simulated clock never advances, so no backoff attempt can add a third report await ctx.submitter.submit('single-retry-activity', createMockCompletedCheckpoint()); await ctx.submitter.submit('single-retry-activity', createMockCompletedCheckpoint()); @@ -740,7 +752,7 @@ test('it keeps a single retry loop per activity across repeated failures', async test('it supersedes a running retry loop when flushHeld runs, delivering exactly once', async () => { let shouldFail = true; const track = mock<() => void>(); - const ctx = setupTest({ retryTimings: { maxTimeout: 20, minTimeout: 5 } }); + const ctx = setupTest(); server.use( mockActivityService.trackActivityProgress.handler(() => { @@ -776,8 +788,10 @@ test('it supersedes a running retry loop when flushHeld runs, delivering exactly expect(remaining).toStrictEqual([]); // the superseded loop never fires a duplicate delivery once its old backoff would have elapsed + ctx.clock.increment(RETRY_BACKOFF_CAP_MS); + await new Promise((resolve) => { - setTimeout(resolve, 40); + setTimeout(resolve, 0); }); expect(track).toHaveBeenCalledTimes(2); @@ -786,7 +800,7 @@ test('it supersedes a running retry loop when flushHeld runs, delivering exactly test('it flushes every held activity immediately and resets their backoff', async () => { let shouldFail = true; const track = mock<() => void>(); - const ctx = setupTest({ retryTimings: { maxTimeout: 20, minTimeout: 5 } }); + const ctx = setupTest(); server.use( mockActivityService.trackActivityProgress.handler(() => { @@ -822,7 +836,7 @@ test('it flushes every held activity immediately and resets their backoff', asyn test('it resends a held terminal checkpoint via the retry loop and empties the queue', async () => { let shouldFail = true; - const ctx = setupTest({ retryTimings: { maxTimeout: 20, minTimeout: 5 } }); + const ctx = setupTest(); server.use( mockActivityService.trackActivityProgress.handler(() => { @@ -849,6 +863,8 @@ test('it resends a held terminal checkpoint via the retry loop and empties the q shouldFail = false; + ctx.clock.increment(RETRY_BACKOFF_CAP_MS); + await waitFor(async () => { const remaining = await readQueuedCheckpoints('held-terminal-activity'); @@ -860,11 +876,7 @@ test('it ends the retry loop silently once the shutdown signal aborts, with no f const shutdownController = new AbortController(); const track = mock<() => void>(); - - const ctx = setupTest({ - retryTimings: { maxTimeout: 20, minTimeout: 5 }, - signal: shutdownController.signal, - }); + const ctx = setupTest({ signal: shutdownController.signal }); server.use( mockActivityService.trackActivityProgress.handler(() => { @@ -889,8 +901,11 @@ test('it ends the retry loop silently once the shutdown signal aborts, with no f const callsAtAbort = track.mock.calls.length; + // an abort that failed to cancel the loop would fire an attempt as soon as the backoff elapses + ctx.clock.increment(RETRY_BACKOFF_CAP_MS); + await new Promise((resolve) => { - setTimeout(resolve, 40); + setTimeout(resolve, 0); }); expect(track.mock.calls.length).toBe(callsAtAbort); diff --git a/libs/game/idle-client/src/submission/create-checkpoint-submitter.ts b/libs/game/idle-client/src/submission/create-checkpoint-submitter.ts index d9605461a..1ff92b995 100644 --- a/libs/game/idle-client/src/submission/create-checkpoint-submitter.ts +++ b/libs/game/idle-client/src/submission/create-checkpoint-submitter.ts @@ -1,6 +1,7 @@ import type { ActivityCheckpoint } from '@vers/idle-core'; import { ActivityCheckpointType } from '@vers/idle-core'; import invariant from 'tiny-invariant'; +import type { ActorOptions, AnyActorLogic } from 'xstate'; import { createActor, waitFor } from 'xstate'; import { buildCheckpointBatchEntry } from './build-checkpoint-batch-entry'; import type { CheckpointActivityChildRef } from './checkpoint-submitter-machine'; @@ -143,10 +144,11 @@ interface CreateCheckpointSubmitterOptions { readonly scheduleFlush?: (flush: () => Promise) => void; /** - * Overrides a held batch's retry backoff timings — test-only, so a suite can drive the retry - * loop without waiting out real backoff delays. + * Overrides the actor clock driving every retry-backoff delay — test-only, so a suite advances + * a simulated clock explicitly instead of waiting out real backoff windows. Spawned per-activity + * children inherit it through the actor system. */ - readonly retryTimings?: { readonly maxTimeout: number; readonly minTimeout: number }; + readonly clock?: ActorOptions['clock']; /** * The runtime's shutdown signal, composed into every held-batch retry loop's own controller so @@ -200,9 +202,12 @@ export function createCheckpointSubmitter( const writeCursors = new Map(); const registrations = new Map>(); - const parentActor = createActor(checkpointSubmitterMachine).start(); + // an options object carrying `clock: undefined` clobbers the actor system's default clock, so + // the option is only forwarded when a caller actually injected one + const actorOptions = options.clock === undefined ? undefined : { clock: options.clock }; + const parentActor = createActor(checkpointSubmitterMachine, actorOptions).start(); - const retryTimings = options.retryTimings ?? { + const retryTimings = { maxTimeout: RETRY_BACKOFF_CAP_MS, minTimeout: PROGRESS_FLUSH_INTERVAL_MS, };