steid

@jamesgill /

refactor: one place decides that git ran out of time

`refs`, `commit` and `blame` each carried the same `is_timeout()` match, written
three times inside one wave because the five branches could not see each other.
The predicate moves to `web::context::timed_out` and each page keeps its own
wording, which is the part that should differ — a branches list, a diff and a
blame all offer something different instead.

`search_repo` stays as it is and the doc comment says why: it answers
`Searched::TimedOut` in the application layer, where a timeout is one of several
results the same form re-renders rather than an error to catch.

Blame's panel is renamed `took_too_long` to match the commit page's, freeing the
name the predicate now holds.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J18ViwAfdswUCMb2DXJZFG
JamesPatrickGill authored 14 hours agoparent8804ad9Browse files00d9c061fd2a562b42ac9d529a51fd7471292cfe

5 files changed+33 −27

plans/current.md+5 −4View file
@@ -154,8 +154,8 @@ instance, and Steid's own source is pushed to it and browsable there.
154154 - **A repository with tags but no branches** says "No branches"; real but exotic, not
155155 fixture-tested.
156156 - **`web::browse::{encode, timestamp}` are now `pub(super)`**, and `repo_stats` takes
157 `handle` and `name` so the sidebar counts can link. Watch for the next page that wants
158 `timed_out` and move it to `web::context` then.
157+ `handle` and `name` so the sidebar counts can link. `timed_out` has since moved to
158+ `web::context`, as this said it should.
159159
160160 - **`git grep` output is collected whole before it is capped.** A one-letter query on a
161161 big repository allocates all of git's output to keep 200 hits. Bounded by the 20 s
@@ -170,8 +170,9 @@ instance, and Steid's own source is pushed to it and browsable there.
170170 line), and ignores `GrepHit::column` when marking.
171171
172172 - **The commit page's timeout state has never been rendered**, like search's and the
173 refs pages'. Three pages now each map `is_timeout()` to a panel of their own; the next
174 one should move the helper to `web::context`.
173+ refs pages'. The predicate itself is now `web::context::timed_out`, shared by `refs`,
174+ `commit` and `blame`; each page still writes its own panel, and `search_repo` still
175+ answers in the application layer.
175176 - **`Diff::files` is emptied wholesale when the patch is truncated**, falling back to the
176177 numstat list, so a 10 MiB commit shows no lines rather than the first few files.
177178 numstat itself could in principle be truncated at ~300,000 changed files.
src/infrastructure/web/blame.rs+5 −7View file
@@ -20,9 +20,7 @@ use topcoat::{
2020 };
2121
2222 use crate::{
23 application::{
24 Blame, BlameContent, BlameFile, BlameGroup, Error, blame::AGE_STEPS, blame_file,
25 },
23+ application::{Blame, BlameContent, BlameFile, BlameGroup, blame::AGE_STEPS, blame_file},
2624 domain::{ObjectId, RefName, RepoPath},
2725 };
2826
@@ -30,7 +28,7 @@ use super::{
3028 browse::{
3129 Switch, ago, crumbs, encode, raw_url, ref_links, refs_for, rev_switcher, size_of, tree_url,
3230 },
33 context::{current_actor, memberships, orgs, queries, repos, server_error},
31+ context::{current_actor, memberships, orgs, queries, repos, server_error, timed_out},
3432 layout::wide,
3533 repo::{Tab, repo_for, repo_header},
3634 };
@@ -93,7 +91,7 @@ async fn blaming(cx: &Cx, rev: RefName, path: RepoPath) -> Result {
9391 let file: Option<BlameFile> = match outcome {
9492 Ok(Some(file)) => Some(file),
9593 Ok(None) => return Err(not_found().into()),
96 Err(Error::GitQuery(error)) if error.is_timeout() => None,
94+ Err(error) if timed_out(&error) => None,
9795 Err(other) => return Err(server_error(std::io::Error::other(other.to_string()))),
9896 };
9997
@@ -172,7 +170,7 @@ async fn blaming(cx: &Cx, rev: RefName, path: RepoPath) -> Result {
172170 Body::Empty => note(message: "This file is empty."),
173171 Body::Binary => note(message: "This file cannot be blamed as text."),
174172 Body::TooLarge(_) => note(message: oversized.as_str()),
175 Body::TimedOut => timed_out(blob: blob.as_str()),
173+ Body::TimedOut => took_too_long(blob: blob.as_str()),
176174 }
177175 </div>
178176 )
@@ -192,7 +190,7 @@ async fn note(message: &str) -> Result {
192190 /// It offers the file itself rather than a retry: the read will take just as long the
193191 /// second time, and the reader came here to see the file.
194192 #[component]
195async fn timed_out(blob: &str) -> Result {
193+async fn took_too_long(blob: &str) -> Result {
196194 view! {
197195 <div class="px-4 py-6 text-center">
198196 <p class="text-sm text-muted-foreground">
src/infrastructure/web/commit.rs+4 −4View file
@@ -33,8 +33,8 @@ use topcoat::{
3333
3434 use crate::{
3535 application::{
36 COMPARE_LOG_LIMIT, Compared, Comparison, Diff, DiffLine, Error, FileChange, FileDiff,
37 FileStat, LineKind, MAX_FILE_DIFF_LINES, RefList, RepoView, compare_revisions, list_refs,
36+ COMPARE_LOG_LIMIT, Compared, Comparison, Diff, DiffLine, FileChange, FileDiff, FileStat,
37+ LineKind, MAX_FILE_DIFF_LINES, RefList, RepoView, compare_revisions, list_refs,
3838 view_commit,
3939 },
4040 components::{
@@ -47,7 +47,7 @@ use crate::{
4747
4848 use super::{
4949 browse::{ago, commit_log, encode, timestamp, tree_url},
50 context::{current_actor, memberships, orgs, queries, repos, server_error},
50+ context::{current_actor, memberships, orgs, queries, repos, server_error, timed_out},
5151 layout::wide,
5252 repo::{Tab, repo_for, repo_header},
5353 };
@@ -711,7 +711,7 @@ fn unwrap_page<T>(loaded: crate::application::Result<Option<T>>) -> Result<Optio
711711 match loaded {
712712 Ok(Some(page)) => Ok(Some(page)),
713713 Ok(None) => Err(not_found().into()),
714 Err(Error::GitQuery(error)) if error.is_timeout() => {
714+ Err(error) if timed_out(&error) => {
715715 eprintln!("steid: {error}");
716716 Ok(None)
717717 }
src/infrastructure/web/context.rs+17 −1View file
@@ -20,7 +20,7 @@ use topcoat::{
2020 };
2121
2222 use crate::{
23 application::{AppConfig, Identity, describe_identity, is_claimed, resolve_actor},
23+ application::{AppConfig, Error, Identity, describe_identity, is_claimed, resolve_actor},
2424 domain::{Actor, SessionTokenHash},
2525 infrastructure::{
2626 git::{DiskGitArchive, DiskGitStorage, GitHttpBackend},
@@ -51,6 +51,22 @@ where
5151 internal_server_error(error).into()
5252 }
5353
54+/// Whether a use case failed because git was too slow rather than because git broke.
55+///
56+/// A timeout is the one read failure that is not a fault: the repository is fine and the
57+/// request asked more of it than one page's budget allows. A page that can say so
58+/// renders a state of its own — in its own words, offering whatever costs less — rather
59+/// than a 500 that claims the instance is broken.
60+///
61+/// Four pages wanted this within one wave (`refs`, `commit`, `blame`, and `search`
62+/// before it moved), which is what it took to earn a home here. `search_repo` is the
63+/// exception and stays as it is: it answers `Searched::TimedOut` in the application
64+/// layer, because there the timeout is one of several results the same form re-renders
65+/// rather than an error to be caught.
66+pub fn timed_out(error: &Error) -> bool {
67+ matches!(error, Error::GitQuery(query) if query.is_timeout())
68+}
69+
5470 pub fn orgs(cx: &Cx) -> SqliteOrgRepo {
5571 SqliteOrgRepo::new(pool(cx).clone())
5672 }
src/infrastructure/web/refs.rs+2 −11View file
@@ -22,14 +22,14 @@ use topcoat::{
2222 };
2323
2424 use crate::{
25 application::{Error, RefPage, RepoView, list_branches, list_tags},
25+ application::{RefPage, RepoView, list_branches, list_tags},
2626 components::badge::{BadgeVariant, badge},
2727 domain::{BranchRow, RefName, TagRow},
2828 };
2929
3030 use super::{
3131 browse::{ago, empty_repo, encode, log_url, timestamp, tree_url},
32 context::{current_actor, memberships, orgs, queries, repos, server_error},
32+ context::{current_actor, memberships, orgs, queries, repos, server_error, timed_out},
3333 layout::wide,
3434 repo::{Tab, clone_url_for, repo_for, repo_header},
3535 };
@@ -61,15 +61,6 @@ fn compare_url(handle: &str, name: &str, base: &RefName, head: &RefName) -> Stri
6161 )
6262 }
6363
64/// Whether a failure was git being too slow rather than git being broken.
65///
66/// A timeout is the one read failure that is not a fault: the repository is fine and the
67/// request asked more of it than one page's budget allows. It gets a page state so the
68/// visitor is told what happened, rather than a 500 that says the instance is broken.
69fn timed_out(error: &Error) -> bool {
70 matches!(error, Error::GitQuery(query) if query.is_timeout())
71}
72
7364 #[page("/{handle}/repos/{name}/branches")]
7465 async fn branches_page(cx: &Cx) -> Result {
7566 let repo = repo_for(cx).await?;