`@wora/relay-store` - Stale data served forever after query fragment change

[matt-dalton]

I might be misunderstanding internals here, but we have a product bug and it's looking like it might be an underlying library issue.

It looks as though the reproduction path for us was:

1. User runs a query Q1 with operation hash H1 against store-or-network. Records are persisted; root entry for H1 is persisted with a valid fetchTime.
2. In a later release we remove a fragment from that query in a clean-up. This changes the cache key used (call it H2)
3. User then opens the app and this query never runs again. So data is now stuck from before they upgraded to this version.

Looking at the internals, after the cache key is changed and the user opens the page again:

• _get_or_create_root_entry finds no existing entry under H2 → creates a new one with fetchTime: null.
  • Records reachable from Q1 are still in the in-memory RecordSource because they are also referenced by other retained operations (or were rehydrated alongside the persisted set). DataChecker.check returns status: 'available'.
• Store.prototype.check calls getAvailabilityStatus with operationFetchTime = null and the inner status 'available'.
• getAvailabilityStatus falls through every guard and returns { status: 'available', fetchTime: null }.
• store-or-network short-circuits, no network request is issued, the user is served whatever records were already in memory.
• Because no network response ever lands, Store.prototype.notify never fires for H2 → fetchTime stays null → step 5 repeats forever.

This is on v7.0.0 of the library - haven't upgrade to v8 yet. But the code looks the same so I assume the issue is the same.

Possible root cause

getAvailabilityStatus (lib/Store.js, current 8.0.0):

function getAvailabilityStatus(operationAvailability, operationLastWrittenAt, operationFetchTime, queryCacheExpirationTime, staleForExpiration) {
    // …invalidation check…
    if (status === 'missing') return { status: 'missing' };
    if (operationFetchTime != null && queryCacheExpirationTime != null) {
        // TTL stale check — only entered when fetchTime is non-null
    }
    return { status: 'available', fetchTime: operationFetchTime ?? null };
}

When operationFetchTime == null, the TTL-based stale check is skipped and the function falls through to 'available'. There is no path that treats "records present but freshness metadata missing" as stale.

Possible fix

We tested this patch on our bug and it fixed it, but not 100% if I'm introducing a side effect here

   if (operationFetchTime != null && queryCacheExpirationTime != null) {
       var isStale = operationFetchTime <= Date.now() - queryCacheExpirationTime;
       if (isStale && staleForExpiration) {
           return { status: 'stale' };
       }
   }
+  if (operationFetchTime == null) {
+      return { status: 'stale' };
+  }
   return { status: 'available', fetchTime: operationFetchTime ?? null };

Does this look like a possible bug, or could there be something wrong on our side?

[morrys]

Adding that code always forces network calls every time a query not present in the store is executed. Forcing this behavior for everyone doesn’t really convince me. Your case seems to come from data in the store that isn’t actually cleaned up until the TTL expires. What you could do instead is use the store cache’s mergeState to clean up all queries that are no longer referenced at startup.

[matt-dalton]

Maybe I wrote the issue misleadingly, I actually don't think the problem is due to orphaned records.

We did actually try a fix like you described, by calling store.__gc() on app load. We tracked what was happening and this was successfully removing some records, but it didn't fix the issue.

We then logged what was happening internally when the issue occurred in a prod build...In the the next Store.check():

• hasRootEntry: true - the new root entry (H2) does exist after retain
• operationLastWrittenAt: null - but notify() has never run for it
• operationFetchTime: null
• inner DataChecker.check status: 'available' - records ARE present in the source

So nothing in the store actually needs cleaning. The records are still in the source (typically because they're shared with other retained operations, or rehydrated from persistence). The new H2 root entry is freshly retained with refCount > 0 and dispose: false, so no cleanup mechanism (TTL/dispose-based or mergeState-based) will touch it.

The bug is purely in getAvailabilityStatus: a root entry with fetchTime: null is given status: 'available' and that pair shouldn't be possible - 'available' => the query is current/up-to-date, fetchTime: null => the store doesn't know when it was fetched. With store-or-network the cache skips the network, no fetch ever runs, so notify() is never called and fetchTime stays null indefinitely.

Adding that code always forces network calls every time a query not present in the store is executed.

Good chance I'm missing some context, but is that definitely true? It looks as though the new branch only fires when both conditions hold:

1. inner status is not 'missing' (records present), and
2. fetchTime == null

When records are missing the early if (status === 'missing') return … returns before this branch can fire, so first-time queries with no cached data are unaffected I think?

I think maybe the other case would be a first-time check on an operation whose records were already in the source somehow. In that case a new request would fire, but in that case wouldn't it be worth populated fetchTime?
Or I guess you check for rootEntry != null before?

[morrys]

@matt-dalton Thanks for the details. I think I’d need a small reproducible example project to better understand the scenario and evaluate the safest fix.

[matt-dalton]

Hi @morrys - I've done my best to recreate here.
There's a failing test that describes the failure, and some logs that show the values described above. We've also struggled to recreate on a local environment, so this recreation is based on the logging we've been seeing in sentry. Hopefully it's close to the actual picture.

[matt-dalton]

Hey @morrys - did the above make sense? Wondering if that helped illustrate the issue.

[matt-dalton]

Hey @morrys - wondering if the above helped?

[morrys]

Hi @matt-dalton , sorry for the delayed reply unfortunately, I have limited time to dedicate to these repositories.

Your reproduction is clear, but before changing this behaviour I would like to understand the minimal real-world flow that triggered the issue.

At first, I thought the issue was mainly about cleaning up old operation entries after a query changed. I now understand that the main problem is different: a new operation can be fully resolved from records already in the store, so store-or-network never fetches it and its fetchTime remains null.

For store cleanup, relay-store already falls back to retainTime when fetchTime is missing:

https://github.com/morrys/wora/blob/master/packages/relay-store/src/Store.ts#L91-L94

retainTime is assigned when the operation is first retained:

https://github.com/morrys/wora/blob/master/packages/relay-store/src/Store.ts#L103-L110

This should prevent unused root entries without a fetchTime from remaining in the store indefinitely.

The availability check, however, only considers fetchTime:

https://github.com/morrys/wora/blob/master/packages/relay-store/src/Store.ts#L252-L260

It may make sense to use retainTime as a fallback there as well. This would allow the new operation to reuse data already available in the store, preserving the benefit of store-or-network, while still marking it as stale once the configured expiration time has passed.

Marking every operation without a fetchTime as immediately stale would instead force a network request on its first execution, even when Relay can fully resolve it from the store.

A minimal example of the actual query lifecycle would help confirm the expected behaviour and possible side effects.

[matt-dalton]

Hey @morrys, thanks - appreciate that.

I've tried to add a more useful test to the repo under integration.test.js that runs the actual upgrade flow: two app launches over the same persisted store, with a field removed from a fragment in between, using real relay-compiler artifacts for both builds. It fails today, and applying my naive fix turns the whole suite green (including some regression tests I added), but agree perhaps there's a more elegant fix that avoids the forced network request everywhere.