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

The secrets page's Server-Timing names its phases: session, each read, render #3452

Merged opened this

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

Merged

scout/forge:scout/3422-the-secrets-page-s-server-timing-names-i → main 1 commit fork 1 thread · 1 unresolved

  • Merged. scout/3422-the-secrets-page-s-server-timing-names-i was merged into main as f831abe.

Review

Sign in to review this change.
2 files changed +60 −2
src/web/pages/secrets.ts +14 −2
@@ -9,6 +9,11 @@
99
1010 import { listSecrets, putSecret, removeSecret } from "../../actions/secrets.ts";
1111 import type { Ctx } from "../../http/context.ts";
12 import {
13 notePhase,
14 takeSessionTime,
15 timePhase,
16 } from "../../core/route_timing.ts";
1217 import {
1318 NAME_MAX,
1419 type SecretName,
@@ -154,19 +159,26 @@ function humanCheckLine(ctx: Ctx): string {
154159
155160 // GET /settings/secrets
156161 export function secretsPage(ctx: Ctx): Response {
162 notePhase(ctx.request, "session", takeSessionTime(ctx.request));
157163 const denied = requireAdmin(ctx);
158164 if (denied) return denied;
159 return page({
165 const card = timePhase(ctx.request, "secrets", () =>
166 secretsSection(ctx, "forge", "")!,
167 );
168 const rendering = performance.now();
169 const response = page({
160170 ctx,
161171 title: "Secrets",
162172 body: html`<div class="page-head"><h1>Secrets</h1></div>
163173 ${settingsLayout(
164174 accountSections("secrets", { admin: true }),
165 html`${secretsSection(ctx, "forge", "")!}
175 html`${card}
166176 <p class="muted small">${humanCheckLine(ctx)}</p>`,
167177 "Account settings",
168178 )}`,
169179 });
180 notePhase(ctx.request, "render", performance.now() - rendering);
181 return response;
170182 }
171183
172184 // POST /settings/secrets
test/secrets-timing.test.ts added +46
@@ -0,0 +1,46 @@
1 /** The secrets page names its phases in Server-Timing (#3422). */
2 import { afterAll, beforeAll, expect, 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 { serve, type Servers } from "../src/http/server.ts";
8 import { seedAdmin } from "./helpers/admin.ts";
9
10 let dataDir = "";
11 let app: App, servers: Servers, bearer = "";
12
13 beforeAll(async () => {
14 dataDir = await mkdtemp(join(tmpdir(), "forge-stime-"));
15 app = createApp({ dataDir, port: 0 });
16 servers = await serve(app);
17 const admin = await seedAdmin(app, { username: "alice", email: "[email protected]", password: "correct-horse-battery" });
18 if (!admin.ok) throw new Error(admin.error);
19 const minted = await fetch(`${servers.url}/api/v1/tokens`, {
20 method: "POST",
21 headers: { "content-type": "application/json" },
22 body: JSON.stringify({ adminConfirm: true, username: "alice", password: "correct-horse-battery", name: "cli", scope: "admin" }),
23 });
24 bearer = ((await minted.json()) as { token: string }).token;
25 });
26
27 afterAll(async () => {
28 servers.stop();
29 app.close();
30 await rm(dataDir, { recursive: true, force: true });
31 });
32
33 test("the app settings page names session, secrets and render once each", async () => {
34 const res = await fetch(`${servers.url}/settings/secrets`, { headers: { authorization: `Bearer ${bearer}` } });

Nit: this is the forge's account secrets page (/settings/secrets), not an app's settings page. Something like "the secrets page names session, secrets and render once each" would say what it covers.

35 const page = await res.text();
36 expect(res.status).toBe(200);
37 expect(page).toContain("Seal");
38 expect(res.headers.get("x-content-type-options")).toBe("nosniff");
39 const parts = (res.headers.get("server-timing") ?? "").split(", ");
40 const names = parts.map((p) => p.split(";")[0]!);
41 for (const n of ["session", "secrets", "render"]) {
42 expect(names.filter((x) => x === n).length).toBe(1);
43 const part = parts.find((p) => p.startsWith(`${n};`))!;
44 expect(Number(/dur=([\d.]+)/.exec(part)?.[1])).not.toBeNaN();
45 }
46 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs