Fix: Multiple manifests are fetched at the same time

Root cause: Due to the timeout, every request that was launched while fetching the previous manifests could led to another fetch and another cache.warmup
Solution: Be more synchronous

spec/server_test.js

@@ -26,7 +26,8 @@ describe('Manifest service', () => {
   beforeEach(async () => {
     mockserver = await createMockServer({ port })
     mockserver.respondWith({
-      '/api/manifest.json': generateSimpleViteManifest({ 'example.js': 'test' })
+      '/api/manifest.json': generateSimpleViteManifest({ 'example.js': 'test' }),
+      '/example.js': ''
     })
   })
 
@@ -54,7 +55,8 @@ describe('Manifest service', () => {
     mockserver.close()
     mockserver = await createMockServer({ port })
     mockserver.respondWith({
-      '/api/manifest.json': generateSimpleViteManifest({ 'example.js': 'other' })
+      '/api/manifest.json': generateSimpleViteManifest({ 'example.js': 'other' }),
+      '/example.js': ''
     })
 
     await new Promise(resolve => setTimeout(resolve, 150))
@@ -75,7 +77,8 @@ describe('Manifest service', () => {
     mockserver.close()
     mockserver = await createMockServer({ port })
     mockserver.respondWith({
-      '/api/manifest.json': generateSimpleViteManifest({ 'example.js': 'other' })
+      '/api/manifest.json': generateSimpleViteManifest({ 'example.js': 'other' }),
+      '/example.js': ''
     })
 
     // wait some time
@@ -97,8 +100,12 @@ describe('Manifest service', () => {
 
     mockserver.close()
     mockserver = await createMockServer({ port })
-    mockserver.respondWith({ '/api/manifest.json': generateSimpleViteManifest({ 'example1.js': 'other' }) })
-    mockserver.respondWith({ '/api/no2/manifest.json': generateSimpleViteManifest({ 'example2.js': 'thing' }) })
+    mockserver.respondWith({
+      '/api/manifest.json': generateSimpleViteManifest({ 'example1.js': 'other' }),
+      '/api/no2/manifest.json': generateSimpleViteManifest({ 'example2.js': 'thing' }),
+      '/example1.js': '',
+      '/example2.js': ''
+    })
 
     process.env.CACHE_TTL = 1
     const app = createApp()

src/createApp.js

@@ -18,7 +18,7 @@ import promBundle from 'express-prom-bundle'
 import swaggerUi from 'swagger-ui-express'
 import yaml from 'js-yaml'
 import fs from 'fs'
-import { getCSSDependenciesFor, getDependencies, getOxManifests, getVersion } from './manifests.js'
+import { getCSSDependenciesFor, getDependencies, getOxManifests, getVersion, loadViteManifests, viteManifestToDeps } from './manifests.js'
 import { fileCache } from './files.js'
 
 const ignorePaths = ['/ready', '/healthy']
@@ -37,7 +37,9 @@ export function createApp () {
   const healthCheck = new health.HealthChecker()
   const readinessCheck = new health.ReadinessCheck('getDependencies', async function () {
     try {
-      await getDependencies()
+      const viteManifests = await loadViteManifests()
+      const deps = viteManifestToDeps(viteManifests)
+      await fileCache.warmUp(viteManifests, deps)
     } catch (e) {
       logger.error(`Failed to get dependencies: ${e.message}`)
       throw e

src/manifests.js

@@ -6,43 +6,60 @@ import { config } from './config.js'
 import { hash } from './util.js'
 
 export const loadViteManifests = (() => {
-  let cache
+  let cachePromise
   let lastCacheTime
+  let lastHash
 
-  return async function loadViteManifests ({ useCache = true } = {}) {
+  async function reload () {
+    await config.load()
+    // vite manifests contains a set of objects with the vite-manifests
+    // from the corresponding registered services
+    const viteManifests = await Promise.all(config.urls.map(async url => {
+      const { origin } = new URL(url)
+      // fetch the manifests
+      const result = await fetch(url)
+      if (!result.ok) throw new Error(`Failed to load manifest for url ${result.url} (Status: ${result.status}: ${result.statusText})`)
+      try {
+        const manifest = await result.json()
+        for (const file in manifest) {
+          manifest[file].meta = manifest[file].meta || {}
+          manifest[file].meta.baseUrl = origin
+        }
+        return manifest
+      } catch (err) {
+        throw new Error(`Failed to load manifest for url ${result.url}: ${err}`)
+      }
+    }))
+
+    // combine all manifests by keys. With duplicates, last wins
+    return viteManifests.reduce((memo, manifest) => Object.assign(memo, manifest), {})
+  }
+
+  return function loadViteManifests ({ useCache = true } = {}) {
     const CACHE_TTL = parseInt(process.env.CACHE_TTL)
 
-    if (!cache || useCache === false || +new Date() > lastCacheTime + CACHE_TTL) {
-      await config.load()
-      // vite manifests contains a set of objects with the vite-manifests
-      // from the corresponding registered services
-      const viteManifests = await Promise.all(config.urls.map(async url => {
-        const { origin } = new URL(url)
-        // fetch the manifests
-        const result = await fetch(url)
-        if (!result.ok) throw new Error(`Failed to load manifest for url ${result.url} (Status: ${result.status}: ${result.statusText})`)
-        try {
-          const manifest = await result.json()
-          for (const file in manifest) {
-            manifest[file].meta = manifest[file].meta || {}
-            manifest[file].meta.baseUrl = origin
+    if (!cachePromise || useCache === false || +new Date() > lastCacheTime + CACHE_TTL) {
+      cachePromise = reload()
+      cachePromise.then(manifests => {
+        // update cache promise
+        const newHash = hash(manifests)
+        if (newHash !== lastHash) {
+          if (lastHash) {
+            const deps = viteManifestToDeps(manifests)
+            // asynchronously rewarm the cache
+            fileCache.warmUp(manifests, deps)
           }
-          return manifest
-        } catch (err) {
-          throw new Error(`Failed to load manifest for url ${result.url}: ${err}`)
+          lastHash = newHash
+          Object.defineProperty(manifests, '__hash__', {
+            enumerable: false,
+            writable: true
+          })
+          manifests.__hash__ = newHash
         }
-      }))
-
-      // combine all manifests by keys. With duplicates, last wins
-      const viteManifest = viteManifests.reduce((memo, manifest) => Object.assign(memo, manifest), {})
-      // only update that object if it really changed to prevent any further parsing from being triggered
-      if (!cache || JSON.stringify(viteManifest) !== JSON.stringify(cache)) {
-        cache = viteManifest
-      }
-
+      })
       lastCacheTime = +new Date()
     }
-    return cache
+    return cachePromise
   }
 })()
 
@@ -99,7 +116,6 @@ export const getDependencies = (() => {
     const viteManifest = await loadViteManifests()
     if (viteManifest !== prevViteManifest) {
       depCache = viteManifestToDeps(viteManifest)
-      fileCache.warmUp(viteManifest, depCache)
       prevViteManifest = viteManifest
     }
     return depCache