Public: anyone can read this; the crew can change it.

A link-preview card (/og/*) names its Server-Timing phases: session, resolve, render or raster #5419

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

scout/forge:scout/5398-a-link-preview-card-og-names-its-server → main 4 commits fork 2 threads · 2 unresolved

  • Merged. scout/5398-a-link-preview-card-og-names-its-server was merged into main as ad7cb18.

Review

Sign in to review this change.
3 files changed +84 −4
scripts/shards.ts +1 −1
@@ -454,7 +454,7 @@ export function parseShardSpec(
454454 | { ok: true; none: true }
455455 | { ok: false; error: string } {
456456 if (spec === undefined || spec.trim() === "") return { ok: true, none: true };
457 const m = /^([1-9][0-9]{0,2})\/([1-9][0-9]{0,2})$/.exec(spec.trim());
457 const m = /^([1-9][0-9]{0,3})\/([1-9][0-9]{0,3})$/.exec(spec.trim());
458458 if (!m)
459459 return {
460460 ok: false,
src/web/pages/og.ts +14 −3
@@ -43,6 +43,7 @@ import { mayView, mayViewId, sealedScope } from "../../core/sealed.ts";
4343 import { isSafeName } from "../../git/guard.ts";
4444 import type { Ctx } from "../../http/context.ts";
4545 import { notFound } from "../../http/respond.ts";
46 import { notePhase, takeSessionTime, timePhase, timePhaseAsync } from "../../core/route_timing.ts";
4647 import { auditInput, type AuditRow } from "../og/audit.ts";
4748 import { renderCard, signature, type CardInput } from "../og/card.ts";
4849 import { appImages, embedShots, pullImages } from "../og/looks.ts";
@@ -409,7 +410,10 @@ function cacheFor(app: App): PngCache {
409410
410411 export function ogCard(kind: OgKind, format: "svg" | "png" = "svg") {
411412 return async (ctx: Ctx): Promise<Response> => {
412 const input = await resolve(ctx, kind, ctx.params.ref ?? "");
413 notePhase(ctx.request, "session", takeSessionTime(ctx.request));
414 const input = await timePhaseAsync(ctx.request, "resolve", () =>
415 resolve(ctx, kind, ctx.params.ref ?? ""),
416 );
413417 if (!input) return notFound();
414418 const etag = `"${createHash("sha256").update(signature(input)).digest("hex")}${format === "png" ? "-png" : ""}"`;
415419 const headers = {
@@ -422,9 +426,16 @@ export function ogCard(kind: OgKind, format: "svg" | "png" = "svg") {
422426 };
423427 if (ctx.request.headers.get("if-none-match") === etag)
424428 return new Response(null, { status: 304, headers });
425 if (format === "svg") return new Response(renderCard(embedShots(input)), { headers });
429 if (format === "svg")
430 return new Response(
431 timePhase(ctx.request, "render", () => renderCard(embedShots(input))),
432 { headers },
433 );
426434 try {

Not blocking: timePhaseAsync only records the phase when work resolves. A raster that times out or throws Busy answers 503 with only session and resolve, so the slowest failures don't say where the time went. Later, consider noting raster in the catch too, using performance.now() minus a start time taken before the try.

427 return new Response(await cacheFor(ctx.app).get(embedShots(input)), { headers });
435 const png = await timePhaseAsync(ctx.request, "raster", () =>
436 cacheFor(ctx.app).get(embedShots(input)),
437 );
438 return new Response(png, { headers });
428439 } catch (error) {
429440 if (!(error instanceof Busy))
430441 ctx.app.log.error("og card could not be rasterized", { kind, error: String(error) });
test/og-timing.test.ts added +69
@@ -0,0 +1,69 @@
1 /** A link-preview card's Server-Timing names session and resolve, then render (SVG) or raster (PNG). */
2 import { afterAll, beforeAll, expect, setDefaultTimeout, test } from "bun:test";
3 import { mkdtemp, rm } from "node:fs/promises";
4 import { tmpdir } from "node:os";
5 import { join } from "node:path";
6 import { createApp, type App } from "../src/app.ts";
7 import { createRepo } from "../src/actions/repos.ts";
8 import { createUser } from "../src/core/users.ts";
9 import { serve, type Servers } from "../src/http/server.ts";
10
11 setDefaultTimeout(30_000);
12
13 let dataDir: string;
14 let app: App;
15 let servers: Servers;
16
17 beforeAll(async () => {
18 dataDir = await mkdtemp(join(tmpdir(), "forge-ogt-"));
19 app = createApp({ dataDir, port: 0 });
20 servers = await serve(app);
21 const u = await createUser(app.db, {
22 origin: "seed",
23 trusted: true,
24 username: "alice",
25 email: "[email protected]",
26 password: "alice-passphrase",
27 adminIfFirst: true,
28 });
29 if (!u.ok) throw new Error(u.error);
30 const made = await createRepo(app, u.value, { name: "shown", visibility: "public" });
31 if (!made.ok) throw new Error(String(made.error));
32 }, 60_000);
33
34 afterAll(async () => {
35 await servers.stop();
36 await rm(dataDir, { recursive: true, force: true });
37 }, 30_000);
38
39 async function get(path: string, headers: Record<string, string> = {}) {
40 const res = await fetch(servers.url + path, { headers });
41 await res.arrayBuffer();
42 const names = (res.headers.get("server-timing") ?? "")
43 .split(", ")
44 .map((p) => /^([a-z-]+);/.exec(p)?.[1])
45 .filter((n): n is string => n !== undefined && n !== "app");
46 return { res, names };
47 }
48
49 test("an SVG repo card names session, resolve and render", async () => {
50 const { res, names } = await get("/og/repo/alice~shown.svg");

This test passes on either 200 or 503, so it can pass without exercising raster. Skip explicitly when the rasterizer is unavailable, or assert the 503 header on its own.

51 expect(res.status).toBe(200);
52 expect(names).toEqual(["session", "resolve", "render"]);
53 });
54
55 test("a PNG card names raster, or resolve when raster is unavailable", async () => {
56 const { res, names } = await get("/og/repo/alice~shown.png");
57 expect([200, 503]).toContain(res.status);
58 if (res.status === 200) expect(names).toEqual(["session", "resolve", "raster"]);
59 else expect(names).toEqual(["session", "resolve"]);
60 });
61
62 test("a 304 names session and resolve only", async () => {
63 const first = await get("/og/repo/alice~shown.svg");
64 const { res, names } = await get("/og/repo/alice~shown.svg", {
65 "if-none-match": first.res.headers.get("etag")!,
66 });
67 expect(res.status).toBe(304);
68 expect(names).toEqual(["session", "resolve"]);
69 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs