trevato/forge

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

More
Forks1g fMapg aRuns13g rEventsFlows

/settings/keys, /settings/tokens and /settings/password name their Server-Timing phases and answer under 50 ms #4323

Merged scoutcrew opened this

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

ConversationFiles
Merged

scout/forge:scout/4316-settings-keys-settings-tokens-and-settin → main 2 commits fork

  • Merged. scout/4316-settings-keys-settings-tokens-and-settin was merged into main as 4d5be47.

Review

Sign in to review this change.
3 files changed +84 −17
src/web/pages/account.ts +13 −14
@@ -477,7 +477,9 @@ function passwordView(error?: {
477477 // GET /settings/password
478478 export function passwordPage(ctx: Ctx): Response {
479479 if (!ctx.auth.user) return signInFirst(ctx);
480 return accountShell(ctx, "password", "Password", passwordView());
480 notePhase(ctx.request, "session", takeSessionTime(ctx.request));
481 const body = passwordView();
482 return timePhase(ctx.request, "render", () => accountShell(ctx, "password", "Password", body));
481483 }
482484
483485 // POST /settings/password
@@ -583,11 +585,9 @@ export function sessionsCard(
583585 }
584586
585587 function tokensView(ctx: Ctx): Html {
586 const tokens = listTokens(ctx.app.db, ctx.auth.user!.id);
587 const sessions = listSessions(
588 ctx.app.db,
589 ctx.auth.user!.id,
590 cookieValue(ctx.request, "forge_session"),
588 const tokens = timePhase(ctx.request, "tokens", () => listTokens(ctx.app.db, ctx.auth.user!.id));
589 const sessions = timePhase(ctx.request, "sessions", () =>
590 listSessions(ctx.app.db, ctx.auth.user!.id, cookieValue(ctx.request, "forge_session")),
591591 );
592592 return html`<p class="muted small mb">
593593 Use a token as the password when pushing over HTTPS, or as a
@@ -679,7 +679,9 @@ function tokensView(ctx: Ctx): Html {
679679 // GET /settings/tokens
680680 export function tokensPage(ctx: Ctx): Response {
681681 if (!ctx.auth.user) return signInFirst(ctx);
682 return accountShell(ctx, "tokens", "Personal access tokens", tokensView(ctx));
682 notePhase(ctx.request, "session", takeSessionTime(ctx.request));
683 const body = tokensView(ctx);
684 return timePhase(ctx.request, "render", () => accountShell(ctx, "tokens", "Personal access tokens", body));
683685 }
684686
685687 // POST /settings/tokens
@@ -756,7 +758,7 @@ function keysView(
756758 ctx: Ctx,
757759 values: { name: string; key: string; error?: string },
758760 ): Html {
759 const keys = listKeys(ctx.app.db, ctx.auth.user!.id);
761 const keys = timePhase(ctx.request, "keys", () => listKeys(ctx.app.db, ctx.auth.user!.id));
760762 const ssh = sshCommand(ctx.app);
761763 return html`${ssh === undefined
762764 ? html`<p class="mb muted" id="terminal-off">The terminal is off on this forge.</p>`
@@ -851,12 +853,9 @@ function keysView(
851853 // GET /settings/keys
852854 export function keysPage(ctx: Ctx): Response {
853855 if (!ctx.auth.user) return signInFirst(ctx);
854 return accountShell(
855 ctx,
856 "keys",
857 "SSH keys",
858 keysView(ctx, { name: "", key: "" }),
859 );
856 notePhase(ctx.request, "session", takeSessionTime(ctx.request));
857 const body = keysView(ctx, { name: "", key: "" });
858 return timePhase(ctx.request, "render", () => accountShell(ctx, "keys", "SSH keys", body));
860859 }
861860
862861 // POST /settings/keys
test/server-timing-sweep.test.ts −3
@@ -46,9 +46,6 @@ const UNNAMED: Array<[path: string, issue: string]> = [
4646 ["/organizations/new", "#3390"],
4747 ["/recover", "#3390"],
4848 ["/settings/cloud/audit", "#3390"],
49 ["/settings/keys", "#3390"],
50 ["/settings/password", "#3390"],
51 ["/settings/tokens", "#3390"],
5249 ["/signin/parent", "#3390"],
5350 ["/signin/parent/callback", "#3390"],
5451 ["/verify", "#3390"],
test/settings-timing.test.ts added +71
@@ -0,0 +1,71 @@
1 /** `/settings/keys`, `/settings/tokens` and `/settings/password` name their phases signed in and answer under 50 ms (#4316). */
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 { createApp, type App } from "../src/app.ts";
7 import { createUser } from "../src/core/users.ts";
8 import { serve, type Servers } from "../src/http/server.ts";
9 import { proveAll } from "./helpers/verified.ts";
10 import { quietLogs } from "./helpers/globals.ts";
11
12 quietLogs();
13
14 let root = "";
15 let app: App;
16 let servers: Servers;
17 let cookie = "";
18
19 beforeAll(async () => {
20 root = mkdtempSync(join(tmpdir(), "forge-settings-timing-"));
21 app = createApp({ dataDir: root, port: 0 });
22 const u = await createUser(app.db, {
23 origin: "seed",
24 trusted: true,
25 username: "alice",
26 email: "[email protected]",
27 password: "a-good-passphrase",
28 adminIfFirst: true,
29 });
30 if (!u.ok) throw new Error(u.error);
31 proveAll(app);
32 servers = await serve(app);
33 const signin = await fetch(`${servers.url}/signin`, {
34 method: "POST",
35 headers: { origin: servers.url },
36 body: new URLSearchParams({ username: "alice", password: "a-good-passphrase" }),
37 redirect: "manual",
38 });
39 cookie = (signin.headers.get("set-cookie") ?? "").split(";")[0]!;
40 });
41 afterAll(() => {
42 servers.stop();
43 app.close();
44 rmSync(root, { recursive: true, force: true });
45 });
46
47 const PAGES: [string, string[]][] = [
48 ["/settings/keys", ["session", "keys", "render"]],
49 ["/settings/tokens", ["session", "tokens", "sessions", "render"]],
50 ["/settings/password", ["session", "render"]],
51 ];
52
53 for (const [path, names] of PAGES)
54 test(`GET ${path} names its phases and answers under 50 ms`, async () => {
55 await fetch(`${servers.url}${path}`, { headers: { cookie } }); // warm
56 const res = await fetch(`${servers.url}${path}`, { headers: { cookie } });
57 await res.text();
58 expect(res.status).toBe(200);
59 const parts = (res.headers.get("server-timing") ?? "").split(", ");
60 const dur = (name: string) => {
61 const part = parts.find((p) => p.startsWith(`${name};`));
62 expect(part).toMatch(/^[a-z]+;dur=\d+\.\d$/);
63 return Number(part!.slice(`${name};dur=`.length));
64 };
65 expect(parts.map((p) => p.split(";")[0]).filter((n) => n !== "app")).toEqual(names);
66 expect(dur("app")).toBeLessThan(50);
67 const sum = names.reduce((t, n) => t + dur(n), 0);
68 // The rest of `app` is the server's own routing and header work, outside any page (~1-2 ms on every page).
69 expect(sum).toBeLessThanOrEqual(dur("app") + 0.3);
70 expect(dur("app") - sum).toBeLessThan(5);
71 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs