From b89676bf1bb596dd1e36bb253efe6ecc75165e1e Mon Sep 17 00:00:00 2001 From: wiiiimm Date: Wed, 9 Sep 2026 13:15:53 +0800 Subject: [PATCH 1/2] fix(github-prs): keep the pane readable when GitHub search is slow GitHub's search backend goes through slow spells and sheds the heaviest requests first, which makes this widget the first of the three to suffer. It reported that in two ways that were both wrong. The banner drew a raw HTML document. `graphql` only shortened GraphQL errors; a non-2xx never reached that branch, so the gateway's page went through whole and wrapped across four rows of the pane saying nothing a reader could act on. `refused` keeps the status and drops the document. A failure on page one was fatal while a failure on any later page was not - `order` is empty on the first round, so the pass returned Err instead of keeping what it had, and a widget with no earlier pass behind it drew "0 of 0 open". A refused round now asks again at half the page size, down to a floor of ten, and only for the errors a smaller page can change: a gateway giving up or a timeout, never bad credentials. Measured against the live API during a slow spell: every size from 25 up returned 502 at about 10.7s while 20 and below answered in three. An hour later, nothing changed at this end, 50 answered in five. The page size is not what decides it - search was slow across the board and the larger pages were simply first over whatever budget it was enforcing - so the default drops to 25 rather than to the floor, and the backoff carries the bad minutes. `limit` was also documented as "maximum pull requests retained", which it has never been: paging runs until every source is exhausted regardless. It is the page size, and it now says so. Closes OPS-84 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01RfDBpF4cTt2CeoTLxkKewS --- config.example.json | 4 +- widgets/src/widgets/github-prs/README.md | 11 +- widgets/src/widgets/github-prs/main.rs | 176 ++++++++++++++++++- widgets/src/widgets/github-prs/settings.json | 4 +- 4 files changed, 182 insertions(+), 13 deletions(-) diff --git a/config.example.json b/config.example.json index 239a4a0..fa9fc52 100644 --- a/config.example.json +++ b/config.example.json @@ -133,8 +133,8 @@ "authored": "is:open is:pr author:@me", "assigned": "is:open is:pr assignee:@me" }, - "_limit_comment": "Maximum pull requests retained after searches are combined.", - "limit": 50, + "_limit_comment": "How many results to ask GitHub for per page. Not a cap on what is shown: paging continues until every search is exhausted. A smaller page is slower to finish and far likelier to be served when GitHub search is having a slow minute; a round that is refused backs off to 10 on its own.", + "limit": 25, "_refresh_comment": "Seconds between GitHub pull-request refreshes.", "refresh": 60.0 }, diff --git a/widgets/src/widgets/github-prs/README.md b/widgets/src/widgets/github-prs/README.md index 5f18f8a..f945d7b 100644 --- a/widgets/src/widgets/github-prs/README.md +++ b/widgets/src/widgets/github-prs/README.md @@ -317,11 +317,20 @@ widget quietly ran on somebody else's credential. "authored": "is:open is:pr author:@me", "assigned": "is:open is:pr assignee:@me" }, - "limit": 50, + "limit": 25, "refresh": 60 } ``` +`limit` is the page size a search asks GitHub for, not a cap on what the +pane shows — paging runs until every source is exhausted either way. It +matters because GitHub's search backend goes through slow spells and sheds +the heaviest requests first: measured during one, every size from 25 up +returned 502 at about 10.7s while 20 and below answered in three, and an +hour later 50 answered in five with nothing changed at this end. So a round +that is refused asks again at half the size, down to a floor of ten, and the +pane says when it had to. + Leave `token` empty and the variable `token_env` names is read instead, defaulting to `GITHUB_TOKEN`. Its value is the variable's name, not a credential. Nothing here reaches into another widget's section. diff --git a/widgets/src/widgets/github-prs/main.rs b/widgets/src/widgets/github-prs/main.rs index afd0e18..ad747ee 100644 --- a/widgets/src/widgets/github-prs/main.rs +++ b/widgets/src/widgets/github-prs/main.rs @@ -129,6 +129,33 @@ struct Rate { limit: Option, } +/// What to say when the transport refused, rather than what it said. +/// +/// A gateway that gives up answers with an HTML page, and `post_json` hands +/// that back whole: `HTTP 502: 502 Bad Gateway ...`. +/// Every other error this widget draws is a sentence, and a pane is no +/// place for markup - the banner wrapped the nginx body across four rows +/// and said nothing a reader could act on. The status is the part that +/// carries meaning, so keep it and drop the document. +fn plain_refusal(said: &str) -> String { + let code = said + .strip_prefix("HTTP ") + .and_then(|rest| rest.split(':').next()) + .and_then(|c| c.trim().parse::<u16>().ok()); + match code { + // GitHub's search backend goes through slow spells and sheds the + // heaviest queries first; the widget is not broken and neither is + // the token, so the wording says which end gave up. + Some(c @ (502 | 503 | 504)) => { + format!("GitHub returned {} - the search was too slow to serve", c) + } + Some(c) => format!("GitHub returned {}", c), + // Not a status at all: a timeout or a curl failure, already a + // sentence, and short enough to draw. + None => said.chars().take(100).collect(), + } +} + fn graphql( query: &str, tok: &str, @@ -144,7 +171,8 @@ fn graphql( ], &body, 45, - )?; + ) + .map_err(|said| plain_refusal(&said))?; let data: serde_json::Value = serde_json::from_str(&out).map_err(|e| e.to_string())?; if let Some(first) = data["errors"].as_array().and_then(|a| a.first()) { return Err(first["message"] @@ -624,6 +652,60 @@ fn fetch_detail( Ok(()) } +/// The smallest page worth asking for before calling a round a failure. +/// +/// Below this the request count climbs faster than the odds of an answer, +/// and a search that will not serve ten per page is not having a slow +/// minute, it is down. +const PAGE_FLOOR: usize = 10; + +/// Whether a failed round is worth asking again, smaller. +/// +/// Only the gateway giving up. A GraphQL error - bad credentials, a query +/// GitHub will not accept - says the same thing however small the page is, +/// and retrying it twice only spends rate limit to reprint the message. +fn worth_retrying(said: &str) -> bool { + said.starts_with("GitHub returned 5") || said.contains("did not answer in") +} + +/// One round of paging, backing off the page size when GitHub refuses. +/// +/// Measured against the live API during a slow spell: every size from 25 +/// up returned 502 at about 10.7s while 20 and below answered in three, +/// and an hour later 50 answered in five. The page size is not what +/// decides it - search was slow across the board and the larger pages were +/// simply the first over whatever budget it was enforcing. So the answer +/// is not a smaller page everywhere, it is a smaller page for as long as +/// GitHub is refusing the big one - which is why the caller feeds the size +/// that worked back in rather than starting each round at the default. +/// Halving twice from there reaches the floor, so a bad minute costs two +/// extra requests once, not once per round. +fn fetch_round( + round: &[String], + cursors: &[Option<String>], + limit: usize, + tok: &str, +) -> Result<(serde_json::Value, usize), String> { + let mut size = limit.max(1); + loop { + match graphql(&list_query(round, size, cursors), tok, serde_json::json!({})) { + Ok(d) => return Ok((d, size)), + Err(said) => match smaller(size).filter(|_| worth_retrying(&said)) { + Some(next) => size = next, + None => return Err(said), + }, + } + } +} + +/// The next page size to try, or `None` at the floor. +/// +/// Its own function so the walk can be tested rather than reimplemented in +/// the test, which is a test of the arithmetic it copied. +fn smaller(size: usize) -> Option<usize> { + (size > PAGE_FLOOR).then(|| (size / 2).max(PAGE_FLOOR)) +} + /// How many open pull requests the pooled sources really cover, and whether /// that number is a floor rather than a count. /// @@ -723,6 +805,14 @@ fn fetch_list( // Why paging stopped early, when it did. Kept apart from `err` so a // partial list is not dressed up as a failed fetch. let mut deepened: Option<String> = None; + // The smallest page any round had to fall back to, when one did, and + // what every later round in this pass starts from. A shorter page is + // not a smaller answer - paging carries on either way - but going back + // to the full size each round would spend a refusal and ten seconds + // relearning the same thing on every one of fifty-odd rounds. It also + // reaches the banner: a pane that says nothing about it looks like a + // pane that simply got slower. + let mut served: Option<usize> = None; while !live.is_empty() { let round: Vec<String> = live.iter().map(|i| queries[*i].clone()).collect(); @@ -734,12 +824,13 @@ fn fetch_list( // and it is the per-node subqueries that cost it, not the depth. // Everything already pooled is real and stays on screen, and // `capped` below already says the total is a lower bound. - let d = match graphql( - &list_query(&round, limit, &round_cursors), - tok, - serde_json::json!({}), - ) { - Ok(d) => d, + let d = match fetch_round(&round, &round_cursors, served.unwrap_or(limit), tok) { + Ok((d, size)) => { + if size < limit { + served = served.map_or(Some(size), |had: usize| Some(had.min(size))); + } + d + } Err(said) if !order.is_empty() => { deepened = Some(said); break; @@ -858,6 +949,16 @@ fn fetch_list( format!("{} · {}", said, note) }; } + if let Some(size) = served { + // Said plainly, because the list is whole and the only thing + // that changed is how many rounds it took to get here. + let note = format!("GitHub refused {} per page; served {}", limit, size); + said = if said.is_empty() { + note + } else { + format!("{} · {}", said, note) + }; + } if deepened.is_some() { // Named as a ceiling rather than as the raw 502, because that // is what it is: GitHub stops serving these pages, the list is @@ -1001,7 +1102,7 @@ fn main() { tc::load_config("github_prs") }; let mut refresh = tc::poll_secs(tc::cfg_f64(&cfg, "refresh", 60.0), 60.0); - let limit = tc::cfg_usize(&cfg, "limit", 50); + let limit = tc::cfg_usize(&cfg, "limit", 25); let sources = sources_from(&cfg); let _ = SOURCES_REFILLED.set(sources_were_emptied(&cfg)); @@ -2524,6 +2625,65 @@ fn detail_view( mod tests { use super::*; + /// Word for word what `run_with_timeout` says when curl outlives its + /// deadline - the other way a slow search reaches this widget. + const TIMED_OUT: &str = "curl did not answer in 45s"; + + #[test] + fn a_gateway_page_never_reaches_the_screen() { + // What `post_json` hands back when nginx gives up, shortened from + // the real body. Left whole it wrapped across four rows of the + // pane and told the reader nothing. + let body = "HTTP 502: <html> <head><title>502 Bad Gateway \ +

502 Bad Gateway

\ +
nginx
"; + let said = plain_refusal(body); + assert!(!said.contains('<'), "markup reached the banner: {}", said); + assert!(said.contains("502"), "the status is the part worth keeping"); + assert!( + said.contains("too slow"), + "say which end gave up, not just the number: {}", + said + ); + + // A status with no meaning of its own still loses the document. + let said = plain_refusal("HTTP 418: teapot"); + assert_eq!(said, "GitHub returned 418"); + + // Not a status at all - curl failed, or the request timed out. + // Already a sentence, and it is kept. + assert_eq!(plain_refusal(TIMED_OUT), TIMED_OUT); + } + + #[test] + fn only_the_gateway_is_worth_asking_again() { + assert!(worth_retrying("GitHub returned 502 - the search was too slow to serve")); + assert!(worth_retrying("GitHub returned 504 - the search was too slow to serve")); + assert!(worth_retrying(TIMED_OUT)); + + // A smaller page does not change GitHub's mind about any of these, + // and asking twice more spends rate limit to reprint the message. + assert!(!worth_retrying("Bad credentials")); + assert!(!worth_retrying("GitHub returned 401")); + assert!(!worth_retrying("GitHub returned 403")); + assert!(!worth_retrying( + "Field 'stackEntry' doesn't exist on type 'PullRequest'" + )); + } + + #[test] + fn the_page_size_halves_to_a_floor_and_stops() { + // The sequence `fetch_round` walks from the shipped default. It + // stops at the floor rather than grinding down to one, because a + // search that will not serve ten per page is down, not slow. + let mut sizes = vec![25usize]; + while let Some(next) = smaller(*sizes.last().unwrap()) { + sizes.push(next); + } + assert_eq!(sizes, vec![25, 12, 10]); + assert_eq!(smaller(PAGE_FLOOR), None, "the floor is where it stops"); + } + #[test] fn a_search_that_cannot_be_paged_is_a_search_capped_at_one_page() { let qs = vec!["is:open is:pr".to_string(), "author:@me".to_string()]; diff --git a/widgets/src/widgets/github-prs/settings.json b/widgets/src/widgets/github-prs/settings.json index c6739ae..e1e6472 100644 --- a/widgets/src/widgets/github-prs/settings.json +++ b/widgets/src/widgets/github-prs/settings.json @@ -19,8 +19,8 @@ "authored": "is:open is:pr author:@me", "assigned": "is:open is:pr assignee:@me" }, - "_limit_comment": "Maximum pull requests retained after searches are combined.", - "limit": 50, + "_limit_comment": "How many results to ask GitHub for per page. Not a cap on what is shown: paging continues until every search is exhausted. A smaller page is slower to finish and far likelier to be served when GitHub search is having a slow minute; a round that is refused backs off to 10 on its own.", + "limit": 25, "_refresh_comment": "Seconds between GitHub pull-request refreshes.", "refresh": 60.0 } From 2893f8a084764067366971eecf0ec4e9145762db Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 9 Sep 2026 06:25:46 +0000 Subject: [PATCH 2/2] fix(github-prs): retry only gateways and real curl timeouts worth_retrying matched every 5xx and looked for run_full's "did not answer in" sentence, which post_json never produces. A timeout is curl exit 28; a 500 is not a slow search. Keep a 401/403/429 body's message so the pane still says why the token was refused, and align the README with the 25 default. Co-authored-by: wiiiimm --- widgets/src/widgets/github-prs/README.md | 12 +++-- widgets/src/widgets/github-prs/main.rs | 69 +++++++++++++++++++++--- 2 files changed, 69 insertions(+), 12 deletions(-) diff --git a/widgets/src/widgets/github-prs/README.md b/widgets/src/widgets/github-prs/README.md index f945d7b..bc57d90 100644 --- a/widgets/src/widgets/github-prs/README.md +++ b/widgets/src/widgets/github-prs/README.md @@ -133,10 +133,13 @@ it to code you have a stake in. `f` cycles which source is shown — `all`, then each by name. It is instant and costs no request, because the pooling already recorded the answer. -**Page size is 50 per source, and every source is paged to exhaustion.** +**Page size is 25 per source, and every source is paged to exhaustion.** Three searches of 100 return HTTP 502; three of 50 do not, so more results -come from more rounds and never from a bigger page. Each source carries its -own cursor and drops out of the round once GitHub says it has no next page. +come from more rounds and never from a bigger page. The shipped default is +25 because a slow spell sheds larger pages first; a gateway 502/503/504 +or a curl timeout asks again at half the size, down to ten. Each source +carries its own cursor and drops out of the round once GitHub says it has +no next page. Rows are published as each round lands, so the board fills while it works rather than staying empty until the last source is done, and the count in the header is the count on screen throughout. @@ -328,7 +331,8 @@ matters because GitHub's search backend goes through slow spells and sheds the heaviest requests first: measured during one, every size from 25 up returned 502 at about 10.7s while 20 and below answered in three, and an hour later 50 answered in five with nothing changed at this end. So a round -that is refused asks again at half the size, down to a floor of ten, and the +that is refused — a gateway 502, 503 or 504, or curl running out of its +45 seconds — asks again at half the size, down to a floor of ten, and the pane says when it had to. Leave `token` empty and the variable `token_env` names is read instead, diff --git a/widgets/src/widgets/github-prs/main.rs b/widgets/src/widgets/github-prs/main.rs index ad747ee..f02a296 100644 --- a/widgets/src/widgets/github-prs/main.rs +++ b/widgets/src/widgets/github-prs/main.rs @@ -149,13 +149,43 @@ fn plain_refusal(said: &str) -> String { Some(c @ (502 | 503 | 504)) => { format!("GitHub returned {} - the search was too slow to serve", c) } - Some(c) => format!("GitHub returned {}", c), + Some(c) => match short_explain(said, c) { + Some(why) => format!("GitHub returned {}: {}", c, why), + None => format!("GitHub returned {}", c), + }, // Not a status at all: a timeout or a curl failure, already a // sentence, and short enough to draw. None => said.chars().take(100).collect(), } } +/// A non-gateway body's useful words, when it has any. +/// +/// Gateway HTML is dropped above. A 401, 403 or 429 usually carries the +/// reason in JSON (`Bad credentials`, a missing scope, the rate-limit +/// message) and that is the sentence the pane should keep. Markup, or an +/// empty body, leaves only the status. +fn short_explain(said: &str, code: u16) -> Option { + let rest = said + .strip_prefix("HTTP ") + .and_then(|s| s.strip_prefix(&code.to_string())) + .unwrap_or("") + .trim_start_matches(':') + .trim(); + if rest.is_empty() || rest.starts_with('<') { + return None; + } + if let Ok(v) = serde_json::from_str::(rest) { + if let Some(m) = v.get("message").and_then(|m| m.as_str()) { + let m = m.trim(); + if !m.is_empty() { + return Some(m.chars().take(80).collect()); + } + } + } + Some(rest.chars().take(80).collect()) +} + fn graphql( query: &str, tok: &str, @@ -661,11 +691,18 @@ const PAGE_FLOOR: usize = 10; /// Whether a failed round is worth asking again, smaller. /// -/// Only the gateway giving up. A GraphQL error - bad credentials, a query -/// GitHub will not accept - says the same thing however small the page is, -/// and retrying it twice only spends rate limit to reprint the message. +/// Only a gateway giving up, or curl hitting `--max-time`. A GraphQL +/// error - bad credentials, a query GitHub will not accept - says the +/// same thing however small the page is, and retrying it twice only +/// spends rate limit to reprint the message. A 500 is the same: the +/// search backend is not shedding load, it is broken, and a smaller +/// page will not change its mind. fn worth_retrying(said: &str) -> bool { - said.starts_with("GitHub returned 5") || said.contains("did not answer in") + said.starts_with("GitHub returned 502") + || said.starts_with("GitHub returned 503") + || said.starts_with("GitHub returned 504") + || said.contains("curl: (28)") + || said.contains("curl exited 28") } /// One round of paging, backing off the page size when GitHub refuses. @@ -2625,9 +2662,11 @@ fn detail_view( mod tests { use super::*; - /// Word for word what `run_with_timeout` says when curl outlives its - /// deadline - the other way a slow search reaches this widget. - const TIMED_OUT: &str = "curl did not answer in 45s"; + /// Word for word what `post_json` hands back when curl hits `--max-time`. + /// Exit 28 is curl's timeout; `run_full`'s "did not answer in Ns" is a + /// different helper and never sits on this path. + const TIMED_OUT: &str = + "curl: (28) Operation timed out after 45000 milliseconds with 0 bytes received"; #[test] fn a_gateway_page_never_reaches_the_screen() { @@ -2650,22 +2689,36 @@ mod tests { let said = plain_refusal("HTTP 418: teapot"); assert_eq!(said, "GitHub returned 418"); + // A 401's body is the reason the token was refused. Dropping it + // left the pane saying only the number. + let said = plain_refusal(r#"HTTP 401: {"message":"Bad credentials"}"#); + assert_eq!(said, "GitHub returned 401: Bad credentials"); + // Not a status at all - curl failed, or the request timed out. // Already a sentence, and it is kept. assert_eq!(plain_refusal(TIMED_OUT), TIMED_OUT); + assert!( + worth_retrying(&plain_refusal(TIMED_OUT)), + "a timeout has to stay retryable after it is shortened" + ); } #[test] fn only_the_gateway_is_worth_asking_again() { assert!(worth_retrying("GitHub returned 502 - the search was too slow to serve")); + assert!(worth_retrying("GitHub returned 503 - the search was too slow to serve")); assert!(worth_retrying("GitHub returned 504 - the search was too slow to serve")); assert!(worth_retrying(TIMED_OUT)); + assert!(worth_retrying("curl exited 28")); // A smaller page does not change GitHub's mind about any of these, // and asking twice more spends rate limit to reprint the message. assert!(!worth_retrying("Bad credentials")); assert!(!worth_retrying("GitHub returned 401")); assert!(!worth_retrying("GitHub returned 403")); + assert!(!worth_retrying("GitHub returned 500")); + assert!(!worth_retrying("GitHub returned 501")); + assert!(!worth_retrying("GitHub returned 505")); assert!(!worth_retrying( "Field 'stackEntry' doesn't exist on type 'PullRequest'" ));