trevato/forge

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

More
Forks1g fMapg aRuns15g rEventsFlows

A repository's app, answers, now and state pages name their Server-Timing phases and answer under 50 ms #4333

Merged scoutcrew opened this

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

ConversationFiles
Merged

scout/forge:scout/4320-a-repository-s-app-answers-now-and-state → main 1 commit fork

  • Merged. scout/4320-a-repository-s-app-answers-now-and-state was merged into main as cc745ad.

Review

Sign in to review this change.
7 files changed +132 −21
src/web/layout.ts +9
@@ -35,6 +35,7 @@ import {
3535 type Flash,
3636 } from "./flash.ts";
3737 import { prefersJson } from "./form.ts";
38 import { notePhase } from "../core/route_timing.ts";
3839 import { html, raw, when, type Html } from "./html.ts";
3940 import { noRoom } from "../spawn/door.ts";
4041 import { icon, iconSprite, inlineIcons, themeGlyph } from "./icons.ts";
@@ -761,6 +762,14 @@ export function fragment(
761762 );
762763 }
763764
765 /** The empty 404 fragment, its build named `render` so the request's phases still add up. */
766 export function missingFragment(request: Request): Response {
767 const started = performance.now();
768 const response = fragment(html``, { status: 404 });
769 notePhase(request, "render", performance.now() - started);
770 return response;
771 }
772
764773 /** Redirect back to a page carrying a message — the shape most handlers want. */
765774 export function flashTo(ctx: Ctx, to: string, flash: Flash): Response {
766775 return redirect(to, { flash, url: ctx.url });
src/web/pages/apps.ts +13 −7
@@ -607,13 +607,19 @@ export function appHref(repo: Repo): string {
607607 // The old URL keeps answering: a reader of an app is sent to the studio app
608608 // page; anyone else gets the same not-found the page always gave them.
609609 export function appPage(ctx: Ctx): Response {
610 const repo = loadRepo(ctx);
611 if (!repo) return notFoundPage(ctx);
612 if (!appView(ctx.app, ctx.auth.user, repo).ok) return notFoundPage(ctx);
613 return new Response(null, {
614 status: 302,
615 headers: { location: appHref(repo), "cache-control": "no-store" },
616 });
610 const req = ctx.request;
611 notePhase(req, "session", takeSessionTime(req));
612 const repo = timePhase(req, "repo", () => loadRepo(ctx));
613 const seen = repo !== undefined && timePhase(req, "view", () => appView(ctx.app, ctx.auth.user, repo).ok);
614 const rendering = performance.now();
615 const response = !repo || !seen
616 ? notFoundPage(ctx)
617 : new Response(null, {
618 status: 302,
619 headers: { location: appHref(repo), "cache-control": "no-store" },
620 });
621 notePhase(req, "render", performance.now() - rendering);
622 return response;
617623 }
618624
619625 // ---- the pull page's preview card ----
src/web/studio/answers.ts +2 −2
@@ -18,7 +18,7 @@ import { getUsersByIds, type User } from "../../core/users.ts";
1818 import type { Ctx } from "../../http/context.ts";
1919 import { loadRepo, repoBase } from "../chrome.ts";
2020 import { html, type Html } from "../html.ts";
21 import { fragment } from "../layout.ts";
21 import { fragment, missingFragment } from "../layout.ts";
2222 import { time } from "../ui.ts";
2323 import { tellHref } from "./nouns.ts";
2424
@@ -116,7 +116,7 @@ export function appAnswersFragment(ctx: Ctx): Response {
116116 const repo = timePhase(ctx.request, "repo", () => loadRepo(ctx));
117117 const user = ctx.auth.user;
118118 if (!repo || !timePhase(ctx.request, "access", () => canSeeAnswers(ctx.app.db, repo, user)))
119 return fragment(html``, { status: 404 });
119 return missingFragment(ctx.request);
120120 const answers = timePhase(ctx.request, "answers", () => answersFor(ctx.app.db, repo, user!));
121121 const rendering = performance.now();
122122 const region = answersRegion(ctx, repo, user!, answers);
src/web/studio/now.ts +3 −4
@@ -24,7 +24,7 @@ import { readRunLogTail } from "../../runs/log.ts";
2424 import { mayReadRunLog } from "../../actions/runs.ts";
2525 import { loadRepo, repoBase } from "../chrome.ts";
2626 import { html, type Html } from "../html.ts";
27 import { fragment } from "../layout.ts";
27 import { fragment, missingFragment } from "../layout.ts";
2828 import { avatar, badge, btn, card, icon, listCard, row, time } from "../ui.ts";
2929 import { changeHref, tellHref } from "./nouns.ts";
3030 import { LOG_TAIL_CAP, logBox } from "./log.ts";
@@ -334,10 +334,9 @@ export async function appNowFragment(ctx: Ctx): Promise<Response> {
334334 const req = ctx.request;
335335 notePhase(req, "session", takeSessionTime(req));
336336 const repo = timePhase(req, "repo", () => loadRepo(ctx));
337 if (!repo) return fragment(html``, { status: 404 });
338337 const db = ctx.app.db;
339 const record = timePhase(req, "record", () => getAppByRepo(db, repo.id));
340 if (!record) return fragment(html``, { status: 404 });
338 const record = repo && timePhase(req, "record", () => getAppByRepo(db, repo.id));
339 if (!repo || !record) return missingFragment(req);
341340 const openPulls = timePhase(req, "pulls", () =>
342341 listPulls(db, repo.id, {
343342 state: "open",
src/web/studio/previews.ts +3 −4
@@ -25,7 +25,7 @@ import { readRunLogTail } from "../../runs/log.ts";
2525 import { loadRepo, repoBase } from "../chrome.ts";
2626 import { testsClause } from "./words.ts";
2727 import { html, when, type Html } from "../html.ts";
28 import { fragment } from "../layout.ts";
28 import { fragment, missingFragment } from "../layout.ts";
2929 import {
3030 badge,
3131 btn,
@@ -361,9 +361,8 @@ export function appStateFragment(ctx: Ctx): Response {
361361 const req = ctx.request;
362362 notePhase(req, "session", takeSessionTime(req));
363363 const repo = timePhase(req, "repo", () => loadRepo(ctx));
364 if (!repo) return fragment(html``, { status: 404 });
365 const record = timePhase(req, "record", () => getAppByRepo(ctx.app.db, repo.id));
366 if (!record) return fragment(html``, { status: 404 });
364 const record = repo && timePhase(req, "record", () => getAppByRepo(ctx.app.db, repo.id));
365 if (!repo || !record) return missingFragment(req);
367366 const openPulls = timePhase(req, "pulls", () =>
368367 listPulls(ctx.app.db, repo.id, {
369368 state: "open",
test/app-pages-timing.test.ts added +102
@@ -0,0 +1,102 @@
1 /** `/:owner/:name/app` and its `answers`, `now` and `state` fragments name their Server-Timing phases and answer under 50 ms (#4320). */
2 import { afterAll, beforeAll, expect, test } from "bun:test";
3 import { mkdtempSync, rmSync } from "node:fs";
4 import { tmpdir } from "node:os";
5 import { join } from "node:path";
6 import { enableApp } from "../src/actions/apps.ts";
7 import { commitToBranch, createRepo } from "../src/actions/repos.ts";
8 import { createApp, type App } from "../src/app.ts";
9 import { createUser } from "../src/core/users.ts";
10 import { serve, type Servers } from "../src/http/server.ts";
11 import { proveAll } from "./helpers/verified.ts";
12 import { quietLogs } from "./helpers/globals.ts";
13
14 quietLogs();
15
16 let root = "";
17 let app: App;
18 let servers: Servers;
19 let cookie = "";
20
21 beforeAll(async () => {
22 root = mkdtempSync(join(tmpdir(), "forge-app-pages-timing-"));
23 app = createApp({ dataDir: root, port: 0, forgeRepo: "alice/forge" });
24 const u = await createUser(app.db, {
25 origin: "seed",
26 trusted: true,
27 username: "alice",
28 email: "[email protected]",
29 password: "a-good-passphrase",
30 adminIfFirst: true,
31 });
32 if (!u.ok) throw new Error(u.error);
33 proveAll(app);
34 const made = await createRepo(app, { id: u.value.id, username: "alice" } as never, { name: "vault" });
35 if (!made.ok) throw new Error(String(made.error));
36 const seeded = await commitToBranch(app, u.value, made.value, {
37 message: "Start",
38 files: [{ path: "README.md", content: "v\n" }],
39 });
40 if (!seeded.ok) throw new Error(JSON.stringify(seeded.error));
41 const enabled = await enableApp(app, u.value, made.value, { slug: "vault" });
42 if (!enabled.ok) throw new Error(String(enabled.error));
43 // A second repository with no app: the not-found paths are named too.
44 const bare = await createRepo(app, { id: u.value.id, username: "alice" } as never, { name: "bare" });
45 if (!bare.ok) throw new Error(String(bare.error));
46 servers = await serve(app);
47 const signin = await fetch(`${servers.url}/signin`, {
48 method: "POST",
49 headers: { origin: servers.url },
50 body: new URLSearchParams({ username: "alice", password: "a-good-passphrase" }),
51 redirect: "manual",
52 });
53 cookie = (signin.headers.get("set-cookie") ?? "").split(";")[0]!;
54 });
55 afterAll(() => {
56 servers.stop();
57 app.close();
58 rmSync(root, { recursive: true, force: true });
59 });
60
61 /** Route → the reads it makes, between `session` and `render`. */
62 const ROUTES: [string, string[]][] = [
63 ["/app", ["repo", "view"]],
64 ["/app/answers", ["repo", "access", "answers"]],
65 ["/app/now", ["repo", "record", "pulls", "crew", "owner", "panels"]],
66 ["/app/state", ["repo", "record", "pulls", "panels"]],
67 ];
68
69 function phases(header: string | null): Map<string, number> {
70 const out = new Map<string, number>();
71 for (const part of (header ?? "").split(", ")) {
72 const m = /^([a-z]+);dur=(\d+\.\d)/.exec(part);
73 if (m) out.set(m[1]!, Number(m[2]));
74 }
75 return out;
76 }
77
78 const get = (path: string) =>
79 fetch(`${servers.url}${path}`, { headers: { cookie }, redirect: "manual" });
80
81 for (const [route, reads] of ROUTES)
82 test(`GET /alice/vault${route} names its phases and answers under 50 ms`, async () => {
83 await (await get(`/alice/vault${route}`)).arrayBuffer(); // warm
84 const res = await get(`/alice/vault${route}`);
85 await res.arrayBuffer();
86 expect(res.status).toBeLessThan(400);
87 const got = phases(res.headers.get("server-timing"));
88 for (const name of ["session", ...reads, "render"]) expect(got.has(name)).toBe(true);
89 const total = got.get("app")!;
90 expect(total).toBeLessThan(50);
91 const sum = ["session", ...reads, "render"].reduce((n, k) => n + got.get(k)!, 0);
92 expect(Math.abs(total - sum)).toBeLessThanOrEqual(1.5); // each is rounded to 0.1
93 });
94
95 for (const [route] of ROUTES)
96 test(`GET /alice/bare${route} names session and render when there is no app`, async () => {
97 const res = await get(`/alice/bare${route}`);
98 await res.arrayBuffer();
99 const got = phases(res.headers.get("server-timing"));
100 expect(got.has("session")).toBe(true);
101 expect(got.has("render")).toBe(true);
102 });
test/server-timing-sweep.test.ts −4
@@ -22,10 +22,6 @@ quietLogs();
2222 const UNNAMED: Array<[path: string, issue: string]> = [
2323 ["/-/confirm", "#3390"],
2424 ["/alice/forge", "#3325"],
25 ["/alice/forge/app", "#3390"],
26 ["/alice/forge/app/answers", "#3390"],
27 ["/alice/forge/app/now", "#3390"],
28 ["/alice/forge/app/state", "#3390"],
2925 ["/alice/forge/changes", "#3323"],
3026 ["/alice/forge/issues/new", "#3390"],
3127 ["/alice/forge/make-it-yours", "#3390"],

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs