fix(link-preview): stop a transient fetch failure from blanking a valid card

A valid link (e.g. a GitHub PR page) could render no preview card for five
minutes after a single unlucky fetch, then quietly work again. One slow
handshake, a rate-limit blip, or a 5xx would poison the card.

Two paths funneled a transient failure into the same sticky negative cache as
a genuine "no metadata" miss:
- The resolver's fetch loader caught any rejection (timeout, network error) as
  a plain null and cached it under NULL_METADATA_RETRY_MS (5 minutes).
- The native fetcher returned Ok(None) for a retryable page status (429, 408,
  425, 5xx), which the resolver also cached for the full miss TTL.

Distinguish transient failures from hard misses:
- The loader now records a rejected fetch as transient and caches it for the
  short transient TTL (30s) instead of the 5-minute miss TTL. A resolved null
  (200 + HTML + no metadata) still holds the full miss TTL.
- The native fetcher surfaces a retryable page status as an error (via a shared
  is_transient_status helper, reused by the image path) so it takes the short
  transient TTL rather than the sticky negative cache.

Also widen the per-request timeout from 4s to 6s (total 10s to 15s) so slow
networks stop tripping the transient path in the first place.

Tests: JS regression that a rejected fetch clears well before the 5-minute
miss TTL while a genuine null miss stays cached; the rejected-retry test now
asserts the 30s transient boundary; Rust unit test that retryable statuses are
transient and permanent ones (404/403/401/410) are hard misses. Full desktop
unit suite 4580 passed; link_preview Rust suite 22 passed.

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

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

@@ -21,8 +21,8 @@ const MAX_IMAGE_FETCH_BYTES: usize = 2 * 1024 * 1024;
 const MAX_IMAGE_DIMENSION: u32 = 4096;
 const MAX_IMAGE_PIXELS: u64 = 16_000_000;
 const MAX_SANITIZED_DIMENSION: u32 = 1200;
-const PREVIEW_FETCH_TIMEOUT: Duration = Duration::from_secs(4);
-const PREVIEW_TOTAL_TIMEOUT: Duration = Duration::from_secs(10);
+const PREVIEW_FETCH_TIMEOUT: Duration = Duration::from_secs(6);
+const PREVIEW_TOTAL_TIMEOUT: Duration = Duration::from_secs(15);
 const MAX_REDIRECTS: usize = 3;
 const MAX_METADATA_CHARS: usize = 180;
 const MAX_METADATA_DESCRIPTION_CHARS: usize = 280;
@@ -87,7 +87,18 @@ async fn fetch_link_preview_metadata_inner(
             continue;
         }
 
-        if !response.status().is_success() || !is_html_response(&response) {
+        if !response.status().is_success() {
+            // A retryable status (rate limit, request timeout, too early, or a
+            // server error) is transient: surface it as an error so the caller
+            // 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 is_transient_status(status) {
+                return Err(format!("link preview request failed: HTTP {status}"));
+            }
+            return Ok(None);
+        }
+        if !is_html_response(&response) {
             return Ok(None);
         }
         let body = read_bytes_prefix(response, MAX_PREVIEW_FETCH_BYTES).await?;
@@ -215,6 +226,16 @@ 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()
@@ -350,11 +371,7 @@ async fn fetch_sanitized_image(
         }
         if !response.status().is_success() {
             let status = response.status();
-            if status == reqwest::StatusCode::TOO_MANY_REQUESTS
-                || status == reqwest::StatusCode::REQUEST_TIMEOUT
-                || status == reqwest::StatusCode::TOO_EARLY
-                || status.is_server_error()
-            {
+            if is_transient_status(status) {
                 let retry_after = retry_after_duration(&response);
                 if let Some(retry_after) = retry_after {
                     set_image_host_cooldown(&url, retry_after);
@@ -651,9 +668,9 @@ mod tests {
     use super::rate_limit::MAX_IMAGE_RETRY_AFTER;
     use super::{
         apply_image_result, declares_animation, extract_favicon_url, extract_image_url,
-        extract_link_preview_metadata, is_html_response, read_bytes_prefix, retry_after_duration,
-        sanitize_image, ImageFetchError, LinkPreviewImageFetchState, LinkPreviewMetadata,
-        MAX_METADATA_DESCRIPTION_CHARS,
+        extract_link_preview_metadata, is_html_response, is_transient_status, read_bytes_prefix,
+        retry_after_duration, sanitize_image, ImageFetchError, LinkPreviewImageFetchState,
+        LinkPreviewMetadata, MAX_METADATA_DESCRIPTION_CHARS,
     };
     use axum::{body::Body, http::Response, routing::get, Router};
     use base64::Engine as _;
@@ -957,4 +974,24 @@ mod tests {
         assert_eq!(extract_link_preview_metadata("<title>   </title>"), None);
         assert_eq!(extract_link_preview_metadata("<html></html>"), None);
     }
+
+    #[test]
+    fn transient_statuses_are_distinguished_from_hard_misses() {
+        use reqwest::StatusCode;
+        // Retryable: surfaced as an error so the caller retries soon instead of
+        // caching an empty card for the full miss TTL.
+        assert!(is_transient_status(StatusCode::TOO_MANY_REQUESTS));
+        assert!(is_transient_status(StatusCode::REQUEST_TIMEOUT));
+        assert!(is_transient_status(StatusCode::TOO_EARLY));
+        assert!(is_transient_status(StatusCode::INTERNAL_SERVER_ERROR));
+        assert!(is_transient_status(StatusCode::BAD_GATEWAY));
+        assert!(is_transient_status(StatusCode::SERVICE_UNAVAILABLE));
+        assert!(is_transient_status(StatusCode::GATEWAY_TIMEOUT));
+        // Genuine hard misses: cached for the full miss TTL, not retried early.
+        assert!(!is_transient_status(StatusCode::NOT_FOUND));
+        assert!(!is_transient_status(StatusCode::FORBIDDEN));
+        assert!(!is_transient_status(StatusCode::UNAUTHORIZED));
+        assert!(!is_transient_status(StatusCode::GONE));
+        assert!(!is_transient_status(StatusCode::OK));
+    }
 }

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

@@ -133,7 +133,7 @@ test("metadata loader retries transient images after the server cooldown", async
   assert.equal(calls, 2);
 });
 
-test("metadata loader retries rejected requests after the negative-cache TTL", async () => {
+test("metadata loader retries a rejected request after the short transient TTL", async () => {
   let now = 1_000;
   let calls = 0;
   const loader = __linkPreviewMetadataTest.createMetadataLoader({
@@ -149,11 +149,61 @@ test("metadata loader retries rejected requests after the negative-cache TTL", a
   assert.equal((await loader.load(preview.href)).metadata, null);
   assert.equal(calls, 1);
 
-  now += 5 * 60_000;
+  now += 30_000;
   assert.deepEqual((await loader.load(preview.href)).metadata, metadata());
   assert.equal(calls, 2);
 });
 
+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.
+  let now = 1_000;
+  let calls = 0;
+  const loader = __linkPreviewMetadataTest.createMetadataLoader({
+    fetcher: async () => {
+      calls += 1;
+      if (calls === 1) throw new Error("temporary failure");
+      return metadata();
+    },
+    now: () => now,
+  });
+
+  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;
+  assert.deepEqual((await loader.load(preview.href)).metadata, metadata());
+  assert.equal(calls, 2);
+});
+
+test("a genuine no-metadata miss stays cached for the full miss TTL", async () => {
+  let now = 1_000;
+  let calls = 0;
+  const loader = __linkPreviewMetadataTest.createMetadataLoader({
+    fetcher: async () => {
+      calls += 1;
+      return null;
+    },
+    now: () => now,
+  });
+
+  assert.equal((await loader.load(preview.href)).metadata, null);
+  assert.equal(calls, 1);
+
+  // A resolved null (200 + HTML + no metadata) is a hard miss: it must not be
+  // refetched inside the transient window.
+  now += 60_000;
+  assert.equal((await loader.load(preview.href)).metadata, null);
+  assert.equal(calls, 1);
+
+  now += 5 * 60_000;
+  assert.equal((await loader.load(preview.href)).metadata, null);
+  assert.equal(calls, 2);
+});
+
 test("metadata loader coalesces fragment variants and bounds concurrency", async () => {
   let active = 0;
   let maxActive = 0;

desktop/src/shared/lib/useResolvedLinkPreviews.ts

@@ -33,6 +33,9 @@ export type LinkPreviewMetadata = {
 type MetadataCacheEntry = {
   expiresAt: number | null;
   metadata: LinkPreviewMetadata | null;
+  /** A transient fetch failure (timeout/429/5xx/network) rather than a genuine
+   * "no metadata" result; retried soon instead of cached for the full miss TTL. */
+  transient?: boolean;
 };
 
 type MetadataLoadResult = MetadataCacheEntry & {
@@ -80,7 +83,12 @@ function metadataCacheKey(href: string): string {
 function metadataExpiry(
   metadata: LinkPreviewMetadata | null,
   now: number,
+  transient = false,
 ): 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;
   if (metadata === null) return now + NULL_METADATA_RETRY_MS;
   if (metadata.imageFetchState !== "transient_failure") return null;
   const retryAfterMs =
@@ -158,11 +166,17 @@ function createMetadataLoader({
 
     const requestGeneration = generation;
     const promise = schedule(() => fetcher(href))
-      .catch(() => null)
-      .then((metadata) => {
+      .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 }) => {
         const entry = {
-          expiresAt: metadataExpiry(metadata, now()),
+          expiresAt: metadataExpiry(metadata, now(), transient),
           metadata,
+          transient,
         };
         if (requestGeneration === generation) {
           cache.set(key, entry);