[Regression on closed #463] TtlCache.invalidate() is still lost when it lands during an in-flight load, and the spec that covers it asserts nothing

Open Beginner friendly
  • Is something wrong?
#739 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
85/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
typescript
Domain
backend, testing

Research direction

Read src/analytics/ttl-cache.ts and the in-flight invalidation case in src/analytics/ttl-cache.spec.ts. Reproduce the race by starting a controlled load, invalidating before it resolves, then calling getOrLoad with a different loader and no second invalidation. Update the cache so that result is not cached, and make the spec prove the fresh loader runs and its value is returned.

Written by the indexing model from the issue text.

Description

bug testing

Location — src/analytics/ttl-cache.ts:11 (invalidate) and src/analytics/ttl-cache.ts:15-33 (getOrLoad, specifically the write at line 26 this.entry = { data, expiresAt: Date.now() + this.ttlMs };). Consumer: src/analytics/analytics.service.ts:91 (invalidatePlatformSummary, the @OnEvent(ANALYTICS_PLATFORM_INVALIDATE_EVENT) handler).

Problem — The lost-invalidation race closed as #463 is present again. invalidate() clears this.entry but has no way to tell an already-running loader that its result is stale, and the loader unconditionally publishes its result into this.entry when it resolves:

invalidate(): void {
  this.entry = null;
}

this.inflight = (async () => {
  try {
    const data = await loader();
    this.entry = { data, expiresAt: Date.now() + this.ttlMs };   // overwrites the invalidation
    return data;
  } finally {
    this.inflight = null;
  }
})();

There is no generation counter, epoch, or "invalidated while loading" flag. So if a bounty is created or paid (which emits ANALYTICS_PLATFORM_INVALIDATE_EVENT) while loadPlatformSummary() is awaiting its three Postgres aggregates, the invalidation is silently discarded and the pre-write snapshot is cached for a full TTL (ANALYTICS_PLATFORM_SUMMARY_TTL_MS, default 60s).

Why it matters — The invalidation exists precisely so the public homepage numbers update immediately after a bounty is funded or paid. The race window is the duration of the aggregate query, which is the window in which bounty writes are most likely (the write that emits the event is concurrent with homepage traffic by definition). The result is GET /analytics/platform serving totalBounties / totalPaidOut that are provably behind committed data for up to a minute, with no way for a client to force a refresh.

The test that is supposed to cover this does not: src/analytics/ttl-cache.spec.ts:55 ('handles invalidate while load is in flight') invalidates during the load, then at line 73 calls cache.invalidate() a second time before the next getOrLoad, so the assertion that the loader runs again passes regardless of whether the first, in-flight invalidation had any effect. Its own comment ("Invalidate should have cleared entry, or subsequent call fetches if expired/invalidated") concedes the test does not distinguish the two. That vacuous test is why #463 could be closed with the bug intact.

Reproduction / failure path

  1. In a unit test: const c = new TtlCache<string>(60000); start getOrLoad with a loader you control; call c.invalidate(); resolve the loader with 'stale'.
  2. Without a second invalidate(), call c.getOrLoad(loaderReturning('fresh')).
  3. It resolves to 'stale' and the second loader is never invoked. Expected: 'fresh'.
  4. End to end: issue GET /analytics/platform on a cold cache and, while the query is in flight, create and pay a bounty. The next 60s of homepage responses omit it.

Suggested fix

  • Add a monotonic private generation = 0; increment it in invalidate(); capture its value when the load starts and only assign this.entry if the captured value still equals the current generation (otherwise return the data to the waiting callers but do not cache it).
  • Rewrite the spec so the in-flight case is asserted directly: invalidate during the load, resolve the loader, then call getOrLoad with a different loader and assert the second loader ran and its value was returned — with no extra invalidate() call in between.
Dominant language
TypeScript
Stars
2
Forks
90
Avg merge
31m
Merged PRs (30d)
20

Getting set up

  • Ships a Dockerfile or Docker Compose file
  • No pull request template
  • No contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from MergeFi/backend

All issues in MergeFi/backend

Similar issues

More TypeScript issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.