enhance: build some safety around assetMap reads

owid · Feb 4, 2025 · 8ce7d67 · 8ce7d67
1 parent ab3cc51
commit 8ce7d67
Show file tree

Hide file tree

Showing 6 changed files with 50 additions and 12 deletions.
diff --git a/baker/buildLocalArchivalBake.ts b/baker/buildLocalArchivalBake.ts
@@ -64,7 +64,6 @@ const copyPublicDir = async () => {
     const ignoredFilesPattern = new RegExp(
         IGNORED_FILES_PATTERNS.map((p) => p.source).join("|")
     )
-    console.log(ignoredFilesPattern)
     await fs.copy(publicDir, targetDir, {
         overwrite: true,
         filter: (src) => {

diff --git a/packages/@ourworldindata/grapher/src/core/loadVariable.ts b/packages/@ourworldindata/grapher/src/core/loadVariable.ts
@@ -2,7 +2,7 @@ import {
     AssetMap,
     OwidVariableDataMetadataDimensions,
 } from "@ourworldindata/types"
-import { fetchWithRetry } from "@ourworldindata/utils"
+import { fetchWithRetry, readFromAssetMap } from "@ourworldindata/utils"
 
 export const getVariableDataRoute = (
     dataApiUrl: string,
@@ -11,9 +11,11 @@ export const getVariableDataRoute = (
 ): string => {
     if (dataApiUrl.includes("v1/indicators/")) {
         const filename = `${variableId}.data.json`
-        if (assetMap?.[filename]) return assetMap[filename]
-        // fetching from Data API, e.g. https://api.ourworldindata.org/v1/indicators/123.data.json
-        return `${dataApiUrl}${filename}`
+        return readFromAssetMap(assetMap, {
+            path: filename,
+            // fetching from Data API, e.g. https://api.ourworldindata.org/v1/indicators/123.data.json
+            fallback: `${dataApiUrl}${filename}`,
+        })
     } else {
         throw new Error(`dataApiUrl format not supported: ${dataApiUrl}`)
     }
@@ -26,9 +28,11 @@ export const getVariableMetadataRoute = (
 ): string => {
     if (dataApiUrl.includes("v1/indicators/")) {
         const filename = `${variableId}.metadata.json`
-        if (assetMap?.[filename]) return assetMap[filename]
-        // fetching from Data API, e.g. https://api.ourworldindata.org/v1/indicators/123.metadata.json
-        return `${dataApiUrl}${filename}`
+        return readFromAssetMap(assetMap, {
+            path: filename,
+            // fetching from Data API, e.g. https://api.ourworldindata.org/v1/indicators/123.metadata.json
+            fallback: `${dataApiUrl}${filename}`,
+        })
     } else {
         throw new Error(`dataApiUrl format not supported: ${dataApiUrl}`)
     }

diff --git a/packages/@ourworldindata/utils/src/Util.ts b/packages/@ourworldindata/utils/src/Util.ts
@@ -178,6 +178,7 @@ import {
     DimensionProperty,
     GRAPHER_CHART_TYPES,
     DbPlainTag,
+    AssetMap,
 } from "@ourworldindata/types"
 import { PointVector } from "./PointVector.js"
 import * as React from "react"
@@ -2076,3 +2077,30 @@ export function isArrayDifferentFromReference<T>(
     if (array.length !== referenceArray.length) return true
     return difference(array, referenceArray).length > 0
 }
+
+// When reading from an asset map, we want a very particular behavior:
+// If the asset map is entirely undefined, then we want to just fail silently and return the fallback.
+// If the asset map is defined but the asset is not found, however, then we want to throw an error.
+// This is to avoid invisible errors that'll lead to runtime errors or 404s.
+
+export function readFromAssetMap(
+    assetMap: AssetMap | undefined,
+    { path, fallback }: { path: string; fallback: string }
+): string
+
+export function readFromAssetMap(
+    assetMap: AssetMap | undefined,
+    { path, fallback }: { path: string; fallback?: string }
+): string | undefined
+
+export function readFromAssetMap(
+    assetMap: AssetMap | undefined,
+    { path, fallback }: { path: string; fallback?: string }
+): string | undefined {
+    if (!assetMap) return fallback
+
+    const assetValue = assetMap[path]
+    if (assetValue === undefined)
+        throw new Error(`Entry for asset not found in asset map: ${path}`)
+    return assetValue
+}
diff --git a/packages/@ourworldindata/utils/src/index.ts b/packages/@ourworldindata/utils/src/index.ts
@@ -128,6 +128,7 @@ export {
     lazy,
     getParentVariableIdFromChartConfig,
     isArrayDifferentFromReference,
+    readFromAssetMap,
 } from "./Util.js"
 
 export {

diff --git a/site/detailsOnDemand.tsx b/site/detailsOnDemand.tsx
@@ -7,6 +7,7 @@ import {
     AssetMap,
     DetailDictionary,
     fetchWithRetry,
+    readFromAssetMap,
 } from "@ourworldindata/utils"
 import { SiteAnalytics } from "./SiteAnalytics.js"
 
@@ -27,8 +28,10 @@ export async function runDetailsOnDemand({
 }: {
     runtimeAssetMap?: AssetMap
 } = {}) {
-    const dodFetchUrl =
-        runtimeAssetMap?.["dods.json"] ?? `${BAKED_BASE_URL}/dods.json`
+    const dodFetchUrl = readFromAssetMap(runtimeAssetMap, {
+        path: "dods.json",
+        fallback: `${BAKED_BASE_URL}/dods.json`,
+    })
 
     window.details = await fetchWithRetry(dodFetchUrl, {
         method: "GET",

diff --git a/site/viteUtils.tsx b/site/viteUtils.tsx
@@ -8,7 +8,7 @@ import {
 } from "../settings/serverSettings.js"
 import { POLYFILL_URL } from "./SiteConstants.js"
 import type { Manifest, ManifestChunk } from "vite"
-import { sortBy } from "@ourworldindata/utils"
+import { readFromAssetMap, sortBy } from "@ourworldindata/utils"
 import urljoin from "url-join"
 import { AssetMap } from "@ourworldindata/types"
 
@@ -111,7 +111,10 @@ export const createTagsForManifestEntry = (
             throw new Error(`Could not find manifest entry for ${entry}`)
 
         const assetKey = manifestEntry?.file ?? entry
-        const assetUrl = assetMap?.[assetKey] ?? urljoin(assetBaseUrl, assetKey)
+        const assetUrl = readFromAssetMap(assetMap, {
+            path: assetKey,
+            fallback: urljoin(assetBaseUrl, assetKey),
+        })
 
         if (entry.endsWith(".css")) {
             assets = [