Skip to content

Commit 3e5db9f

Browse files
authored
[pre-push] Clean up GraphQL backoff code (#289)
gherrit-pr-id: Gb586e728a212776be323db7d30e4a55851fa1208
1 parent 86b81a9 commit 3e5db9f

File tree

1 file changed

+66
-58
lines changed

1 file changed

+66
-58
lines changed

src/pre_push.rs

Lines changed: 66 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use std::{
2+
cmp,
23
collections::HashMap,
34
fmt::{self, Write},
45
process::Stdio,
@@ -1035,10 +1036,10 @@ where
10351036
//
10361037
// [1] https://docs.github.com/en/graphql/overview/rate-limits-and-query-limits-for-the-graphql-api#node-limit
10371038
// [2] https://github.blog/changelog/2025-09-01-graphql-api-resource-limits/
1038-
let mut batch_size = 64;
1039+
let mut batch_size = cmp::min(64, items.len());
10391040
let mut cursor = 0;
10401041
while cursor < items.len() {
1041-
let end = std::cmp::min(cursor + batch_size, items.len());
1042+
let end = cmp::min(cursor + batch_size, items.len());
10421043
let chunk = &items[cursor..end];
10431044

10441045
let query_body: String = chunk
@@ -1055,68 +1056,75 @@ where
10551056
}
10561057
);
10571058

1058-
// HEURISTIC: Check query size before sending. GitHub's WAF/load
1059-
// balancer/some other middleware seems to silently drop or truncate
1060-
// requests larger than ~600KB, leading to confusing "missing query
1061-
// attribute" errors. We preemptively backoff if we exceed a conservative
1062-
// limit (256KB).
1063-
let query_size = query.len();
1064-
const MAX_QUERY_SIZE: usize = 256 * 1024; // 256 KB
1059+
// Attempt to perform the query. Returns:
1060+
// - Ok(Some(response)): Success
1061+
// - Ok(None): Heuristic or API limit hit (needs backoff)
1062+
// - Err(e): Fatal error (bail)
1063+
let response = async {
1064+
// HEURISTIC: Check query size before sending. GitHub's WAF/load
1065+
// balancer/some other middleware seems to silently drop or truncate
1066+
// requests larger than ~600KB, leading to confusing "missing query
1067+
// attribute" errors. We preemptively backoff if we exceed a
1068+
// conservative limit (256KB).
1069+
const MAX_QUERY_SIZE: usize = 256 * 1024; // 256 KB
1070+
if query.len() > MAX_QUERY_SIZE {
1071+
log::warn!(
1072+
"GraphQL query size ({} bytes) exceeds heuristic limit ({} bytes).",
1073+
query.len(),
1074+
MAX_QUERY_SIZE
1075+
);
1076+
return Ok(None);
1077+
}
1078+
1079+
log::trace!("Sending GraphQL Query (Length: {}): {}", query.len(), query);
1080+
let request_payload = json!({ "query": query });
1081+
let response: serde_json::Value = octocrab
1082+
.graphql(&request_payload)
1083+
.await
1084+
.wrap_err("GraphQL batched operation failed")?;
1085+
1086+
if let Some(errors) = response.get("errors") {
1087+
let is_resource_limit = errors.as_array().is_some_and(|errs| {
1088+
errs.iter().any(|e| {
1089+
let type_str = e.get("type").and_then(|t| t.as_str());
1090+
let msg_str = e.get("message").and_then(|m| m.as_str());
1091+
1092+
matches!(type_str, Some("RESOURCE_LIMITS_EXCEEDED" | "MAX_NODE_LIMIT_EXCEEDED"))
1093+
// HEURISTIC: We've seen this specific error message in
1094+
// practice, likely due to the middleware issue
1095+
// described above. We assume it's as a result of an
1096+
// overly-large query.
1097+
|| matches!(msg_str, Some("A query attribute must be specified and must be a string."))
1098+
})
1099+
});
1100+
1101+
if is_resource_limit {
1102+
log::warn!("Hit GitHub resource limit with GraphQL batch update of size {batch_size}");
1103+
return Ok(None);
1104+
}
10651105

1066-
if query_size > MAX_QUERY_SIZE {
1106+
log::error!("GraphQL errors: {}", errors);
1107+
bail!("GraphQL errors: {:?}", errors);
1108+
}
1109+
1110+
Ok(Some(response))
1111+
}.await?;
1112+
1113+
let Some(response) = response else {
10671114
let new_batch_size = batch_size / 2;
1068-
log::warn!(
1069-
"GraphQL query size ({query_size} bytes) exceeds heuristic limit ({MAX_QUERY_SIZE} bytes). Backing off batch size from {batch_size} to {new_batch_size}.",
1070-
);
1071-
// FIXME: In this case, fall back to the REST API.
1115+
log::warn!("Backing off batch size from {batch_size} to {new_batch_size}.");
1116+
10721117
if new_batch_size == 0 {
1073-
bail!("Single PR update exceeds query size limit. Cannot sync.");
1118+
// NOTE: Since we always divide by 2, we're guaranteed to never
1119+
// skip 1 (4 -> 2 -> 1 and 3 -> 1), so we know that if we reach
1120+
// this point, a single PR update exceeds GitHub's limits.
1121+
//
1122+
// FIXME: In this case, fall back to the REST API.
1123+
bail!("Single PR update exceeds GitHub resource limits. Cannot sync.");
10741124
}
10751125
batch_size = new_batch_size;
10761126
continue;
1077-
}
1078-
1079-
log::trace!("Sending GraphQL Query (Length: {}): {}", query.len(), query);
1080-
1081-
let request_payload = json!({ "query": query });
1082-
let response: serde_json::Value = octocrab
1083-
.graphql(&request_payload)
1084-
.await
1085-
.wrap_err("GraphQL batched operation failed")?;
1086-
1087-
if let Some(errors) = response.get("errors") {
1088-
let is_resource_limit = errors.as_array().is_some_and(|errs| {
1089-
errs.iter().any(|e| {
1090-
let type_str = e.get("type").and_then(|t| t.as_str());
1091-
let msg_str = e.get("message").and_then(|m| m.as_str());
1092-
1093-
matches!(type_str, Some("RESOURCE_LIMITS_EXCEEDED" | "MAX_NODE_LIMIT_EXCEEDED"))
1094-
// HEURISTIC: We've seen this specific error message in
1095-
// practice, likely due to the middleware issue described
1096-
// above. We assume it's as a result of an overly-large
1097-
// query.
1098-
|| msg_str
1099-
== Some("A query attribute must be specified and must be a string.")
1100-
})
1101-
});
1102-
if is_resource_limit {
1103-
let new_batch_size = batch_size / 2;
1104-
log::warn!(
1105-
"Hit GitHub resource limit with GraphQL batch update of size {}. Backing off to {}.",
1106-
batch_size,
1107-
new_batch_size
1108-
);
1109-
// FIXME: In this case, fall back to the REST API.
1110-
if new_batch_size == 0 {
1111-
bail!("Single PR update exceeds GitHub resource limits. Cannot sync.");
1112-
}
1113-
batch_size = new_batch_size;
1114-
continue;
1115-
}
1116-
1117-
log::error!("GraphQL errors: {}", errors);
1118-
bail!("GraphQL errors: {:?}", errors);
1119-
}
1127+
};
11201128

11211129
let data = json_get!(response["data"])?;
11221130

0 commit comments

Comments
 (0)