Merge pull request #2866 from LemmyNet/remove-reqwest-retry

Optimize fetching of post url metadata

Cargo.lock

@@ -2771,7 +2771,6 @@ dependencies = [
  "pict-rs",
  "reqwest",
  "reqwest-middleware",
- "reqwest-retry",
  "reqwest-tracing",
  "serde",
  "serde_json",
@@ -4331,26 +4330,6 @@ dependencies = [
  "thiserror",
 ]
 
-[[package]]
-name = "reqwest-retry"
-version = "0.2.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "e29d842a94e8ab9b581fd3b906053872aef2fb3e474cbd88712047895d2deee4"
-dependencies = [
- "anyhow",
- "async-trait",
- "chrono",
- "futures",
- "http",
- "hyper",
- "reqwest",
- "reqwest-middleware",
- "retry-policies",
- "task-local-extensions",
- "tokio",
- "tracing",
-]
-
 [[package]]
 name = "reqwest-tracing"
 version = "0.4.0"
@@ -4372,17 +4351,6 @@ version = "0.1.9"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "4389f1d5789befaf6029ebd9f7dac4af7f7e3d61b69d4f30e2ac02b57e7712b0"
 
-[[package]]
-name = "retry-policies"
-version = "0.1.1"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "47f9e19b18c6cdd796cc70aea8a9ea5ee7b813be611c6589e3624fcdbfd05f9d"
-dependencies = [
- "anyhow",
- "chrono",
- "rand 0.8.5",
-]
-
 [[package]]
 name = "rgb"
 version = "0.8.34"

Cargo.toml

@@ -78,7 +78,6 @@ reqwest-middleware = "0.2.0"
 reqwest-tracing = "0.4.0"
 clokwerk = "0.3.5"
 doku = { version = "0.20.0", features = ["url-2"] }
-reqwest-retry = "0.2.0"
 bcrypt = "0.13.0"
 chrono = { version = "0.4.22", features = ["serde"], default-features = false }
 serde_json = { version = "1.0.87", features = ["preserve_order"] }
@@ -134,7 +133,6 @@ reqwest-middleware = { workspace = true }
 reqwest-tracing = { workspace = true }
 clokwerk = { workspace = true }
 doku = { workspace = true }
-reqwest-retry = { workspace = true }
 serde_json = { workspace = true }
 futures = { workspace = true }
 actix = { workspace = true }

crates/apub/src/objects/post.rs

@@ -185,6 +185,9 @@ impl Object for ApubPost {
       name = name.chars().take(MAX_TITLE_LENGTH).collect();
     }
 
+    // read existing, local post if any (for generating mod log)
+    let old_post = page.id.dereference_local(context).await;
+
     let form = if !page.is_mod_action(context).await? {
       let first_attachment = page.attachment.into_iter().map(Attachment::url).next();
       let url = if first_attachment.is_some() {
@@ -195,10 +198,13 @@ impl Object for ApubPost {
       } else {
         None
       };
-      let (metadata_res, thumbnail_url) = if let Some(url) = &url {
-        fetch_site_data(context.client(), context.settings(), Some(url)).await
-      } else {
-        (None, page.image.map(|i| i.url.into()))
+      // Only fetch metadata if the post has a url and was not seen previously. We dont want to
+      // waste resources by fetching metadata for the same post multiple times.
+      let (metadata_res, thumbnail_url) = match &url {
+        Some(url) if old_post.is_err() => {
+          fetch_site_data(context.client(), context.settings(), Some(url)).await
+        }
+        _ => (None, page.image.map(|i| i.url.into())),
       };
       let (embed_title, embed_description, embed_video_url) = metadata_res
         .map(|u| (u.title, u.description, u.embed_video_url))
@@ -245,8 +251,6 @@ impl Object for ApubPost {
         .updated(page.updated.map(|u| u.naive_local()))
         .build()
     };
-    // read existing, local post if any (for generating mod log)
-    let old_post = page.id.dereference_local(context).await;
 
     let post = Post::create(context.pool(), &form).await?;
 

src/lib.rs

@@ -34,7 +34,6 @@ use lemmy_utils::{
 };
 use reqwest::Client;
 use reqwest_middleware::ClientBuilder;
-use reqwest_retry::{policies::ExponentialBackoff, RetryTransientMiddleware};
 use reqwest_tracing::TracingMiddleware;
 use std::{env, thread, time::Duration};
 use tracing::subscriber::set_global_default;
@@ -110,18 +109,11 @@ pub async fn start_lemmy_server() -> Result<(), LemmyError> {
   let reqwest_client = Client::builder()
     .user_agent(user_agent.clone())
     .timeout(REQWEST_TIMEOUT)
+    .connect_timeout(REQWEST_TIMEOUT)
     .build()?;
 
-  let retry_policy = ExponentialBackoff {
-    max_n_retries: 3,
-    max_retry_interval: REQWEST_TIMEOUT,
-    min_retry_interval: Duration::from_millis(100),
-    backoff_exponent: 2,
-  };
-
   let client = ClientBuilder::new(reqwest_client.clone())
     .with(TracingMiddleware::default())
-    .with(RetryTransientMiddleware::new_with_policy(retry_policy))
     .build();
 
   // Pictrs cannot use the retry middleware