Give slow reads and long jobs the time they need to finish
A cut-off call loses its work: a repository's first security overview runs its dependency scan inline, so at 15s it would fail on every visit; an import, a mirror sync, a model's draft or a computer waking can take minutes. Those calls, by service and method, now get 60s (slow reads) or 10 minutes (long jobs); everything else keeps 15s for reads and 60s for writes. The long jobs are all actions, which the browser never retries. The browser's stuck-navigation rescue now waits 75s, past the longest read the server allows, so a slow page that would arrive is never cut off and started again.
- Claude-Session:
- https://claude.ai/code/session_014hsoDNfMdQ3Uk8giKNG2YG
4 files+93−100/4 viewed
| 1 | 1 | import assert from "node:assert/strict"; | |
| 2 | 2 | import { test } from "node:test"; | |
| 3 | 3 | ||
| 4 | − | import { DeadlineExceeded, withDeadline } from "./deadline.ts"; | |
| 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"; | |
| 5 | 14 | ||
| 6 | 15 | test("an answer in time is passed through", async () => { | |
| 7 | 16 | assert.equal(await withDeadline("repos get_repo", 1000, async () => "ok"), "ok"); | |
| ⋯ | |||
| 47 | 56 | await assert.rejects(call, /visitor left/); | |
| 48 | 57 | assert.equal(seen?.aborted, true); | |
| 49 | 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 | + | }); | |
| 9 | 9 | /** A read: no page should wait longer than this on one service call. */ | |
| 10 | 10 | export const READ_DEADLINE_MS = 15_000; | |
| 11 | 11 | ||
| 12 | − | /** A write: some do real work (an import, a merge), so they get longer. */ | |
| 12 | + | /** A write: some do real work (a merge, a commit), so they get longer. */ | |
| 13 | 13 | export const WRITE_DEADLINE_MS = 60_000; | |
| 14 | 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 | + | ||
| 15 | 70 | /** What a call past its deadline throws. */ | |
| 16 | 71 | export class DeadlineExceeded extends Error { | |
| 17 | 72 | constructor(what: string, ms: number) { |
| 18 | 18 | sessionFor, | |
| 19 | 19 | SESSION_SERVICES, | |
| 20 | 20 | } from "./perf"; | |
| 21 | − | import { READ_DEADLINE_MS, WRITE_DEADLINE_MS, withDeadline } from "./deadline"; | |
| 21 | + | import { deadlineFor, withDeadline } from "./deadline"; | |
| 22 | 22 | ||
| 23 | 23 | /** | |
| 24 | 24 | * What one request to the site did: when it started, each service call | |
| ⋯ | |||
| 116 | 116 | * body here costs nothing and puts it under the deadline too. | |
| 117 | 117 | */ | |
| 118 | 118 | async function answerInTime(binding: ServiceBinding, name: string, method: string, input: string, init?: RequestInit): Promise<Response> { | |
| 119 | − | const ms = mayWrite(method) ? WRITE_DEADLINE_MS : READ_DEADLINE_MS; | |
| 119 | + | const ms = deadlineFor(name, method, mayWrite(method)); | |
| 120 | 120 | return withDeadline( | |
| 121 | 121 | `${name} ${method || input}`, | |
| 122 | 122 | ms, | |
| 3 | 3 | ||
| 4 | 4 | /** | |
| 5 | 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 a service read may take on | |
| 7 | − | * the server (lib/deadline.ts `READ_DEADLINE_MS`), so a slow service shows | |
| 8 | − | * its error page first; this catches what the server cannot: a request lost | |
| 9 | − | * on the way, a connection that went quiet, a route's code that never | |
| 10 | − | * arrived. | |
| 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 | 11 | */ | |
| 12 | − | export const STUCK_NAVIGATION_MS = 20_000; | |
| 12 | + | export const STUCK_NAVIGATION_MS = 75_000; | |
| 13 | 13 | ||
| 14 | 14 | /** | |
| 15 | 15 | * Where to send a navigation that has been loading too long, or null when |