The check that ran before the write

The check that ran before the write

The problem

An owner reports her dog missing. The app raises an emergency: a row on the server, and an ongoing alert on her lock screen that survives the app being killed. Two days later the dog comes home. She taps Found.

The alert disappears.

She relaunches the app that evening and it opens straight into the emergency screen, alert running, dog missing. She taps Found again. Same thing the next morning.

The obvious suspect is the resolve call — it said it worked and it didn't. But POST /sos/incidents/{id}/resolve returned 200, the incident it named really was closed, and stopSosAlert() really did run. The tear-down worked perfectly.

It worked perfectly on the one incident the app knew about. Her account had thirty-two.

Where it actually was

Two separate faults, either of which alone produces the same report.

The client could only ever know about one of them. Open incidents are cached in a map keyed by the thing that is supposed to be unique:

static String incidentKey(String petId, SosAlertKind kind) =>
    '$petId:${kind.wire}';

And the boot-time reconcile fills it from the server's list:

openIncidents
  ..clear()
  ..addEntries(
    open.map((i) => MapEntry(incidentKey(i.petId, i.kind), i.id)),
  );

addEntries overwrites on a duplicate key. Thirty-two open incidents for one dog collapse into one entry, last one wins. A map keyed by an invariant cannot represent a violation of that invariant — the bug wasn't hard to see from the client, it was structurally unrepresentable there.

So resolve closed what it could see, and then correctly concluded it was done:

final incidentId = openIncidents.remove(key);
if (incidentId != null && api != null) {
  await SosApi(api).resolve(incidentId, found: found);
}
await _stopAlertIfNoneOpen(liveActivity);

The map is empty now, so the alert stops. Correct behaviour on incomplete information, which is the worst kind of correct — it leaves nothing to log.

Next launch, boot reconciles against the server, finds thirty-one still open, and starts the alert again. That is the entire "it came back".

Where the duplicates came from is the actual race. raise_incident had two idempotency layers, both deliberate:

existing = await self._incidents().get(spec.incident_id)
if existing is not None:
    return _incident_from(existing)
already_open = await self.open_for_pet(spec.pet_id, spec.kind)
if already_open is not None:
    return already_open

The first absorbs a client retry — same id. The second absorbs the same emergency raised again under a different id: a second device, a reinstall, a session that got cleared and minted a fresh UUID.

Both check, then write. Two raises arriving close enough together both pass the second check and both create a row.

The guard that normally catches this can't fire here. Every create the repository floor builds carries:

"ConditionExpression": "attribute_not_exists(pk)",

That refuses a colliding id. But the ids differ by construction — the client mints a UUIDv7 per emergency, and the entire reason the second check exists is that a legitimate second raise carries a different one. The rows don't collide. Nothing is refused. Both land.

DynamoDB has no unique secondary index. The uniqueness we needed was on (pet_id, kind), which was not a primary key, so nothing in the database was in a position to enforce it. And open_for_pet is a list-and-filter — an eventually consistent read — which widens the window rather than narrowing it.

The fix, and the trap next to it

Make the invariant a row, and let its primary key be the invariant.

def _lock_id(pet_id: str, kind: SosKind) -> str:
    return f"{pet_id}:{kind.value}"

Then put it in the same transaction as the incident and its event:

lock_item, _ = self._locks().build_put_transact_item(
    _lock_id(spec.pet_id, spec.kind),
    {"incident_id": spec.incident_id, "pet_id": spec.pet_id, ...},
)
await default_engine().emit(
    SOS_RAISED, {...},
    lane=DeliveryLane.DURABLE,
    extra_writes=[transact_item, lock_item],
)

The lock's id is the pair, so two concurrent raises collide on it even though their incidents don't. The attribute_not_exists(pk) that was already on every create now has something to refuse. And because the lock rides the same commit, there is never an instant where an incident exists without its lock for the next reader to catch.

Nothing here was invented. The condition was already on every create. State and event were already landing in one transaction. The only new thing is a row whose id says what we actually meant.

The trap is what you do with the cancellation — the same trap as the duplicate-posts fix, using the same helper:

def _refused_by_condition(exc: TransactionCancelled) -> bool:
    return bool(exc.reasons) and all(
        r.code == "ConditionalCheckFailed" for r in exc.reasons
    )

A transaction cancels for reasons that have nothing to do with losing a race: contention that outlived the floor's retries, a throttle, an unexplained cancel with no per-item reasons. Answer one of those by handing back somebody else's incident and a genuine failure becomes a confident wrong answer about whether a pet is missing.

There's a second trap this one has that the posts fix didn't. The loser re-reads to find the winner, and the re-read can miss:

already_open = await self.open_for_pet(spec.pet_id, spec.kind)
if already_open is None:
    raise

open_for_pet is eventually consistent, so the winner's row may not be visible yet. Failing is the honest answer — every raise is idempotent on its own incident id, so the caller's retry will find it. Swallowing that and reporting "no emergency" to someone whose dog is gone, purely to avoid surfacing an error, is not a trade worth making.

The lock is deleted on resolve, best-effort, with a loud log and no ability to fail the resolve. A lock left behind would block the next real emergency for that pet, which is bad. Failing a resolve the owner is standing there waiting on is worse.

The third fault, which was not a database problem at all

The alert is posted before the network call, carrying the locally minted id. That's deliberate: an emergency has to appear on a dead connection.

If the server deduped onto an already-open incident, it answered with a different id. The client adopted it:

openIncidents[key] = saved.id;

But the notification already on the lock screen still carried the local one — and that notification's Found and Emergency over buttons deep-link their id straight back into the app to resolve it.

So the two buttons on the alert addressed a row that had never been stored. The resolve 404'd. The real incident stayed open. Next launch brought the alert back.

She taps Found on the notification and nothing happens. Same symptom, entirely different cause, and it would have survived the backend fix untouched.

openIncidents[key] = saved.id;
if (saved.id == incidentId) return;
await liveActivity.startSosAlert(
  incidentId: saved.id, ..., startedAt: saved.raisedAt ?? startedAt,
);

startSosAlert replaces rather than stacks, and the raise time is the server's own, so re-posting doesn't restart the elapsed timer on a dog that's been gone since Tuesday.

How it was verified, not just believed

The server test forces the race rather than hoping for it:

barrier = asyncio.Barrier(2)

async def gated(self, target_pet_id, kind):
    nonlocal calls
    calls += 1
    if calls <= 2:  # only the two pre-checks wait; a loser's re-read does not
        await barrier.wait()
    return await real_open_for_pet(self, target_pet_id, kind)

Both pre-checks are held until both have arrived, so both are guaranteed to observe "nothing open" before either writes. Then the assertions are about the world, not the return value:

assert first.id == second.id
assert {i.id for i in await svc.open_incidents()} == {first.id}

Two details cost real time. It needs DynamoDB Local, not moto — moto's in-memory backend isn't thread-safe, so it serialises the two calls, and the test then passes against the unfixed code. And every id has to be minted fresh per run, because DynamoDB Local is persistent: a fixed "i1" lets a previous run's row satisfy the existing fast-path before the barrier's second party ever arrives, and the test hangs on a stale lock instead of racing anything.

On the client, resolve now closes every row the server has open for that pet and kind:

final open = await sos.openIncidents();
final ids = <String>{
  ?tracked,
  if (open != null)
    for (final incident in open)
      if (incident.petId == petId && incident.kind == kind) incident.id,
};
await Future.wait(ids.map((id) => sos.resolve(id, found: found)));

A failed read degrades to closing the tracked row alone — never to closing nothing.

That half matters more than it looks. The lock stops new duplicates being created. It does nothing whatsoever for the accounts that already have them, and one of those had thirty-two.

Key takeaways

A read-then-write uniqueness check is not a uniqueness check. It's a race with a window as wide as the write path is slow. Ours was two layers deep, well-reasoned, and documented.

If the database can't enforce your invariant, give it a row that can. DynamoDB has no unique secondary index — but a row whose primary key is the invariant converts the constraint into a primary-key collision, which it enforces for free. Putting that row in the same transaction is what stops the two from ever disagreeing.

The loser must converge, not fail — and you have to know it's the loser. Separating "refused by a condition I wrote" from "cancelled for a reason I didn't anticipate" is the entire safety of the pattern. Collapse them and every genuine fault gets reported to the user as "already done".

Idempotency needs both sides to agree what the thing is called. The client minted an id so the alert could appear offline. The server answered with its own. Both were individually right, and the notification sitting between them pointed at neither.

A cache keyed by an invariant cannot represent a violation of it. Map<String, String> keyed petId:kind is an accurate model of the world we intended and a blind spot in the world we had.

Fixing the cause does not fix the accounts. Duplicates already written are a separate piece of work, and the users holding them are the ones who reported the bug.

Author's note

The dedupe in raise_incident was not sloppy. It had two layers, each aimed at a real failure shape, and a comment explaining both — the same id is a client retry, so return the stored row untouched rather than resetting the raise time and restarting the timer on someone's lock screen mid-emergency; a different id while one is open is a second device, so converge on the open one instead of racing two alerts.

That is someone who thought hard about duplicates. It reasons correctly about which shapes of duplicate exist and not at all about whether two can arrive at once. You read it, you nod, you move on — because it sounds exactly like someone who already considered the problem.

Which is the same shape as the last one of these I wrote up. The safeguard that's missing announces itself eventually. The one that's present, careful, and answering a question adjacent to yours is the one that costs you.

The other thing worth saying: the answer was already in the codebase. _refused_by_condition is lifted from community/engagement.py — same helper, same docstring, written for the duplicate-posts fix months earlier. Two modules, two duplicate-write bugs, one mechanism, copied by hand the second time. When a pattern shows up twice like that, the question stops being "where do I paste this" and starts being why it isn't in the repository floor, waiting for whichever module hits it third.

Read more