From da84d4f51a82e865de27b16f1d46a2ad0fc03718 Mon Sep 17 00:00:00 2001
From: Hassan Abdel-Rahman <hassan.abdelrahman@gmail.com>
Date: Thu, 9 Jul 2026 10:23:40 -0400
Subject: [PATCH] Stop flaky atomic instance batch depending on unindexed
 modules
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

atomic-endpoints-test "can write multiple instances that depend on each
other" writes place.gts + country.gts in one /_atomic batch, then writes
Country/Place instances that adopt from them in a second /_atomic batch.

/_atomic returns once writes are durable, not once they are indexed, and
_batchWriteUnlocked with waitForIndex:false deliberately skips waiting for
a prior write's pending index. So the second batch's fileSerialization can
look up the Place definition while the first batch's modules are still
indexing. That on-demand definition lookup then races the in-flight module
index — the generation bump discards the prerender result and the lookup
returns undefined — so the batch fails with FilterRefersToNonexistentType
and a 500. Under CI load the module index lags into that window; locally it
usually wins, so the test only flakes in CI.

The earlier module-before-instance ordering fix only serializes modules
and instances within a single batch; this test splits them across two
batches, so that fix does not apply.

Fix: the module batch is setup, so post it with ?waitForIndex=true. The
modules are indexed before the instance batch runs, removing the race. This
is the documented way for a caller writing modules then dependent instances
across separate batches to stay correct.

Diagnostics kept alongside the fix so a recurrence is debuggable without a
rerun:
- the atomic 500 handler now logs e.cause; FilterRefersToNonexistentType
  carries which module/definition was missing (or that a concurrent
  invalidation discarded the lookup) in its cause, not its message
- the instance-batch assertion prints the response body on non-201

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 .../tests/atomic-endpoints-test.ts            | 21 +++++++++++++++++--
 packages/runtime-common/realm.ts              | 16 ++++++++++++--
 2 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/packages/realm-server/tests/atomic-endpoints-test.ts b/packages/realm-server/tests/atomic-endpoints-test.ts
index 9a65d614bb2..1548ece6812 100644
--- a/packages/realm-server/tests/atomic-endpoints-test.ts
+++ b/packages/realm-server/tests/atomic-endpoints-test.ts
@@ -554,8 +554,19 @@ module(basename(import.meta.filename), function () {
               },
             ],
           };
+          // This module batch is setup for the instance batch below, which
+          // adopts from Place/Country. /_atomic returns as soon as writes are
+          // durable — not indexed — and a subsequent /_atomic write does not
+          // wait for a prior write's indexing to settle. Without waitForIndex
+          // the modules can still be indexing when the instance batch's
+          // fileSerialization looks up the Place definition; that lookup then
+          // races the in-flight module index (generation bump discards the
+          // on-demand prerender result) and the batch fails with
+          // FilterRefersToNonexistentTypeError. waitForIndex makes the modules
+          // indexed before we move on, which is what a caller writing modules
+          // then dependent instances across separate batches must do.
           let response = await request
-            .post('/_atomic')
+            .post('/_atomic?waitForIndex=true')
             .set('Accept', SupportedMimeType.JSONAPI)
             .set(
               'Authorization',
@@ -637,7 +648,13 @@ module(basename(import.meta.filename), function () {
               `Bearer ${createJWT(testRealm, 'user', ['read', 'write'])}`,
             )
             .send(JSON.stringify(instanceDoc));
-          assert.strictEqual(instanceResponse.status, 201);
+          assert.strictEqual(
+            instanceResponse.status,
+            201,
+            `expected 201, got ${instanceResponse.status}: ${JSON.stringify(
+              instanceResponse.body,
+            )}`,
+          );
           assert.strictEqual(instanceResponse.body['atomic:results'].length, 2);
         });
         test('can write new instance with new module', async function (assert) {
diff --git a/packages/runtime-common/realm.ts b/packages/runtime-common/realm.ts
index 67d3ca286af..96fa6facfe2 100644
--- a/packages/runtime-common/realm.ts
+++ b/packages/runtime-common/realm.ts
@@ -2462,9 +2462,21 @@ export class Realm {
           // Log the underlying exception before returning 500 —
           // otherwise callers only see "Write Error" and the original
           // stack trace is lost, making atomic-batch failures
-          // effectively undebuggable.
+          // effectively undebuggable. Include e.cause explicitly: errors
+          // like FilterRefersToNonexistentTypeError carry the actionable
+          // detail (which module/definition was missing, or that a
+          // concurrent invalidation discarded the lookup) in their cause,
+          // not their message, so without this the real reason is swallowed.
+          let cause =
+            e?.cause instanceof Error
+              ? `${e.cause.message}\n${e.cause.stack ?? '(no stack)'}`
+              : e?.cause != null
+                ? String(e.cause)
+                : undefined;
           this.#log.error(
-            `Atomic write failed: ${e.message}\n${e.stack ?? '(no stack)'}`,
+            `Atomic write failed: ${e.message}${
+              cause ? `\ncause: ${cause}` : ''
+            }\n${e.stack ?? '(no stack)'}`,
           );
           return createResponse({
             body: JSON.stringify({