feat(link-preview): retry a transient blip inline before the self-heal wait

Give a blanked link one immediate second chance so a momentary failure
recovers right away instead of waiting out the full self-heal window.

- A transient fetch failure (timeout/5xx/network) earns one inline retry after
  a short backoff before falling into the 30s self-heal wait. If that retry
  succeeds the card shows immediately.
- A rate limit (429) skips the inline retry (an immediate retry would just be
  throttled again) and waits out its Retry-After instead, so we do not re-trip
  the limit. The whole sequence stays under one coalesced promise inside the
  2-concurrent scheduler.
- Move the rate-limit / transient-status classification into the rate_limit
  submodule where retry_after_duration already lives, keeping link_preview.rs
  within the file-size ratchet.

Co-authored-by: Taylor Ho <taylorkmho@gmail.com>
Signed-off-by: Taylor Ho <taylorkmho@gmail.com>

desktop/src-tauri/src/commands/link_preview.rs

@@ -14,7 +14,10 @@ use url::Url;
 #[path = "link_preview_rate_limit.rs"]
 mod rate_limit;
 
-use rate_limit::{image_host_cooldown_remaining, retry_after_duration, set_image_host_cooldown};
+use rate_limit::{
+    image_host_cooldown_remaining, is_transient_status, rate_limited_response_error,
+    retry_after_duration, set_image_host_cooldown,
+};
 
 const MAX_PREVIEW_FETCH_BYTES: usize = 256 * 1024;
 const MAX_IMAGE_FETCH_BYTES: usize = 2 * 1024 * 1024;
@@ -93,6 +96,9 @@ async fn fetch_link_preview_metadata_inner(
             // retries soon instead of caching an empty card for the full miss
             // TTL. Any other non-success (e.g. 404) is a genuine hard miss.
             let status = response.status();
+            if let Some(error) = rate_limited_response_error(&response) {
+                return Err(error);
+            }
             if is_transient_status(status) {
                 return Err(format!("link preview request failed: HTTP {status}"));
             }
@@ -226,16 +232,6 @@ async fn send_pinned_request(url: &Url, accept: &str) -> Result<reqwest::Respons
         .map_err(|error| format!("link preview request failed: {error}"))
 }
 
-/// A retryable HTTP status: rate limit, request timeout, too early, or any
-/// server error. These are transient and should be retried soon rather than
-/// cached as a genuine "no metadata" miss.
-fn is_transient_status(status: reqwest::StatusCode) -> bool {
-    status == reqwest::StatusCode::TOO_MANY_REQUESTS
-        || status == reqwest::StatusCode::REQUEST_TIMEOUT
-        || status == reqwest::StatusCode::TOO_EARLY
-        || status.is_server_error()
-}
-
 fn is_html_response(response: &reqwest::Response) -> bool {
     response
         .headers()

desktop/src-tauri/src/commands/link_preview_rate_limit.rs

@@ -22,6 +22,43 @@ pub(super) fn retry_after_duration(response: &reqwest::Response) -> Option<Durat
         .map(|duration| duration.min(MAX_IMAGE_RETRY_AFTER))
 }
 
+/// Marker prefix the caller matches to distinguish a rate limit (429) from an
+/// ordinary transient blip. When the server supplied a Retry-After, its whole
+/// seconds are appended so the caller can wait exactly that long instead of
+/// blindly retrying or falling back to the default transient window.
+pub(super) const RATE_LIMITED_ERROR_PREFIX: &str = "link preview rate limited";
+
+pub(super) fn rate_limited_error(retry_after: Option<Duration>) -> String {
+    match retry_after {
+        Some(retry_after) => {
+            format!(
+                "{RATE_LIMITED_ERROR_PREFIX}: retry-after {}",
+                retry_after.as_secs()
+            )
+        }
+        None => RATE_LIMITED_ERROR_PREFIX.to_string(),
+    }
+}
+
+/// A 429 is a genuine rate limit: an immediate retry would just be throttled
+/// again, so surface it distinctly (carrying any Retry-After) rather than as an
+/// ordinary transient blip. Returns `None` for every other status so the caller
+/// keeps its usual transient/hard-miss handling.
+pub(super) fn rate_limited_response_error(response: &reqwest::Response) -> Option<String> {
+    (response.status() == reqwest::StatusCode::TOO_MANY_REQUESTS)
+        .then(|| rate_limited_error(retry_after_duration(response)))
+}
+
+/// A retryable HTTP status: rate limit, request timeout, too early, or any
+/// server error. These are transient and should be retried soon rather than
+/// cached as a genuine "no metadata" miss.
+pub(super) fn is_transient_status(status: reqwest::StatusCode) -> bool {
+    status == reqwest::StatusCode::TOO_MANY_REQUESTS
+        || status == reqwest::StatusCode::REQUEST_TIMEOUT
+        || status == reqwest::StatusCode::TOO_EARLY
+        || status.is_server_error()
+}
+
 pub(super) fn image_host_cooldown_remaining(url: &Url) -> Option<Duration> {
     let host = url.host_str()?;
     let cooldowns = IMAGE_HOST_COOLDOWNS.get_or_init(|| Mutex::new(HashMap::new()));
@@ -60,3 +97,30 @@ pub(super) fn set_image_host_cooldown(url: &Url, retry_after: Duration) {
         cooldowns.insert(host.to_string(), expires_at);
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::{rate_limited_error, RATE_LIMITED_ERROR_PREFIX};
+    use std::time::Duration;
+
+    #[test]
+    fn rate_limited_error_carries_retry_after_seconds() {
+        // A rate limit surfaces a distinct marker so the caller skips the fast
+        // inline retry; when the server gave a Retry-After we append its whole
+        // seconds so the caller can wait exactly that long.
+        assert_eq!(
+            rate_limited_error(Some(Duration::from_secs(45))),
+            format!("{RATE_LIMITED_ERROR_PREFIX}: retry-after 45"),
+        );
+        // Sub-second remainders truncate to whole seconds.
+        assert_eq!(
+            rate_limited_error(Some(Duration::from_millis(1_500))),
+            format!("{RATE_LIMITED_ERROR_PREFIX}: retry-after 1"),
+        );
+        // No Retry-After: bare marker, caller falls back to its default window.
+        assert_eq!(
+            rate_limited_error(None),
+            RATE_LIMITED_ERROR_PREFIX.to_string(),
+        );
+    }
+}

desktop/src/shared/lib/useResolvedLinkPreviews.test.mjs

@@ -133,10 +133,14 @@ test("metadata loader retries transient images after the server cooldown", async
   assert.equal(calls, 2);
 });
 
-test("metadata loader retries a rejected request after the short transient TTL", async () => {
-  let now = 1_000;
+test("a transient blip earns one immediate inline retry before caching", async () => {
+  // A single unlucky fetch (timeout/5xx/network) should not blank the card: the
+  // loader retries once inline after a short backoff. If that second attempt
+  // succeeds, the card resolves right away with no wait at all.
+  const now = 1_000;
   let calls = 0;
   const loader = __linkPreviewMetadataTest.createMetadataLoader({
+    delay: () => Promise.resolve(),
     fetcher: async () => {
       calls += 1;
       if (calls === 1) throw new Error("temporary failure");
@@ -145,25 +149,104 @@ test("metadata loader retries a rejected request after the short transient TTL",
     now: () => now,
   });
 
+  assert.deepEqual((await loader.load(preview.href)).metadata, metadata());
+  assert.equal(calls, 2);
+});
+
+test("a blip that also fails the inline retry caches transient, recovering after the short TTL", async () => {
+  // When both the initial fetch and its inline retry fail, the loader gives up
+  // and caches a transient entry (short 30s TTL), not a full 5-min hard miss.
+  let now = 1_000;
+  let calls = 0;
+  const loader = __linkPreviewMetadataTest.createMetadataLoader({
+    delay: () => Promise.resolve(),
+    fetcher: async () => {
+      calls += 1;
+      // Both the initial attempt and its inline retry fail; success afterward.
+      if (calls <= 2) throw new Error("temporary failure");
+      return metadata();
+    },
+    now: () => now,
+  });
+
   assert.equal((await loader.load(preview.href)).metadata, null);
   assert.equal((await loader.load(preview.href)).metadata, null);
-  assert.equal(calls, 1);
+  assert.equal(calls, 2);
 
   now += 30_000;
   assert.deepEqual((await loader.load(preview.href)).metadata, metadata());
-  assert.equal(calls, 2);
+  assert.equal(calls, 3);
 });
 
 test("a rejected fetch does not poison the cache for the full miss TTL", async () => {
   // Regression: a single transient failure (timeout/429/5xx/network) used to be
   // cached like a genuine "no metadata" miss, blanking a perfectly valid link
-  // for 5 minutes. It must clear well before the miss TTL so the card recovers.
+  // for 5 minutes. Even when the inline retry also fails, the transient entry
+  // must clear well before the miss TTL so the card recovers.
   let now = 1_000;
   let calls = 0;
   const loader = __linkPreviewMetadataTest.createMetadataLoader({
+    delay: () => Promise.resolve(),
     fetcher: async () => {
       calls += 1;
-      if (calls === 1) throw new Error("temporary failure");
+      if (calls <= 2) throw new Error("temporary failure");
+      return metadata();
+    },
+    now: () => now,
+  });
+
+  assert.equal((await loader.load(preview.href)).metadata, null);
+  assert.equal(calls, 2);
+
+  // Well before NULL_METADATA_RETRY_MS (5 min) the transient entry has expired
+  // and the retry succeeds — a hard miss would still be cached here.
+  now += 60_000;
+  assert.deepEqual((await loader.load(preview.href)).metadata, metadata());
+  assert.equal(calls, 3);
+});
+
+test("a rate limit skips the inline retry and waits out its Retry-After", async () => {
+  // A 429 must NOT get the fast inline retry (an immediate retry would just be
+  // throttled again). It caches transient and honors the server's Retry-After
+  // as the wait, then refetches once that window elapses.
+  let now = 1_000;
+  let calls = 0;
+  const loader = __linkPreviewMetadataTest.createMetadataLoader({
+    delay: () => Promise.resolve(),
+    fetcher: async () => {
+      calls += 1;
+      if (calls === 1) {
+        throw new Error("link preview rate limited: retry-after 45");
+      }
+      return metadata();
+    },
+    now: () => now,
+  });
+
+  assert.equal((await loader.load(preview.href)).metadata, null);
+  // No inline retry on a rate limit: exactly one fetch so far.
+  assert.equal(calls, 1);
+
+  // The default transient window has passed, but the server asked for 45s — the
+  // entry is still cached, no refetch yet.
+  now += 30_000;
+  assert.equal((await loader.load(preview.href)).metadata, null);
+  assert.equal(calls, 1);
+
+  // Past the honored Retry-After the entry expires and the refetch succeeds.
+  now += 15_000;
+  assert.deepEqual((await loader.load(preview.href)).metadata, metadata());
+  assert.equal(calls, 2);
+});
+
+test("a rate limit without Retry-After falls back to the default transient window", async () => {
+  let now = 1_000;
+  let calls = 0;
+  const loader = __linkPreviewMetadataTest.createMetadataLoader({
+    delay: () => Promise.resolve(),
+    fetcher: async () => {
+      calls += 1;
+      if (calls === 1) throw new Error("link preview rate limited");
       return metadata();
     },
     now: () => now,
@@ -172,9 +255,7 @@ test("a rejected fetch does not poison the cache for the full miss TTL", async (
   assert.equal((await loader.load(preview.href)).metadata, null);
   assert.equal(calls, 1);
 
-  // Well before NULL_METADATA_RETRY_MS (5 min) the transient entry has expired
-  // and the retry succeeds — a hard miss would still be cached here.
-  now += 60_000;
+  now += 30_000;
   assert.deepEqual((await loader.load(preview.href)).metadata, metadata());
   assert.equal(calls, 2);
 });

desktop/src/shared/lib/useResolvedLinkPreviews.ts

@@ -45,6 +45,38 @@ type MetadataLoadResult = MetadataCacheEntry & {
 const DEFAULT_TRANSIENT_RETRY_MS = 30_000;
 const NULL_METADATA_RETRY_MS = 5 * 60_000;
 const MAX_CONCURRENT_METADATA_FETCHES = 2;
+/** Backoff before the single inline retry that a transient blip earns before
+ * it falls into the longer {@link DEFAULT_TRANSIENT_RETRY_MS} self-heal wait. */
+const INLINE_RETRY_BACKOFF_MS = 750;
+/** Prefix the native fetcher uses to mark a 429. A rate limit must NOT get the
+ * fast inline retry (we would just be throttled again); it waits instead. Kept
+ * in sync with RATE_LIMITED_ERROR_PREFIX in link_preview.rs. */
+const RATE_LIMITED_ERROR_PREFIX = "link preview rate limited";
+
+type TransientFailure = {
+  /** A 429 rate limit: skip the inline retry and wait it out. */
+  rateLimited: boolean;
+  /** Server-supplied Retry-After, in ms, when present on a rate limit. */
+  retryAfterMs?: number;
+};
+
+/** Classify a rejected native fetch. Any rejection is transient (a genuine
+ * "no metadata" result resolves, it does not reject); we only distinguish a
+ * rate limit — which must not be retried immediately — from an ordinary blip. */
+function classifyTransientFailure(reason: unknown): TransientFailure {
+  const message = reason instanceof Error ? reason.message : String(reason);
+  if (!message.startsWith(RATE_LIMITED_ERROR_PREFIX)) {
+    return { rateLimited: false };
+  }
+  const match = /retry-after (\d+)/.exec(message);
+  const retryAfterSeconds = match ? Number.parseInt(match[1], 10) : NaN;
+  return {
+    rateLimited: true,
+    retryAfterMs: Number.isFinite(retryAfterSeconds)
+      ? retryAfterSeconds * 1_000
+      : undefined,
+  };
+}
 
 /**
  * React may flush an interaction-triggered effect before the browser paints.
@@ -84,11 +116,13 @@ function metadataExpiry(
   metadata: LinkPreviewMetadata | null,
   now: number,
   transient = false,
+  transientRetryMs = DEFAULT_TRANSIENT_RETRY_MS,
 ): number | null {
   // A transient failure (timeout/429/5xx/network) must not stick for the full
   // miss TTL: one unlucky fetch would otherwise blank the card for 5 minutes
-  // even though the link is perfectly valid. Retry it soon instead.
-  if (transient) return now + DEFAULT_TRANSIENT_RETRY_MS;
+  // even though the link is perfectly valid. Retry it soon instead — after the
+  // server's Retry-After when it gave us one, otherwise the default window.
+  if (transient) return now + transientRetryMs;
   if (metadata === null) return now + NULL_METADATA_RETRY_MS;
   if (metadata.imageFetchState !== "transient_failure") return null;
   const retryAfterMs =
@@ -128,10 +162,13 @@ function createTaskScheduler(concurrency: number) {
 
 function createMetadataLoader({
   concurrency = MAX_CONCURRENT_METADATA_FETCHES,
+  delay = (ms: number) =>
+    new Promise<void>((resolve) => setTimeout(resolve, ms)),
   fetcher,
   now = Date.now,
 }: {
   concurrency?: number;
+  delay?: (ms: number) => Promise<void>;
   fetcher: (href: string) => Promise<LinkPreviewMetadata | null>;
   now?: () => number;
 }) {
@@ -165,16 +202,57 @@ function createMetadataLoader({
     }
 
     const requestGeneration = generation;
-    const promise = schedule(() => fetcher(href))
-      .then(
-        (metadata) => ({ metadata, transient: false }),
-        // A rejected fetch is a transient failure (timeout, network error, or a
-        // retryable status surfaced as an error), not a genuine "no metadata".
-        () => ({ metadata: null, transient: true }),
-      )
-      .then(({ metadata, transient }) => {
+    // One coalesced attempt sequence. A transient blip earns a single inline
+    // retry after a short backoff before we give up and fall into the longer
+    // self-heal wait; a rate limit (429) skips that retry — an immediate retry
+    // would just be throttled again — and waits out its Retry-After instead.
+    const attempt = (): Promise<{
+      metadata: LinkPreviewMetadata | null;
+      transient: boolean;
+      transientRetryMs: number;
+    }> =>
+      fetcher(href).then(
+        (metadata) => ({
+          metadata,
+          transient: false,
+          transientRetryMs: DEFAULT_TRANSIENT_RETRY_MS,
+        }),
+        (reason) => {
+          const failure = classifyTransientFailure(reason);
+          if (failure.rateLimited) {
+            return {
+              metadata: null,
+              transient: true,
+              transientRetryMs:
+                failure.retryAfterMs ?? DEFAULT_TRANSIENT_RETRY_MS,
+            };
+          }
+          return delay(INLINE_RETRY_BACKOFF_MS)
+            .then(() => fetcher(href))
+            .then(
+              (metadata) => ({
+                metadata,
+                transient: false,
+                transientRetryMs: DEFAULT_TRANSIENT_RETRY_MS,
+              }),
+              () => ({
+                metadata: null,
+                transient: true,
+                transientRetryMs: DEFAULT_TRANSIENT_RETRY_MS,
+              }),
+            );
+        },
+      );
+
+    const promise = schedule(attempt).then(
+      ({ metadata, transient, transientRetryMs }) => {
         const entry = {
-          expiresAt: metadataExpiry(metadata, now(), transient),
+          expiresAt: metadataExpiry(
+            metadata,
+            now(),
+            transient,
+            transientRetryMs,
+          ),
           metadata,
           transient,
         };
@@ -182,7 +260,8 @@ function createMetadataLoader({
           cache.set(key, entry);
         }
         return { key, ...entry };
-      });
+      },
+    );
     cache.set(key, promise);
     return promise;
   };