Withhold a render failure the environment caused, at the row that publishes it

packages/realm-server/prerender/prerender-app.ts

@@ -187,6 +187,44 @@ export function shouldRerenderForStaleShell({
   );
 }
 
+// The row types whose *own* failure is a missing export, rather than types
+// that merely carry one among the console errors `RenderRunner` merged onto
+// them.
+//
+// Two narrowings, and both matter because of what the answer is used for. The
+// broader `hasMissingExportError` drives the re-render, where being wrong costs
+// one render; this gates withholding a row, where being wrong hides a genuine
+// break. And it answers per row type rather than per response, because a single
+// visit produces the instance and file rows independently — a card render that
+// hit the stale bundle says nothing about a file extraction that failed beside
+// it for its own reasons.
+function unattributableRowTypes(
+  response: RenderVisitResponse,
+): ('instance' | 'file')[] {
+  let types = new Set<'instance' | 'file'>();
+  let consider = (
+    candidate: { error?: { message?: unknown } } | undefined,
+    type: 'instance' | 'file',
+  ) => {
+    let message = candidate?.error?.message;
+    if (typeof message === 'string' && isMissingExportMessage(message)) {
+      types.add(type);
+    }
+  };
+  consider(response.card?.error, 'instance');
+  consider(response.fileExtract?.error, 'file');
+  consider(response.fileRender?.error, 'file');
+  // A page that never became usable produced neither render, so the failure
+  // belongs to both rows rather than to one of them.
+  let pageUnusable = response.pageUnusableError;
+  let pageMessage = pageUnusable?.error?.message;
+  if (typeof pageMessage === 'string' && isMissingExportMessage(pageMessage)) {
+    types.add('instance');
+    types.add('file');
+  }
+  return [...types];
+}
+
 function hasMissingExportError(response: RenderVisitResponse): boolean {
   // Every sub-response that can carry a render failure, because every one of
   // them is persisted the same way: `prerender-html-visit` writes
@@ -276,6 +314,29 @@ export function stampHostShellTokens(
   };
 }
 
+// Record that this failure is the environment's rather than the card's, so the
+// write site can decline to publish it as the card's content.
+//
+// Stated by this server because it is the only place holding both the reported
+// and warmed tokens at the moment of the render. The write site checks for the
+// field's presence and nothing else — it does not re-derive the conclusion, so
+// there is one implementation of the rule rather than two that can drift.
+export function stampStaleShellFailure(
+  response: RenderVisitResponse,
+  rowTypes: ('instance' | 'file')[],
+): void {
+  if (rowTypes.length === 0) {
+    return;
+  }
+  response.meta = {
+    ...(response.meta ?? {}),
+    diagnostics: {
+      ...(response.meta?.diagnostics ?? {}),
+      staleShellFailure: rowTypes,
+    },
+  };
+}
+
 // A one-shot notification that shutdown has begun, which the holder releases
 // when it no longer needs it.
 type DrainSubscription = {
@@ -1321,6 +1382,38 @@ export function buildPrerenderApp(options: {
           discardedMs,
           shellAtStart,
         );
+        // The re-render failed the same way and the pool still cannot be shown
+        // to have been on the shell being served, so this failure describes the
+        // environment and not the card. Say so on the response and return it
+        // unchanged: the write site declines to publish it as the card's
+        // content, which is the only place that decision can be made — a
+        // prerender server can delay a write but never prevent one.
+        //
+        // Narrower than the re-render's own test on purpose. That one accepts a
+        // missing export anywhere in the error, including the console errors
+        // merged onto an unrelated timeout, because being wrong there costs one
+        // render. Withholding a row wants the missing export to be the failure
+        // itself.
+        let unattributable = unattributableRowTypes(response);
+        if (
+          unattributable.length > 0 &&
+          shouldRerenderForStaleShell({
+            response,
+            warmedAtStart,
+            warmedAtCompletion,
+            reportedAtCompletion: shellAtCompletion,
+          })
+        ) {
+          log.warn(
+            'visit of %s failed to resolve a module on a pool warmed against %s -> %s while the current host shell is %s, after a re-render; marking the %s row(s) unattributable to the card',
+            url,
+            warmedAtStart ?? 'none',
+            warmedAtCompletion ?? 'none',
+            shellAtCompletion,
+            unattributable.join(', '),
+          );
+          stampStaleShellFailure(response, unattributable);
+        }
       }
       let totalMs = Date.now() - start;
       let poolFlags = Object.entries({

packages/realm-server/tests/prerender-host-shell-recycle-test.ts

@@ -9,6 +9,7 @@ import {
   raceAgainstDrain,
   shouldRerenderForStaleShell,
   stampHostShellTokens,
+  stampStaleShellFailure,
 } from '../prerender/prerender-app.ts';
 
 // Unit tests for the host-shell recycle decision a prerender server makes on
@@ -390,6 +391,48 @@ module(basename(import.meta.filename), function () {
     });
   });
 
+  module('stampStaleShellFailure', function () {
+    // The write site tests only for this field's presence, so its encoding is
+    // the whole contract: absence has to mean "no verdict" rather than
+    // "attributable", or a response from anything that does not stamp it would
+    // read as a licence to withhold a row.
+    test('names the rows it covers and leaves the rest of diagnostics alone', function (assert) {
+      let response = {
+        meta: { requestId: 'abc', diagnostics: { renderMs: 12 } },
+      } as unknown as RenderVisitResponse;
+      stampStaleShellFailure(response, ['instance']);
+      assert.deepEqual(response.meta, {
+        requestId: 'abc',
+        diagnostics: { renderMs: 12, staleShellFailure: ['instance'] },
+      } as unknown as typeof response.meta);
+    });
+
+    // A visit's rows fail independently, so a verdict that named the response
+    // rather than the rows would let one row's stale failure withhold
+    // another's genuine one. The write site tests membership, so an empty
+    // verdict must not be written at all.
+    test('an empty verdict stamps nothing', function (assert) {
+      let response = {
+        meta: { diagnostics: { renderMs: 12 } },
+      } as unknown as RenderVisitResponse;
+      stampStaleShellFailure(response, []);
+      assert.false(
+        'staleShellFailure' in ((response.meta as any).diagnostics ?? {}),
+        'absent rather than an empty array a reader might mis-test',
+      );
+    });
+
+    test('an unmarked response carries no verdict at all', function (assert) {
+      let response = {
+        meta: { diagnostics: { renderMs: 12 } },
+      } as unknown as RenderVisitResponse;
+      assert.false(
+        'staleShellFailure' in ((response.meta as any).diagnostics ?? {}),
+        'absent rather than false — a reader must require presence',
+      );
+    });
+  });
+
   module('raceAgainstDrain', function () {
     // Stands in for the server's drain subscription, counting how many are
     // outstanding. The count is the whole point: a subscription that survives

packages/realm-server/tests/prerender-html-split-test.ts

@@ -656,6 +656,102 @@ module(basename(import.meta.filename), function () {
       };
     }
 
+    // Withholding a stale-shell failure has to happen on THIS channel as well
+    // as on `boxel_index`, because `effectiveHasError()` is
+    // `COALESCE(i.has_error, FALSE) OR (ph.error_doc IS NOT NULL AND
+    // ph.generation >= i.generation)` — a current error doc here makes the row
+    // read as errored whatever the index channel says, and `effectiveErrorDoc()`
+    // then serves this column. Postgres uses the split channel by default, so a
+    // suppression that covered only the index channel would be inert in
+    // production.
+    module('withholding a stale-shell failure', function () {
+      async function writeError(
+        generation: number,
+        url: string,
+        opts: { diagnostics?: Diagnostics; message?: string } = {},
+      ) {
+        let batch = await makeBatch(generation);
+        await batch.seedPrerenderedHtmlInvalidations([
+          { url, operation: 'update' },
+        ]);
+        await batch.updatePrerenderedHtmlEntry(new URL(url), {
+          type: 'instance-error',
+          error: {
+            message: opts.message ?? 'has no exported member',
+            status: 500,
+            additionalErrors: null,
+          },
+          ...(opts.diagnostics ? { diagnostics: opts.diagnostics } : {}),
+        } as any);
+        await batch.done();
+      }
+
+      test('a covered verdict keeps the published render and writes no error', async function (assert) {
+        let url = `${testRealm}withheld.json`;
+        await writeInstance(1, url, '<div>good</div>');
+
+        await writeError(2, url, {
+          diagnostics: { staleShellFailure: ['instance'] } as Diagnostics,
+        });
+
+        let row = await productionRow(url);
+        assert.strictEqual(
+          row.error_doc,
+          null,
+          'no error doc, so `effectiveHasError` does not see a current render error',
+        );
+        assert.strictEqual(
+          row.isolated_html,
+          '<div>good</div>',
+          'and the last good render is still what readers get',
+        );
+      });
+
+      test('a verdict naming another row does not withhold this one', async function (assert) {
+        let url = `${testRealm}other-row.json`;
+        await writeInstance(1, url, '<div>good</div>');
+
+        // The card render hit the stale bundle; this instance row failed for
+        // its own reasons and must stay visible.
+        await writeError(2, url, {
+          diagnostics: { staleShellFailure: ['file'] } as Diagnostics,
+        });
+
+        let row = await productionRow(url);
+        assert.ok(
+          row.error_doc,
+          'the error is published, because the verdict did not cover this row',
+        );
+      });
+
+      test('an unmarked failure is published as before', async function (assert) {
+        let url = `${testRealm}unmarked.json`;
+        await writeInstance(1, url, '<div>good</div>');
+        await writeError(2, url);
+
+        let row = await productionRow(url);
+        assert.ok(
+          row.error_doc,
+          'absence of a verdict means no verdict, never "withhold"',
+        );
+      });
+
+      test('a first render has nothing to keep, so its failure surfaces', async function (assert) {
+        let url = `${testRealm}brand-new.json`;
+        // No prior good render at all — withholding here would leave the row
+        // blank rather than protecting anything.
+        await writeError(1, url, {
+          diagnostics: { staleShellFailure: ['instance'] } as Diagnostics,
+        });
+
+        let row = await productionRow(url);
+        assert.ok(
+          row.error_doc,
+          'the failure has to surface somewhere when there is no good content',
+        );
+      });
+    });
+
     test('writes rendered rows and swaps them under the carried generation', async function (assert) {
       let url = `${testRealm}1.json`;
       await writeInstance(5, url, '<h1>v5</h1>');