From 797db839d7f77ccb1f3f92614e6d7b4f7be0759f Mon Sep 17 00:00:00 2001 From: Daniel Bachler Date: Wed, 21 Aug 2024 14:46:18 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=A8=20incoprorate=20PR=20feedback?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- functions/_common/env.ts | 12 ++++++++++++ functions/_common/grapherRenderer.ts | 26 +++++++++++++++++++------- functions/grapher/[slug].ts | 23 ++++++++++++++--------- functions/grapher/thumbnail/[slug].ts | 14 +------------- 4 files changed, 46 insertions(+), 29 deletions(-) create mode 100644 functions/_common/env.ts diff --git a/functions/_common/env.ts b/functions/_common/env.ts new file mode 100644 index 00000000000..31012379905 --- /dev/null +++ b/functions/_common/env.ts @@ -0,0 +1,12 @@ +export interface Env { + ASSETS: { + fetch: typeof fetch + } + url: URL + GRAPHER_CONFIG_R2_BUCKET_URL: string + GRAPHER_CONFIG_R2_BUCKET_FALLBACK_URL: string + GRAPHER_CONFIG_R2_BUCKET_PATH: string + GRAPHER_CONFIG_R2_BUCKET_FALLBACK_PATH: string + CF_PAGES_BRANCH: string + ENV: string +} diff --git a/functions/_common/grapherRenderer.ts b/functions/_common/grapherRenderer.ts index 1b488d3d020..e98d1d26b58 100644 --- a/functions/_common/grapherRenderer.ts +++ b/functions/_common/grapherRenderer.ts @@ -16,7 +16,7 @@ import LatoRegular from "../_common/fonts/LatoLatin-Regular.ttf.bin" import LatoMedium from "../_common/fonts/LatoLatin-Medium.ttf.bin" import LatoBold from "../_common/fonts/LatoLatin-Bold.ttf.bin" import PlayfairSemiBold from "../_common/fonts/PlayfairDisplayLatin-SemiBold.ttf.bin" -import { Env } from "../grapher/thumbnail/[slug].js" +import { Env } from "./env.js" declare global { // eslint-disable-next-line no-var @@ -164,11 +164,11 @@ interface FetchGrapherConfigResult { etag: string | undefined } -export async function fetchGrapherConfig( +export async function fetchUnparsedGrapherConfig( slug: string, env: Env, - etag: string -): Promise { + etag?: string +) { // The top level directory is either the bucket path (should be set in dev environments and production) // or the branch name on preview staging environments console.log("branch", env.CF_PAGES_BRANCH) @@ -205,16 +205,28 @@ export async function fetchGrapherConfig( } // Fetch grapher config - const fetchResponse = await fetchFromR2(requestUrl, undefined, fallbackUrl) + return fetchFromR2(requestUrl, etag, fallbackUrl) +} + +export async function fetchGrapherConfig( + slug: string, + env: Env, + etag?: string +): Promise { + const fetchResponse = await fetchUnparsedGrapherConfig(slug, env, etag) if (fetchResponse.status !== 200) { - console.log("Failed to fetch grapher config", fetchResponse.status) + console.log( + "Status code is not 200, returning empty response with status code", + fetchResponse.status + ) return { grapherConfig: null, status: fetchResponse.status, etag: fetchResponse.headers.get("etag"), } } + const grapherConfig: GrapherInterface = await fetchResponse.json() console.log("grapher title", grapherConfig.title) return { @@ -232,7 +244,7 @@ async function fetchAndRenderGrapherToSvg( ) { const grapherLogger = new TimeLogger("grapher") - const grapherConfigResponse = await fetchGrapherConfig(slug, env, etag) + const grapherConfigResponse = await fetchGrapherConfig(slug, env) if (grapherConfigResponse.status !== 200) { return null diff --git a/functions/grapher/[slug].ts b/functions/grapher/[slug].ts index 15644fbfc61..b2b15a5ea1b 100644 --- a/functions/grapher/[slug].ts +++ b/functions/grapher/[slug].ts @@ -1,8 +1,8 @@ -import { Env } from "./thumbnail/[slug].js" +import { Env } from "../_common/env.js" import { - fetchGrapherConfig, getOptionalRedirectForSlug, createRedirectResponse, + fetchUnparsedGrapherConfig, } from "../_common/grapherRenderer.js" import { IRequestStrict, Router, error, cors } from "itty-router" @@ -145,7 +145,7 @@ async function handleConfigRequest( etag: string | undefined ) { const shouldCache = searchParams.get("nocache") === null - console.log("Preapring json response for ", slug) + console.log("Preparing json response for ", slug) // All our grapher slugs are lowercase by convention. // To allow incoming links that may contain uppercase characters to work, we redirect to the lowercase version. const lowerCaseSlug = slug.toLowerCase() @@ -157,14 +157,15 @@ async function handleConfigRequest( ) return createRedirectResponse( - redirectSlug ? `${redirectSlug}.config.json` : lowerCaseSlug, + `${redirectSlug ?? lowerCaseSlug}.config.json`, env.url ) } - const grapherPageResp = await fetchGrapherConfig(slug, env, etag) + const grapherPageResp = await fetchUnparsedGrapherConfig(slug, env, etag) if (grapherPageResp.status === 304) { + console.log("Returning 304 for ", slug) return new Response(null, { status: 304 }) } @@ -177,27 +178,31 @@ async function handleConfigRequest( env ) if (redirectSlug && redirectSlug !== slug) { + console.log("Redirecting to ", redirectSlug) return createRedirectResponse( `${redirectSlug}.config.json`, env.url ) } else { + console.log("Returning 404 for ", slug) // Otherwise we just return the status code. return new Response(null, { status: grapherPageResp.status }) } } - console.log("Grapher page response", grapherPageResp.grapherConfig.title) + console.log("Returning 200 for ", slug) const cacheControl = shouldCache ? "public, s-maxage=3600, max-age=0, must-revalidate" : "public, s-maxage=0, max-age=0, must-revalidate" - return new Response(JSON.stringify(grapherPageResp.grapherConfig), { + //grapherPageResp.headers.set("Cache-Control", cacheControl) + return new Response(grapherPageResp.body as any, { + status: 200, headers: { - "content-type": "application/json", + "Content-Type": "application/json", "Cache-Control": cacheControl, - ETag: grapherPageResp.etag, + ETag: grapherPageResp.headers.get("ETag") ?? "", }, }) } diff --git a/functions/grapher/thumbnail/[slug].ts b/functions/grapher/thumbnail/[slug].ts index e49e32508fc..d92306bd67d 100644 --- a/functions/grapher/thumbnail/[slug].ts +++ b/functions/grapher/thumbnail/[slug].ts @@ -1,19 +1,7 @@ +import { Env } from "../../_common/env.js" import { fetchAndRenderGrapher } from "../../_common/grapherRenderer.js" import { IRequestStrict, Router, error } from "itty-router" -export interface Env { - ASSETS: { - fetch: typeof fetch - } - url: URL - GRAPHER_CONFIG_R2_BUCKET_URL: string - GRAPHER_CONFIG_R2_BUCKET_FALLBACK_URL: string - GRAPHER_CONFIG_R2_BUCKET_PATH: string - GRAPHER_CONFIG_R2_BUCKET_FALLBACK_PATH: string - CF_PAGES_BRANCH: string - ENV: string -} - const router = Router() router .get(