DEV Community

Daniel Pertu
Daniel Pertu

Posted on

Our on-device product cache key ends in v3, because a field that is absent reads as an answer

Munchable answers a barcode scan in a supermarket, which means it answers on a phone, on store wifi, while somebody is holding a pack and waiting. There are three read tiers in front of the lookup, and the interesting one is the middle: a small AsyncStorage cache whose storage key ends in v3.

That suffix is not housekeeping. It is there because of a specific way a cache can be worse than no cache at all.

Three tiers, each with a different job

Top tier is a module-level Map, alive for the session:

/**
 * Module-level cache of products resolved this session. The result screen reads
 * this before falling back to the persisted cache and (dev only) the samples.
 */
const resolved = new Map<string, ResolvedProduct>();
Enter fullscreen mode Exit fullscreen mode

Then the persisted one:

/**
 * Small AsyncStorage backed cache of resolved products, so a product the user
 * scanned or contributed survives an app restart (the "restart dead end" fix).
 * Products only - never conditions, never verdicts, never photos. Bounded and
 * best effort: any storage failure is swallowed, the session memory in api.ts
 * stays the source of truth.
 */
Enter fullscreen mode Exit fullscreen mode

And the server, joined through a single-flight map so two scans of the same pack cannot become two requests.

The persisted tier has exactly one job, and knowing which job it is sets its expiry:

/**
 * An hour is chosen against what this leg is actually for. Repeat scans inside
 * one session are already free, served out of the session map above without
 * touching storage; all this leg adds is the user who restarts the app partway
 * through a shop and rescans the same pack. An hour covers that comfortably and
 * caps how long a correction can go unseen at one hour of use, which is the
 * side of the trade this data deserves.
 */
const SCAN_CACHE_MAX_AGE_MS = 60 * 60 * 1000;
Enter fullscreen mode Exit fullscreen mode

A product record is not a static asset. The database behind it gets corrected, and a cache with no expiry means a correction reaches everybody except the people who already scanned the thing. One hour is short enough that nobody is stuck with a stale answer for long, and long enough to cover "I came back out of the app and scanned it again".

That bound was added later, which is my favourite kind of detail, because the data it needed was already being written:

 * The scan path needs the age, not just the row: it may answer a scan straight
 * out of this cache without asking the server at all, and a cache with no
 * expiry would mean a product Munchable has since corrected or withdrawn keeps
 * being served on a device that once scanned it. `at` has always been written;
 * until now nothing read it.
Enter fullscreen mode Exit fullscreen mode

Size is set by a different tier again:

/** Keep a little more than the recent scans list (12) so recalls always hit. */
const MAX_ENTRIES = 24;
Enter fullscreen mode Exit fullscreen mode

The recent scans list shows 12. If the cache held fewer than that, tapping the oldest item in a list the app is showing you would go to the network, which is the one case where a cache miss looks like a bug.

The version suffix, and the bug that put it there

/**
 * Bump on every change to the shape of a stored `ResolvedProduct`.
 *
 * Without it an entry written by an older build rehydrates with the new fields
 * simply absent, and absent reads as a real answer rather than as "we do not
 * know".
 */
const STORE_KEY = 'munchable-products-v3';
Enter fullscreen mode Exit fullscreen mode

The second sentence is the whole lesson. A JSON blob written by version N and read by version N+1 does not announce itself. It deserializes cleanly, type-checks against the new interface as far as TypeScript is concerned at runtime (which is to say not at all), and hands you an object whose new fields are undefined. If any of your code treats undefined as a value rather than as an absence, the old entry is now an assertion.

Here is the one that actually happened:

 * `named` is the case that bit: undefined counts as unnamed, so after an
 * app update the capture screen told people "nobody has added this barcode yet"
 * for products Munchable had names for, and then took the name they typed.
Enter fullscreen mode Exit fullscreen mode

Read that twice, because it has two halves and the second one is the expensive one.

Half one: the app told people something false. A product with a perfectly good name in the database rehydrated from the cache with named absent, absent was read as "not named", and the screen said nobody had added this barcode yet.

Half two: the screen that says that has a text field under it. So the user helpfully typed the name of the product, and the app wrote it. A stale cache entry became a prompt, and the prompt became a write to shared data. The read-only bug became a data-quality bug, and the only trace of the original cause was in a JSON blob on one device.

That is what the version suffix buys: an old entry under an old key is never read, so it cannot be mistaken for an answer. The newer bump has the same shape. v3 added a field whose absence would be read as a definite negative rather than as "this build does not know", so an entry written by a v2 build must not be rehydrated into a v3 build at all. Not migrated, not defaulted. Ignored.

The defaulting instinct is the trap. Writing named ?? false makes the type checker happy and keeps the exact bug.

What the cache is allowed to hold

/**
 * One cached product. Public product data only, so it is safe to keep across
 * accounts on a shared device: nothing in here is about a person.
 */
Enter fullscreen mode Exit fullscreen mode

Products only. Never the conditions somebody picked, never a verdict, never a photo. That is partly architecture (verdicts are computed on the device from the profile, so caching one would be caching a stale combination of two things) and partly the privacy posture written into the privacy policy: health data stays on the phone.

The things that are about a person live under different keys, in a store whose own comment is unambiguous:

/**
 * The scan history and food log. Health data by any reading, so it lives here
 * and nowhere else: never synced, never exported by the server's account
 * export, cleared with the rest of the device on account deletion.
 */
Enter fullscreen mode Exit fullscreen mode

"Never exported by the server's account export" is a strange sentence to have to write until you think about what a GDPR export is. The server cannot export a food log it has never seen. The honest export is the one that does not pretend to include it.

Writing this post found the drift

Account deletion wipes local data, and that wipe is a hand-maintained list of keys:

await AsyncStorage.multiRemove([
  'munchable-profile', // useProfile persist (health profile)
  'munchable-history', // useHistory persist (scan history + food log)
  'munchable-products-v1', // productCache (pre-v2 key, still cleared)
  'munchable-products-v2', // productCache
  ...
Enter fullscreen mode Exit fullscreen mode

v1 is cleared. v2 is cleared. The key the app actually writes is v3, and it was not in that list, so deleting your account left the current product cache on the device.

The blast radius is small, which is exactly why it survived: that cache holds public product data and nothing about a person, so what remained was a list of barcodes and the names of the products behind them. It is still wrong, because the function's own comment promises to remove "all Munchable data persisted on this device", and now it does:

  'munchable-products-v2', // productCache (pre-v3 key, still cleared)
  'munchable-products-v3', // productCache (current key: bump this list with it)
Enter fullscreen mode Exit fullscreen mode

The general shape of this mistake is worth naming: a version suffix turns one constant into two places that have to agree, and the second place is one nobody opens except when writing a privacy policy. So there is now a test, and it reads the two files as text rather than importing them:

test('account deletion clears the product cache key the app actually writes', () => {
  const key = /STORE_KEY = '([^']+)'/.exec(read('../productCache.ts'))?.[1];
  assert.ok(key, 'productCache.ts no longer declares STORE_KEY as a literal');
  assert.match(read('../wipe.ts'), new RegExp(`'${key}'`), `wipe.ts does not clear ${key}`);
});
Enter fullscreen mode Exit fullscreen mode

Reading source instead of importing it is not laziness here. Both modules import AsyncStorage, which wants a native host that node --test does not have, and the thing being checked is a textual agreement between two literals rather than any behaviour. The next version bump cannot be half-done.

Two more rules in the same file

The cache is only consulted for rows the server has not withheld:

if (
  stored &&
  stored.entry.status === undefined &&
  Date.now() - stored.at < SCAN_CACHE_MAX_AGE_MS
) {
Enter fullscreen mode Exit fullscreen mode

And when the server does withdraw something, the device drops its copy rather than waiting for it to age out:

/** Drop a persisted product the server has since withheld or removed. */
export async function removePersistedProduct(barcode: string): Promise<void> {
Enter fullscreen mode Exit fullscreen mode

Every write to this store is wrapped in a try with a comment instead of a handler:

  } catch {
    // Cache only - a failed write costs a re-lookup, nothing else.
  }
Enter fullscreen mode Exit fullscreen mode

A cache that can fail a user action is not a cache. The session map is the source of truth within a session, and AsyncStorage is an optimisation that is allowed to be broken.

The tier above all of this is the one that does nothing

The scanner starts resolving a barcode before the user has confirmed it, and the rule for that path is the strictest in the file:

 * Fire and forget by design: the ONLY effect it is allowed to have
 * is that `scanProduct` for the same code answers faster, because it will join
 * this very promise instead of starting its own.
 *
 * What it deliberately does not do: it does not count a scan, it does not read
 * or spend quota, and it does not navigate.
Enter fullscreen mode Exit fullscreen mode

And it is capped at three in flight, for a reason that is not about the server:

 * The cap is not really about server load, which is one cheap keyed read per
 * request; it is about the device. React Native's fetch shares a small
 * connection pool, so a dozen requests nobody asked for are a dozen requests
 * ahead of the one for the pack the user is actually holding, and the speed-up
 * would turn into a slow-down.
Enter fullscreen mode Exit fullscreen mode

A camera pointed at a shelf acquires codes it was not asked about. Speculating on all of them is how a latency optimisation becomes a latency problem, and three is the number that covers the real case: one code re-armed once or twice while somebody steadies their hand.

The public end of all this is readable without installing anything: the eight conditions, the ingredient answers, and what we say we store, which is the document the key list above exists to keep true.

Top comments (1)

Collapse
 
nark3d profile image
Adam Lewis •

Agreement is only guarded on one side. The version suffix covers the shape of the stored record. It says nothing about the key list in the wipe, which is the same problem one level up: a field added to the record and a key added to storage both break agreement between two files, and only one of them got a guard. Doing the same text-read for the product key at write time would catch a key added later without a matching wipe entry, the way the deletion test catches it after the fact.