Skip to content
Sign up

flagon-io/g1t

public
0

Commit

Merge branch 'claude/zen-ptolemy-01vsx1': never leave a page stuck on its loading bar

Claudeagentcommitted Parents4ef467f0ddb02cBrowse files
7 files+296−100/7 viewed
+77−0
1+import assert from "node:assert/strict";
2+import { test } from "node:test";
3+
4+import {
5+ DeadlineExceeded,
6+ LONG_DEADLINE_MS,
7+ READ_DEADLINE_MS,
8+ SLOW_READ_DEADLINE_MS,
9+ WRITE_DEADLINE_MS,
10+ deadlineFor,
11+ withDeadline,
12+} from "./deadline.ts";
13+import { STUCK_NAVIGATION_MS } from "./stuck.ts";
14+
15+test("an answer in time is passed through", async () => {
16+ assert.equal(await withDeadline("repos get_repo", 1000, async () => "ok"), "ok");
17+});
18+
19+test("a call that never answers fails at its deadline, and its signal aborts", async () => {
20+ let seen: AbortSignal | undefined;
21+ const never = withDeadline("repos get_repo", 20, (signal) => {
22+ seen = signal;
23+ return new Promise<never>(() => {});
24+ });
25+ await assert.rejects(never, (error: unknown) => error instanceof DeadlineExceeded && /repos get_repo did not answer within 20ms/.test(error.message));
26+ assert.equal(seen?.aborted, true);
27+});
28+
29+test("a call that ignores its signal is still let go", async () => {
30+ await assert.rejects(withDeadline("chat sidebar", 10, () => new Promise<never>(() => {})), DeadlineExceeded);
31+});
32+
33+test("a failure in time is the call's own, not a deadline", async () => {
34+ await assert.rejects(
35+ withDeadline("work get_pull", 1000, async () => {
36+ throw new Error("get_pull failed with status 500");
37+ }),
38+ /status 500/,
39+ );
40+});
41+
42+test("the caller's own signal aborts the call", async () => {
43+ const outer = new AbortController();
44+ let seen: AbortSignal | undefined;
45+ const call = withDeadline(
46+ "repos get_repo",
47+ 1000,
48+ (signal) =>
49+ new Promise<never>((_, reject) => {
50+ seen = signal;
51+ signal.addEventListener("abort", () => reject(signal.reason));
52+ }),
53+ outer.signal,
54+ );
55+ outer.abort(new Error("visitor left"));
56+ await assert.rejects(call, /visitor left/);
57+ assert.equal(seen?.aborted, true);
58+});
59+
60+test("each call gets the time its work needs", () => {
61+ assert.equal(deadlineFor("repos", "get_repo", false), READ_DEADLINE_MS);
62+ assert.equal(deadlineFor("work", "merge_pull", true), WRITE_DEADLINE_MS);
63+ assert.equal(deadlineFor("security", "overview", false), SLOW_READ_DEADLINE_MS);
64+ assert.equal(deadlineFor("repos", "blame", false), SLOW_READ_DEADLINE_MS);
65+ assert.equal(deadlineFor("integrations", "github_import", true), LONG_DEADLINE_MS);
66+ assert.equal(deadlineFor("agents", "computer_sleep", true), LONG_DEADLINE_MS);
67+});
68+
69+test("a long method of one service is not long in another", () => {
70+ assert.equal(deadlineFor("agents", "draft", true), LONG_DEADLINE_MS);
71+ assert.equal(deadlineFor("work", "draft", true), WRITE_DEADLINE_MS);
72+ assert.equal(deadlineFor("identity", "overview", false), READ_DEADLINE_MS);
73+});
74+
75+test("no read outlasts the browser's patience with a navigation", () => {
76+ assert.ok(SLOW_READ_DEADLINE_MS < STUCK_NAVIGATION_MS && READ_DEADLINE_MS < STUCK_NAVIGATION_MS);
77+});
+109−0
1+/**
2+ * How long the site waits on a service before it gives up. Nothing on
3+ * Workers ends a subrequest that stops answering: the page waits as long
4+ * as the visitor does, so a service stuck on one call holds the page on
5+ * its loading bar for good. With a deadline it fails instead, as an error
6+ * the page can draw and the next click can get past.
7+ */
8+
9+/** A read: no page should wait longer than this on one service call. */
10+export const READ_DEADLINE_MS = 15_000;
11+
12+/** A write: some do real work (a merge, a commit), so they get longer. */
13+export const WRITE_DEADLINE_MS = 60_000;
14+
15+/** Reads that do real work inline the first time, or walk history. */
16+export const SLOW_READ_DEADLINE_MS = 60_000;
17+
18+/** Jobs a person starts and waits on: an import, a model's draft, a computer waking. */
19+export const LONG_DEADLINE_MS = 600_000;
20+
21+/**
22+ * Reads that can take longer than `READ_DEADLINE_MS` and still be right,
23+ * by service binding and method. Cut off, their work is lost and starts
24+ * again on the next visit, so they get the time to finish.
25+ */
26+const SLOW_READS = new Set([
27+ // A repository's first look runs its dependency scan inline (services/security/src/deps.rs).
28+ "security overview",
29+ "security dependency_graph",
30+ "security sbom",
31+ // Walks up to 400 commits, reading trees and blobs one after another (services/repos/src/blame.rs).
32+ "repos blame",
33+ // Up to 2,500 store reads per call (services/repos/src/last_commits.rs).
34+ "repos last_commits",
35+]);
36+
37+/**
38+ * Writes that run for minutes and still finish: copying a repository in,
39+ * syncing a mirror, a model writing a draft, a computer's home moved in or
40+ * out of storage, a scan over many repositories.
41+ */
42+const LONG_CALLS = new Set([
43+ "repos create",
44+ "integrations github_import",
45+ "integrations mirror_add",
46+ "integrations mirror_sync",
47+ "integrations mirror_move_in",
48+ "integrations mirror_hand_back",
49+ "agents draft",
50+ "agents try_draft",
51+ "agents redraft",
52+ "agents draft_skill",
53+ "agents sync_skill_mirror",
54+ "agents computer_wake",
55+ "agents computer_sleep",
56+ "security dry_run_pattern",
57+ "security rescan",
58+ "security check_updates",
59+ "artifacts add_repo_space",
60+]);
61+
62+/** How long a call to `service`'s `method` may take; `writes` as lib/perf.ts `mayWrite` says. */
63+export function deadlineFor(service: string, method: string, writes: boolean): number {
64+ const call = `${service} ${method}`;
65+ if (LONG_CALLS.has(call)) return LONG_DEADLINE_MS;
66+ if (SLOW_READS.has(call)) return SLOW_READ_DEADLINE_MS;
67+ return writes ? WRITE_DEADLINE_MS : READ_DEADLINE_MS;
68+}
69+
70+/** What a call past its deadline throws. */
71+export class DeadlineExceeded extends Error {
72+ constructor(what: string, ms: number) {
73+ super(`${what} did not answer within ${ms}ms`);
74+ this.name = "DeadlineExceeded";
75+ }
76+}
77+
78+/**
79+ * `run`'s answer, or a `DeadlineExceeded` after `ms`. `run` is handed a
80+ * signal that aborts at the deadline, so the call it makes is cancelled
81+ * too; `outer`, when given, aborts it sooner. Whatever `run` reads must be
82+ * read inside it: the deadline ends when it returns.
83+ */
84+export async function withDeadline<T>(
85+ what: string,
86+ ms: number,
87+ run: (signal: AbortSignal) => Promise<T>,
88+ outer?: AbortSignal | null,
89+): Promise<T> {
90+ const controller = new AbortController();
91+ let timer: ReturnType<typeof setTimeout> | undefined;
92+ const expired = new Promise<never>((_, reject) => {
93+ timer = setTimeout(() => {
94+ const error = new DeadlineExceeded(what, ms);
95+ controller.abort(error);
96+ reject(error);
97+ }, ms);
98+ });
99+ const stop = () => controller.abort(outer?.reason);
100+ if (outer?.aborted) stop();
101+ else outer?.addEventListener("abort", stop, { once: true });
102+ try {
103+ // Raced as well as aborted: a binding that ignores the signal still lets go.
104+ return await Promise.race([run(controller.signal), expired]);
105+ } finally {
106+ clearTimeout(timer);
107+ outer?.removeEventListener("abort", stop);
108+ }
109+}
+32−4
1818 sessionFor,
1919 SESSION_SERVICES,
2020 } from "./perf";
21+import { deadlineFor, withDeadline } from "./deadline";
2122
2223 /**
2324 * What one request to the site did: when it started, each service call
7172 return {
7273 async fetch(input: string, init?: RequestInit) {
7374 const perf = scope.getStore();
74− if (!perf) return binding.fetch(input, init);
75+ const method = rpcMethodOf(input);
76+ if (!perf) return answerInTime(binding, name, method, input, init);
7577 const session = sessionFor(name, perf.bookmarks, perf.writing, Math.floor(Date.now() / 1000));
7678 let sent = init;
7779 if (session) {
8082 sent = { ...init, headers };
8183 perf.sessions.set(name, session.startsWith("first-") ? session.slice(6) : "bookmark");
8284 }
83− if (SESSION_SERVICES.has(name) && mayWrite(rpcMethodOf(input))) perf.wrote = true;
85+ if (SESSION_SERVICES.has(name) && mayWrite(method)) perf.wrote = true;
8486 const from = Date.now();
85− const response = await binding.fetch(input, sent);
87+ let response: Response;
88+ try {
89+ response = await answerInTime(binding, name, method, input, sent);
90+ } finally {
91+ // A call that failed or ran out of time still took the page's time.
92+ perf.intervals.push([from, Date.now()]);
93+ }
8694 const to = Date.now();
87− perf.intervals.push([from, to]);
8895 const timing = (perf.services[name] ??= { calls: 0, wallMs: 0, serviceMs: 0 });
8996 timing.calls += 1;
9097 timing.wallMs += to - from;
103110 }
104111
105112 /**
113+ * One call to `binding`, answered in full (body read) within its deadline
114+ * (lib/deadline.ts): a service that stops answering fails the call rather
115+ * than holding the page open. RPC answers are small JSON, so reading the
116+ * body here costs nothing and puts it under the deadline too.
117+ */
118+async function answerInTime(binding: ServiceBinding, name: string, method: string, input: string, init?: RequestInit): Promise<Response> {
119+ const ms = deadlineFor(name, method, mayWrite(method));
120+ return withDeadline(
121+ `${name} ${method || input}`,
122+ ms,
123+ async (signal) => {
124+ const response = await binding.fetch(input, { ...init, signal });
125+ const body = await response.arrayBuffer();
126+ // A 204 or 304 may not be rebuilt with a body, even an empty one.
127+ return new Response(body.byteLength > 0 ? body : null, response);
128+ },
129+ init?.signal,
130+ );
131+}
132+
133+/**
106134 * Whether this request must read current data: it writes, or the person
107135 * wrote moments ago (lib/perf.ts `PRIMARY_WINDOW_SECONDS`). Caches step
108136 * aside then (lib/cache.server.ts).
+18−0
1+import assert from "node:assert/strict";
2+import { test } from "node:test";
3+
4+import { stuckTarget } from "./stuck.ts";
5+
6+const location = { pathname: "/flagon-io/g1t/pulls", search: "?state=closed", hash: "#top" };
7+
8+test("an idle page has nothing to retry", () => {
9+ assert.equal(stuckTarget({ state: "idle" }), null);
10+});
11+
12+test("a navigation still loading is retried at its full address", () => {
13+ assert.equal(stuckTarget({ state: "loading", location }), "/flagon-io/g1t/pulls?state=closed#top");
14+});
15+
16+test("a form still being sent is never sent again", () => {
17+ assert.equal(stuckTarget({ state: "submitting", location }), null);
18+});
+41−0
1+import { useEffect } from "react";
2+import { type Location, createPath, useNavigation } from "react-router";
3+
4+/**
5+ * How long a client navigation may load before the page gives up on it and
6+ * loads the address as a whole page. Longer than any read may take on the
7+ * server (lib/deadline.ts `SLOW_READ_DEADLINE_MS`): a slow page that would
8+ * arrive is never cut off and started again, and a stuck service shows its
9+ * error page first. This catches what the server cannot: a request lost on
10+ * the way, a connection that went quiet, a route's code that never arrived.
11+ */
12+export const STUCK_NAVIGATION_MS = 75_000;
13+
14+/**
15+ * Where to send a navigation that has been loading too long, or null when
16+ * there is nothing safe to retry. Only "loading" is retried: it reads, even
17+ * after a form was posted (the action already answered and the page is
18+ * reading what it changed). "submitting" is a write still on its way, and
19+ * sending it again could do it twice.
20+ */
21+export function stuckTarget(navigation: { state: string; location?: Pick<Location, "pathname" | "search" | "hash"> | null }): string | null {
22+ if (navigation.state !== "loading" || !navigation.location) return null;
23+ return createPath(navigation.location);
24+}
25+
26+/**
27+ * Never leaves the page on its loading bar: a navigation still loading
28+ * after `STUCK_NAVIGATION_MS` is finished as a document load of the same
29+ * address, which either arrives or shows the server's error page.
30+ */
31+export function useStuckNavigationRescue(ms = STUCK_NAVIGATION_MS) {
32+ const target = stuckTarget(useNavigation());
33+ useEffect(() => {
34+ if (!target) return;
35+ const timer = window.setTimeout(() => {
36+ console.warn(`g1t: navigation to ${target} still loading after ${ms}ms; loading it as a whole page`);
37+ window.location.assign(target);
38+ }, ms);
39+ return () => window.clearTimeout(timer);
40+ }, [target, ms]);
41+}
+3−0
4949 import { addresses } from "./lib/addresses.server";
5050 import { RELOADED_KEY, RELOAD_GIVE_UP_MS, clientNavigated, reloadFixes, reloadedBefore } from "./lib/stale-build";
5151 import { useNonce } from "./lib/nonce";
52+import { useStuckNavigationRescue } from "./lib/stuck";
5253 import { isNeedsSignIn } from "./lib/website-token";
5354 import { offerTicket, setLiveViaToken } from "./lib/live-socket";
5455 import { socketTicketFor } from "./lib/socket-ticket.server";
476477 identify(userId, lastUserId.current);
477478 lastUserId.current = userId;
478479 }, [userId]);
480+ // A navigation that never arrives is loaded as a whole page (lib/stuck.ts).
481+ useStuckNavigationRescue();
479482 return <Outlet />;
480483 }
481484
+16−6
77 import { hardenRegistryHeaders } from "../app/lib/content-safety";
88 import { withSiteHeaders } from "../app/lib/page-headers";
99 import { finishResponse, withRequestPerf } from "../app/lib/perf.server";
10+import { READ_DEADLINE_MS, withDeadline } from "../app/lib/deadline";
1011 import { gitLimited, pageLimited, usercontentLimited } from "../app/lib/front-door-limits";
1112 import { goImport } from "../app/lib/go-get";
1213 import { repositoryOfPage, stillPublic } from "../app/lib/public-cache";
125126 /** Fresh for this long; then served once more while a new copy is made. */
126127 const PUBLIC_FRESH_SECONDS = 30;
127128 const PUBLIC_STALE_SECONDS = 300;
129+/** The longest a cache lookup holds up a public page. */
130+const CACHE_DEADLINE_MS = 2_000;
128131
129132 function anonymousPage(request: Request, pathname: string): boolean {
130133 if (request.method !== "GET") return false;
142145 const key = new Request(keyUrl, { method: "GET" });
143146 const repository = PUBLIC_PROJECT.test(new URL(request.url).pathname) ? repositoryOfPage(new URL(request.url).pathname) : null;
144147 // Asked alongside the cache, so a hit waits for one indexed read at most.
145− const [cached, visible] = await Promise.all([cache.match(key), repository ? isStillPublic(env, repository) : Promise.resolve(true)]);
148+ // A cache that does not answer is a miss: the page is drawn afresh.
149+ const [cached, visible] = await Promise.all([
150+ withDeadline("cache", CACHE_DEADLINE_MS, () => cache.match(key)).catch(() => undefined),
151+ repository ? isStillPublic(env, repository) : Promise.resolve(true),
152+ ]);
146153 if (cached && !visible) {
147154 ctx.waitUntil(cache.delete(key).then(() => undefined, () => undefined));
148155 return render();
179186 /** Whether the repository is there and public; anything else, including no answer, is no. */
180187 async function isStillPublic(env: Env, repository: string): Promise<boolean> {
181188 try {
182− const answer = await env.REPOS.fetch("https://service/rpc/visibility", {
183− method: "POST",
184− headers: { "content-type": "application/json" },
185− body: JSON.stringify({ paths: [repository] }),
189+ return await withDeadline("repos visibility", READ_DEADLINE_MS, async (signal) => {
190+ const answer = await env.REPOS.fetch("https://service/rpc/visibility", {
191+ method: "POST",
192+ headers: { "content-type": "application/json" },
193+ body: JSON.stringify({ paths: [repository] }),
194+ signal,
195+ });
196+ return answer.ok && stillPublic(await answer.json(), repository);
186197 });
187− return answer.ok && stillPublic(await answer.json(), repository);
188198 } catch {
189199 return false;
190200 }