Side A fixes a real production bug (cloud IPs hitting Reddit's block page), adds a proper OAuth-required retry/refresh path, error truncation, and a test, providing substantial lasting infrastructure value. Side B is a small, correct bugfix moving a guard before side effects, valuable but much narrower in scope and impact than A's change.
constitution · epochs · watch · epoch 3
comparison
c_ca9169f732b8 (tommy-mor) vs c_abd68b5e771a (tommy-mor)
download prompt · raw event · cmp_cfe3f883f252d7
council reasoning
Commit A fixes a production-blocking path: when OAuth creds exist it never falls back to www.reddit.com (which returns network-security 403 HTML from cloud IPs), adds AuthRejected + forced token refresh/retry, and tightens error reporting—real lasting API reliability. Commit B is a correct but tiny guard reordering so zero-ratio votes skip ensure_item/voted_pairs; valuable data-integrity hygiene, yet far narrower in scope than A’s OAuth redesign.
Side A fixes a production-facing reliability issue by requiring OAuth when credentials exist, retrying once with a refreshed token on 401/403, propagating token acquisition errors instead of silently falling back, and improving error handling with truncated diagnostics and a regression test. Side B corrects an ordering bug by moving the zero-ratio early return before `ensure_item` and `voted_pairs.insert`, preventing ghost state, but it is a smaller, localized fix with narrower impact.
sides
A — c_ca9169f732b8 (tommy-mor)
message
[8f69c309] Require Reddit OAuth when credentials are set and refresh on 401/403. Avoid falling back to the public www.reddit.com API from cloud IPs, which returns Reddit's network-security block page. Also pin SORTER2_BASE_URL in fly.toml. Co-authored-by: Cursor <cursoragent@cursor.com>
diff preview
diff --git a/fly.toml b/fly.toml
index f0c6a39f643c204987debc177234d95a8ef44b65..ca7e0088a7efd7d58d29d808f8d89080b2bff233 100644
--- a/fly.toml
+++ b/fly.toml
@@ -5,6 +5,7 @@ primary_region = "iad"
dockerfile = "Dockerfile"
[env]
+ SORTER2_BASE_URL = "https://reddit.sorter.social"
SORTER2_DATA_DIR = "/data"
SORTER2_EVENT_LOG = "/data/events.jsonl"
PORT = "8080"
diff --git a/server/src/reddit.rs b/server/src/reddit.rs
index a874814f8927192ee62cab2d0db1efd27dcd57b7..f409764c1e1f36216f1b08107043c2eab905694c 100644
--- a/server/src/reddit.rs
+++ b/server/src/reddit.rs
@@ -283,26 +283,35 @@ async fn reddit_worker(
);
tokio::time::sleep(current_delay).await;
- if let Some(c) = &creds {
- oauth = ensure_oauth_token(&client, &oauth_token_base, c, oauth.take()).await;
- }
-
- let token = oauth.as_ref().map(|t| t.access_token.as_str());
- let fetch_base = if token.is_some() {
- tracing::debug!(
- item = %fetch_id,
- base = %oauth_api_base,
- "reddit fetch using OAuth bearer"
- );
- &oauth_api_base
- } else {
- &api_base
- };
- let url = match kind {
- FetchKind::SelfEntity => map_item_to_reddit_api(&fetch_id, fetch_base),
- FetchKind::Children => map_children_url(&fetch_id, fetch_base),
+ let outcome = match &creds {
+ Some(c) => {
+ // OAuth is required when credentials are configured — never fall
+ // back to the public www.reddit.com JSON endpoints (cloud IPs
+ // get blocked with a 403 HTML interstitial).
+ fetch_with_oauth(
+ &client,
+ &oauth_token_base,
+ &oauth_api_base,
+ c,
+ &mut oauth,
+ &fetch_id,
+ kind,
+ )
+ .await
+ }
+ None => {
+ let url = match kind {
+ FetchKind::SelfEntity => map_item_to_reddit_api(&fetch_id, &api_base),
+ FetchKind::Children => map_children_url(&fetch_id, &api_base),
+ };
+ match do_fetch(&client, &url, &fetch_id, None).await {
+ Ok(FetchOutcome::AuthRejected { status, detail }) => {
+ Err(format!("Reddit API {status}: {detail}"))
+ }
+ other => other,
+ }
+ }
};
- let outcome = do_fetch(&client, &url, &fetch_id, token).await;
match outcome {
Ok(FetchOutcome::Payload(payload)) => {
@@ -342,6 +351,12 @@ async fn reddit_worker(
current_delay = (current_delay * 2).min(Duration::from_secs(60));
notify(done, FetchJobResult::RateLimited { reset_secs });
}
+ Ok(FetchOutcome::AuthRejected { status, detail }) => {
+ let e = format!("Reddit API {status}: {detail}");
+ tracing::warn!(item = %fetch_id, err = %e, "reddit fetch auth rejected");
+ current_delay = (current_delay * 2).min(Duration::from_secs(60));
+ notify(done, FetchJobResult::Failed(e));
+ }
Err(e) => {
tracing::warn!(item = %fetch_id, err = %e, "reddit fetch failed");
current_delay = (current_delay * 2).min(Duration::from_secs(60));
@@ -357,6 +372,60 @@ enum FetchOutcome {
Payload(Value),
NotFound,
RateLimited { reset_secs: u64 },
+ /// Bearer rejected — caller should drop the cached token and retry once.
+ AuthRejected { status: StatusCode, detail: String },
+}
+
+async fn fetch_with_oauth(
+ client: &Client,
+ oauth_token_base: &str,
+ oauth_api_base: &str,
+ creds: &RedditCredentials,
+ oauth: &mut Option<OAuthToken>,
+ fetch_id: &ItemId,
+ kind: FetchKind,
+) -> Result<FetchOutcome, String> {
+ for attempt in 0..2 {
+ let force_refresh = attempt > 0;
+ *oauth = Some(
+ ensure_oauth_token(client, oauth_token_base, creds, oauth.take(), force_refresh)
+ .await?,
+ );
+ let token = oauth
+ .as_ref()
+ .expect("token set above")
+ .access_token
+ .clone();
+
+ tracing::debug!(
+ item = %fetch_id,
+ base = %oauth_api_base,
+ attempt,
+ "reddit fetch using OAuth bearer"
+ );
+
+ let url = match kind {
+ FetchKind::SelfEntity => map_item_to_reddit_api(fetch_id, oauth_api_base),
+ FetchKind::Children => map_children_url(fetch_id, oauth_api_base),
+ };
+ match do_fetch(client, &url, fetch_id, Some(&token)).await? {
+ FetchOutcome::AuthRejected { status, detail } if attempt == 0 => {
+ tracing::warn!(
+ item = %fetch_id,
+ %status,
+ %detail,
+ "reddit OAuth rejected; refreshing token and retrying"
+ );
+ *oauth = None;
+ continue;
+ }
+ FetchOutcome::AuthRejected { status, detail } => {
+ return Err(format!("Reddit API {status}: {detail}"));
+ }
+ other => return Ok(other),
+ }
+ }
+ unreachable!("loop always returns")
}
async fn ensure_oauth_token(
@@ -364,35 +433,35 @@ async fn ensure_oauth_token(
oauth_base: &str,
creds: &RedditCredentials,
existing: Option<OAuthToken>,
-) -> Option<OAuthToken> {
- if let Some(t) = existing {
- if Instant::now() < t.expires_at - Duration::from_secs(60) {
- tracing::debug!("reddit OAuth token still valid");
- return Some(t);
+ force_refresh: bool,
+) -> Result<OAuthToken, String> {
+ if !force_refresh {
+ if let Some(t) = existing {
+ if Instant::now() < t.expires_at - Duration::from_secs(60) {
+ tracing::debug!("reddit OAuth token still valid");
+ return Ok(t);
+ }
}
}
let url = format!("{}/api/v1/access_token", oauth_base.trim_end_matches('/'));
- tracing::debug!(%url, "reddit OAuth token request");
+ tracing::debug!(%url, force_refresh, "reddit OAuth token request");
let resp = client
.post(&url)
.basic_auth(&creds.client_id, Some(&creds.client_secret))
.form(&[("grant_type", "client_credentials")])
.send()
- .await;
-
- let resp = match resp {
- Ok(r) => r,
- Err(e) => {
- tracing::warn!("reddit OAuth token request failed: {e}");
- return None;
- }
- };
+ .await
+ .map_err(|e| format!("Reddit OAuth token request failed: {e}"))?;
if !resp.status().is_success() {
- tracing::warn!("reddit OAuth token HTTP {}", resp.status());
- return None;
+ let status = resp.status();
+ let body = resp.text().await.unwrap_or_default();
+ return Err(format!(
+ "Reddit OAuth token HTTP {status}: {}",
+ truncate_for_error(&body)
+ ));
}
#[derive(Deserialize)]
@@ -401,21 +470,40 @@ async fn ensure_oauth_token(
expires_in: u64,
}
- let body: TokenResponse = match resp.json().await {
- Ok(b) => b,
- Err(e) => {
- tracing::warn!("reddit OAuth token parse failed: {e}");
- return None;
- }
- };
+ let body: TokenResponse = resp
+ .json()
+ .await
+ .map_err(|e| format!("Reddit OAuth token parse failed: {e}"))?;
- tracing::debug!(expires_in = body.expires_in, "reddit OAuth token acquired");
- Some(OAuthToken {
+ tracing::info!(expires_in = body.expires_in, "reddit OAuth token acquired");
+ Ok(OAuthToken {
access_token: body.access_token,
expires_at: Instant::now() + Duration::from_secs(body.expires_in),
})
}
+fn truncate_for_error(body: &str) -> String {
+ let compact: String = body.split_whitespace().collect::<Vec<_>>().join(" ");
+ if compact.is_empty() {
+ return "(empty body)".into();
+ }
+ // Prefer the human-readable block message over dumping Reddit's CSS.
+ if let Some(idx) = compact.find("You've been blocked") {
+ let slice: String = compact.chars().skip(idx).take(160).collect();
+ return if compact.chars().count() > idx + 160 {
+ format!("{slice}…")
+ } else {
+ slice
+ };
+ }
+ let chars: String = compact.chars().take(200).collect();
+ if compact.chars().count() > 200 {
+ format!("{chars}…")
+ } else {
+ chars
+ }
+}
+
async fn do_fetch(
client: &Client,
url: &str,
@@ -460,15 +548,16 @@ async fn do_fetch(
if !status.is_success() {
let body = resp.text().await.unwrap_or_default();
+ let detail = truncate_for_error(&body);
tracing::debug!(
item = %id,
%status,
body_len = body.len(),
- body_prefix = %body.chars().take(240).collect::<String>(),
+ %detail,
"reddit non-success body"
);
if status == StatusCode::FORBIDDEN || status == StatusCode::UNAUTHORIZED {
- return Err(format!("Reddit API {status}: {body}"));
+ return Ok(FetchOutcome::AuthRejected { status, detail });
}
return Ok(FetchOutcome::NotFound);
}
@@ -716,6 +805,15 @@ fn reddit_direct_image_url(url: &str) -> bool {
mod tests {
use super::*;
+ #[test]
+ fn truncate_error_prefers_block_message() {
+ let html = r#"<style>.x{color:red}</style><div>You've been blocked by network security. To continue, log in</div>"#;
+ let msg = truncate_for_error(html);
+ assert!(msg.starts_with("You've been blocked"));
+ assert!(msg.len() < 200);
+ assert!(!msg.contains(".x{color"));
+ }
+
#[test]
fn map_subreddit_about_url() {
let id = ItemId::from_url("https://reddit.com/r/rust").unwrap();
B — c_abd68b5e771a (tommy-mor)
message
[81de487b] Fix zero-ratio guard in reducer to drop before registering items or pair. Previously the early-return for zero-weight votes happened after ensure_item and voted_pairs.insert, leaving ghost items in the index and the pair incorrectly marked as voted. Move the check to before any side effects. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
diff preview
diff --git a/server/src/reducer.rs b/server/src/reducer.rs
index 6841d35cfc9de2389f340a22b8a45acb335e36c3..0e36979abe0f051493038ff7e652efc7f7a0ac80 100644
--- a/server/src/reducer.rs
+++ b/server/src/reducer.rs
@@ -112,6 +112,10 @@ impl GroupState {
if vote.ratio_right < 0 {
vote.ratio_right = 0;
}
+ if vote.ratio_left == 0 || vote.ratio_right == 0 {
+ // Zero on either side produces no valid edge; drop before registering items or pair.
+ return;
+ }
let a_idx = self.ensure_item(&vote.a);
let b_idx = self.ensure_item(&vote.b);
@@ -121,10 +125,6 @@ impl GroupState {
let w_a = vote.ratio_left as f64;
let w_b = vote.ratio_right as f64;
- if w_a == 0.0 || w_b == 0.0 {
- // Zero on either side produces no valid edge; drop the vote.
- return;
- }
self.add_edge_weight(b_idx, a_idx, w_a);
self.add_edge_weight(a_idx, b_idx, w_b);
diff --git a/server/tests/basic.rs b/server/tests/basic.rs
index cc8c1a0d139f3722ba6ecd13dd001c65be835b67..08159f4a7f0850fd165817a4a1af4f31ced2ad76 100644
--- a/server/tests/basic.rs
+++ b/server/tests/basic.rs
@@ -546,12 +546,10 @@ fn reducer_negative_ratio_clamped_to_zero() {
delegate: Some("00000000-0000-0000-0000-000000000000:test:local/test".to_string()),
thread_tag: "t".to_string(),
});
- // Items are registered, but the zero-clamped vote produces no edges.
- assert_eq!(group.idx_to_item.len(), 2);
- let a_idx = group.item_to_idx[&item_id("https://slug.social/~/t/a")];
- let b_idx = group.item_to_idx[&item_id("https://slug.social/~/t/b")];
- assert!(!group.edges.contains_key(&(a_idx, b_idx)));
- assert!(!group.edges.contains_key(&(b_idx, a_idx)));
+ // Nothing registered: zero-clamped vote is dropped before ensure_item.
+ assert!(group.idx_to_item.is_empty());
+ assert!(group.edges.is_empty());
+ assert!(group.voted_pairs.is_empty());
}
Hardlinks — judgments / attempts / prompt
judgments
attempts
Prompt text is loaded only by the download route.