realm-server: reset retrieveIndexHTML cache when work throws

packages/realm-server/handlers/serve-index.ts

@@ -3,7 +3,6 @@ import { JSDOM } from 'jsdom';
 import merge from 'lodash/merge';
 import type { DBAdapter, Realm } from '@cardstack/runtime-common';
 import {
-  Deferred,
   hasExtension,
   logger,
   param,
@@ -83,12 +82,6 @@ export function createServeIndex(deps: ServeIndexDeps): ServeIndexHandlers {
       return promiseForIndexHTML;
     }
 
-    let deferred = new Deferred<string>();
-
-    if (!isDev) {
-      promiseForIndexHTML = deferred.promise;
-    }
-
     let rewriteRealmURL = (url?: string) => {
       if (!url) {
         return url;
@@ -101,74 +94,86 @@ export function createServeIndex(deps: ServeIndexDeps): ServeIndexHandlers {
       ).href;
     };
 
-    let indexHTML = (await getIndexHTML()).replace(
-      /(<meta name="@cardstack\/host\/config\/environment" content=")([^"].*)(">)/,
-      (_match, g1, g2, g3) => {
-        let config = JSON.parse(decodeURIComponent(g2));
-
-        if (config.publishedRealmBoxelSpaceDomain === 'localhost:4201') {
-          // if this is the default, this needs to be the realm server’s host
-          // to work in Matrix tests, since publishedRealmBoxelSpaceDomain is currently
-          // the default domain for publishing a realm
-          config.publishedRealmBoxelSpaceDomain = serverURL.host;
-        }
+    let work = (async () => {
+      let indexHTML = (await getIndexHTML()).replace(
+        /(<meta name="@cardstack\/host\/config\/environment" content=")([^"].*)(">)/,
+        (_match, g1, g2, g3) => {
+          let config = JSON.parse(decodeURIComponent(g2));
+
+          if (config.publishedRealmBoxelSpaceDomain === 'localhost:4201') {
+            // if this is the default, this needs to be the realm server’s host
+            // to work in Matrix tests, since publishedRealmBoxelSpaceDomain is currently
+            // the default domain for publishing a realm
+            config.publishedRealmBoxelSpaceDomain = serverURL.host;
+          }
+
+          if (config.publishedRealmBoxelSiteDomain === 'localhost:4201') {
+            // if this is the default, this needs to be the realm server’s host
+            // to work in Matrix tests, since publishedRealmBoxelSiteDomain is currently
+            // the default domain for publishing a realm
+            config.publishedRealmBoxelSiteDomain = serverURL.host;
+          }
+
+          config = merge({}, config, {
+            hostsOwnAssets: false,
+            assetsURL: assetsURL.href,
+            matrixURL: matrixClient.matrixURL.href.replace(/\/$/, ''),
+            matrixServerName:
+              process.env.MATRIX_SERVER_NAME || matrixClient.matrixURL.hostname,
+            realmServerURL: serverURL.href,
+            resolvedBaseRealmURL: rewriteRealmURL(config.resolvedBaseRealmURL),
+            resolvedCatalogRealmURL: rewriteRealmURL(
+              config.resolvedCatalogRealmURL,
+            ),
+            resolvedSkillsRealmURL: rewriteRealmURL(
+              config.resolvedSkillsRealmURL,
+            ),
+            resolvedOpenRouterRealmURL: rewriteRealmURL(
+              config.resolvedOpenRouterRealmURL,
+            ),
+            defaultSystemCardId: rewriteRealmURL(config.defaultSystemCardId),
+            defaultFieldSpecId: rewriteRealmURL(config.defaultFieldSpecId),
+            cardSizeLimitBytes,
+            fileSizeLimitBytes,
+            publishedRealmDomainOverrides:
+              process.env.PUBLISHED_REALM_DOMAIN_OVERRIDES ??
+              config.publishedRealmDomainOverrides,
+          });
+          return `${g1}${encodeURIComponent(JSON.stringify(config))}${g3}`;
+        },
+      );
 
-        if (config.publishedRealmBoxelSiteDomain === 'localhost:4201') {
-          // if this is the default, this needs to be the realm server’s host
-          // to work in Matrix tests, since publishedRealmBoxelSiteDomain is currently
-          // the default domain for publishing a realm
-          config.publishedRealmBoxelSiteDomain = serverURL.host;
-        }
+      indexHTML = indexHTML.replace(/(src|href)="\//g, `$1="${assetsURL.href}`);
+
+      // Strip any static favicon/apple-touch-icon links from the base HTML
+      // since these are now dynamically injected between the head markers
+      indexHTML = indexHTML
+        .replace(/<link[^>]*\brel="icon"[^>]*\/?>/gi, '')
+        .replace(/<link[^>]*\brel="apple-touch-icon"[^>]*\/?>/gi, '');
+
+      // Recompute the hash in dev mode (where index.html is not cached) so
+      // that changes to the shell are reflected in the ETag.
+      if (!indexHTMLHash || isDev) {
+        let { createHash } = await import('crypto');
+        indexHTMLHash = createHash('md5')
+          .update(indexHTML)
+          .digest('hex')
+          .slice(0, 8);
+      }
 
-        config = merge({}, config, {
-          hostsOwnAssets: false,
-          assetsURL: assetsURL.href,
-          matrixURL: matrixClient.matrixURL.href.replace(/\/$/, ''),
-          matrixServerName:
-            process.env.MATRIX_SERVER_NAME || matrixClient.matrixURL.hostname,
-          realmServerURL: serverURL.href,
-          resolvedBaseRealmURL: rewriteRealmURL(config.resolvedBaseRealmURL),
-          resolvedCatalogRealmURL: rewriteRealmURL(
-            config.resolvedCatalogRealmURL,
-          ),
-          resolvedSkillsRealmURL: rewriteRealmURL(
-            config.resolvedSkillsRealmURL,
-          ),
-          resolvedOpenRouterRealmURL: rewriteRealmURL(
-            config.resolvedOpenRouterRealmURL,
-          ),
-          defaultSystemCardId: rewriteRealmURL(config.defaultSystemCardId),
-          defaultFieldSpecId: rewriteRealmURL(config.defaultFieldSpecId),
-          cardSizeLimitBytes,
-          fileSizeLimitBytes,
-          publishedRealmDomainOverrides:
-            process.env.PUBLISHED_REALM_DOMAIN_OVERRIDES ??
-            config.publishedRealmDomainOverrides,
-        });
-        return `${g1}${encodeURIComponent(JSON.stringify(config))}${g3}`;
-      },
-    );
+      return indexHTML;
+    })();
 
-    indexHTML = indexHTML.replace(/(src|href)="\//g, `$1="${assetsURL.href}`);
-
-    // Strip any static favicon/apple-touch-icon links from the base HTML
-    // since these are now dynamically injected between the head markers
-    indexHTML = indexHTML
-      .replace(/<link[^>]*\brel="icon"[^>]*\/?>/gi, '')
-      .replace(/<link[^>]*\brel="apple-touch-icon"[^>]*\/?>/gi, '');
-
-    // Recompute the hash in dev mode (where index.html is not cached) so
-    // that changes to the shell are reflected in the ETag.
-    if (!indexHTMLHash || isDev) {
-      let { createHash } = await import('crypto');
-      indexHTMLHash = createHash('md5')
-        .update(indexHTML)
-        .digest('hex')
-        .slice(0, 8);
+    if (!isDev) {
+      promiseForIndexHTML = work;
+      // If the work rejects, clear the cache so the next request retries
+      // instead of awaiting a permanently-rejected (or pending) promise.
+      work.catch(() => {
+        promiseForIndexHTML = undefined;
+      });
     }
 
-    deferred.fulfill(indexHTML);
-    return indexHTML;
+    return work;
   }
 
   function defaultIconLinks(): string[] {

packages/realm-server/tests/index.ts

@@ -243,6 +243,7 @@ const ALL_TEST_FILES: string[] = [
   './server-endpoints/screenshot-card-endpoint-test',
   './server-endpoints/search-test',
   './server-endpoints/search-prerendered-test',
+  './serve-index-test',
   './server-config-test',
   './server-endpoints/info-test',
   './server-endpoints/stripe-session-test',

packages/realm-server/tests/serve-index-test.ts

@@ -0,0 +1,116 @@
+import { module, test } from 'qunit';
+import { basename } from 'path';
+
+import { createServeIndex } from '../handlers/serve-index';
+
+function buildDeps(getIndexHTML: () => Promise<string>) {
+  return {
+    serverURL: new URL('http://127.0.0.1:4448'),
+    // Non-localhost so the production cache branch is active.
+    assetsURL: new URL('http://example.com/notional-assets-host/'),
+    realms: [],
+    reconciler: {} as any,
+    dbAdapter: {} as any,
+    matrixClient: {
+      matrixURL: new URL('http://localhost:8008/'),
+    } as any,
+    getIndexHTML,
+    cardSizeLimitBytes: 0,
+    fileSizeLimitBytes: 0,
+  };
+}
+
+function validIndexHTML(): string {
+  return `<html><head><meta name="@cardstack/host/config/environment" content="${encodeURIComponent(
+    JSON.stringify({
+      matrixURL: 'http://localhost:8008',
+      matrixServerName: 'localhost',
+      realmServerURL: 'http://localhost:4201/',
+      publishedRealmBoxelSpaceDomain: 'localhost:4201',
+      publishedRealmBoxelSiteDomain: 'localhost:4201',
+    }),
+  )}"></head><body></body></html>`;
+}
+
+module(basename(__filename), function () {
+  test('a thrown error in retrieveIndexHTML clears the cache so the next call retries', async function (assert) {
+    let calls = 0;
+    let { retrieveIndexHTML } = createServeIndex(
+      buildDeps(async () => {
+        calls += 1;
+        if (calls === 1) {
+          throw new Error('simulated getIndexHTML failure');
+        }
+        return validIndexHTML();
+      }),
+    );
+
+    await assert.rejects(
+      retrieveIndexHTML(),
+      /simulated getIndexHTML failure/,
+      'first call propagates the underlying error',
+    );
+
+    let html = await retrieveIndexHTML();
+    assert.ok(
+      html.includes('@cardstack/host/config/environment'),
+      'second call recovers and returns the rewritten index HTML',
+    );
+    assert.strictEqual(
+      calls,
+      2,
+      'getIndexHTML was re-invoked after the failure (cache cleared)',
+    );
+  });
+
+  test('a synchronous throw inside the rewrite step also clears the cache', async function (assert) {
+    let calls = 0;
+    let { retrieveIndexHTML } = createServeIndex(
+      buildDeps(async () => {
+        calls += 1;
+        if (calls === 1) {
+          // Malformed embedded config — JSON.parse inside the meta-rewrite
+          // replacer will throw.
+          return `<html><head><meta name="@cardstack/host/config/environment" content="not-a-valid-encoded-json"></head><body></body></html>`;
+        }
+        return validIndexHTML();
+      }),
+    );
+
+    await assert.rejects(
+      retrieveIndexHTML(),
+      'first call propagates the JSON.parse failure',
+    );
+
+    let html = await retrieveIndexHTML();
+    assert.ok(
+      html.includes('@cardstack/host/config/environment'),
+      'second call recovers after the synchronous rewrite failure',
+    );
+    assert.strictEqual(calls, 2, 'getIndexHTML was re-invoked after the throw');
+  });
+
+  test('successful calls are memoized — getIndexHTML runs once across concurrent callers', async function (assert) {
+    let calls = 0;
+    let { retrieveIndexHTML } = createServeIndex(
+      buildDeps(async () => {
+        calls += 1;
+        return validIndexHTML();
+      }),
+    );
+
+    let [a, b, c] = await Promise.all([
+      retrieveIndexHTML(),
+      retrieveIndexHTML(),
+      retrieveIndexHTML(),
+    ]);
+
+    assert.strictEqual(calls, 1, 'getIndexHTML was only invoked once');
+    assert.strictEqual(a, b, 'concurrent callers receive the same string');
+    assert.strictEqual(b, c, 'concurrent callers receive the same string');
+
+    let d = await retrieveIndexHTML();
+    assert.strictEqual(calls, 1, 'subsequent calls also reuse the cache');
+    assert.strictEqual(d, a, 'cached value is returned identically');
+  });
+});