fix: remember a failed hub index read briefly instead of retrying every open

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

backend/windmill-store/src/resources.rs

@@ -2613,13 +2613,19 @@ struct HubCached<T> {
     value: T,
 }
 
-/// Ids only change when a resource type is published to the hub; picks move slowly and
-/// only reorder a list. Both are read on every drawer open, hence caching at all.
-const HUB_RT_IDS_TTL: std::time::Duration = std::time::Duration::from_secs(60 * 60);
+/// The index changes only when a resource type is published to the hub; picks move slowly
+/// and only reorder a list. Both are read on every drawer open, hence caching at all.
+const HUB_RT_INDEX_TTL: std::time::Duration = std::time::Duration::from_secs(60 * 60);
 const HUB_RT_PICKS_TTL: std::time::Duration = std::time::Duration::from_secs(5 * 60);
 
-static HUB_RT_IDS: LazyLock<
-    std::sync::RwLock<Option<HubCached<HashMap<String, HubResourceType>>>>,
+/// How long a *failed* index read is remembered. Short, and deliberately not the hour a
+/// success is good for: this read is on the path of every picker open, so an unreachable
+/// hub must not cost an outbound timeout each time, while one blip must not silence pick
+/// reporting for an hour.
+const HUB_RT_INDEX_FAILURE_TTL: std::time::Duration = std::time::Duration::from_secs(60);
+
+static HUB_RT_INDEX: LazyLock<
+    std::sync::RwLock<Option<HubCached<Option<HashMap<String, HubResourceType>>>>>,
 > = LazyLock::new(|| std::sync::RwLock::new(None));
 static HUB_RT_PICKS: LazyLock<std::sync::RwLock<Option<HubCached<Vec<HubResourceTypePicks>>>>> =
     LazyLock::new(|| std::sync::RwLock::new(None));
@@ -2649,7 +2655,10 @@ fn hub_cache_put<T>(cache: &std::sync::RwLock<Option<HubCached<T>>>, hub_base_ur
 struct HubResourceTypeEntry {
     id: i64,
     name: String,
-    app: String,
+    /// Optional so a hub that does not send it costs only the mapping. Required, it would
+    /// fail the whole parse and take pick reporting — which needs just the id — with it.
+    #[serde(default)]
+    app: Option<String>,
 }
 
 #[derive(Clone)]
@@ -2662,38 +2671,64 @@ struct HubResourceType {
     app: String,
 }
 
+/// Reads the index cache, choosing the TTL by what is stored: a failure expires far sooner
+/// than a success. `None` is a miss, `Some(None)` a remembered failure.
+fn hub_index_cached(hub_base_url: &str) -> Option<Option<HashMap<String, HubResourceType>>> {
+    let guard = HUB_RT_INDEX.read().ok()?;
+    let entry = guard.as_ref()?;
+    if entry.hub_base_url != hub_base_url {
+        return None;
+    }
+    let ttl = if entry.value.is_some() {
+        HUB_RT_INDEX_TTL
+    } else {
+        HUB_RT_INDEX_FAILURE_TTL
+    };
+    (entry.fetched_at.elapsed() < ttl).then(|| entry.value.clone())
+}
+
 /// What the hub knows about every published resource type, keyed by the name Windmill
 /// addresses it by. `None` when the hub cannot be reached or does not answer with a list.
 async fn hub_resource_types(
     db: &DB,
     hub_base_url: &str,
 ) -> Option<HashMap<String, HubResourceType>> {
-    if let Some(index) = hub_cache_get(&HUB_RT_IDS, hub_base_url, HUB_RT_IDS_TTL) {
-        return Some(index);
+    if let Some(cached) = hub_index_cached(hub_base_url) {
+        return cached;
     }
-    let response = windmill_common::utils::http_get_from_hub(
-        &windmill_common::utils::HTTP_CLIENT,
-        &format!("{hub_base_url}/resource_types/list"),
-        false,
-        None,
-        Some(db),
-    )
-    .await
-    .ok()?;
-    if !response.status().is_success() {
-        return None;
-    }
-    // Only the id and the app are kept. That listing carries every type's schema — around
-    // a megabyte — and neither reporting a pick nor grouping types by integration needs it.
-    let index: HashMap<String, HubResourceType> = response
-        .json::<Vec<HubResourceTypeEntry>>()
+    let index = async {
+        let response = windmill_common::utils::http_get_from_hub(
+            &windmill_common::utils::HTTP_CLIENT,
+            &format!("{hub_base_url}/resource_types/list"),
+            false,
+            None,
+            Some(db),
+        )
         .await
-        .ok()?
-        .into_iter()
-        .map(|rt| (rt.name, HubResourceType { id: rt.id, app: rt.app }))
-        .collect();
-    hub_cache_put(&HUB_RT_IDS, hub_base_url, index.clone());
-    Some(index)
+        .ok()?;
+        if !response.status().is_success() {
+            return None;
+        }
+        // Only the id and the app are kept. That listing carries every type's schema —
+        // around a megabyte — and neither reporting a pick nor grouping types by
+        // integration needs it.
+        Some(
+            response
+                .json::<Vec<HubResourceTypeEntry>>()
+                .await
+                .ok()?
+                .into_iter()
+                .map(|rt| {
+                    let app = rt.app.unwrap_or_else(|| rt.name.clone());
+                    (rt.name, HubResourceType { id: rt.id, app })
+                })
+                .collect::<HashMap<String, HubResourceType>>(),
+        )
+    }
+    .await;
+
+    hub_cache_put(&HUB_RT_INDEX, hub_base_url, index.clone());
+    index
 }
 
 #[derive(Serialize)]
@@ -2741,7 +2776,7 @@ async fn pick_hub_resource_type(
     Ok(Json(PickHubResourceTypeResult { success }))
 }
 
-#[derive(Serialize, Deserialize, Clone)]
+#[derive(Deserialize, Clone)]
 struct HubResourceTypePicks {
     name: String,
     /// The hub counts picks in a bigint, which its driver serialises as a string.
@@ -2756,7 +2791,34 @@ struct HubPickedResourceTypes {
 
 #[cfg(test)]
 mod hub_picks_tests {
-    use super::HubPickedResourceTypes;
+    use super::*;
+
+    /// Two properties of the index cache that a later edit could quietly drop: it is keyed on
+    /// the hub it came from, and a failure is forgotten long before a success is.
+    #[test]
+    fn the_index_cache_is_keyed_on_the_hub_and_forgets_failures_sooner() {
+        let index = || {
+            Some(HashMap::from([(
+                "slack".to_string(),
+                HubResourceType { id: 1, app: "slack".to_string() },
+            )]))
+        };
+
+        hub_cache_put(&HUB_RT_INDEX, "https://hub.example", index());
+        assert!(hub_index_cached("https://hub.example").is_some_and(|v| v.is_some()));
+        // Switching hubs must miss rather than serve the previous hub's mapping.
+        assert!(hub_index_cached("https://other.example").is_none());
+
+        // A remembered failure reads as a hit (so the hub is not re-attempted) carrying
+        // nothing, and only until the shorter of the two TTLs.
+        hub_cache_put(&HUB_RT_INDEX, "https://hub.example", None);
+        assert!(hub_index_cached("https://hub.example").is_some_and(|v| v.is_none()));
+        assert!(HUB_RT_INDEX_FAILURE_TTL < HUB_RT_INDEX_TTL);
+
+        if let Ok(mut guard) = HUB_RT_INDEX.write() {
+            *guard = None;
+        }
+    }
 
     /// The hub counts picks in a bigint, which postgres.js serialises as a string. Typing
     /// the field as a plain i64 fails the whole response, and the ranking silently empties.
@@ -2778,7 +2840,7 @@ mod hub_picks_tests {
 }
 
 /// One hub resource type as the pickers need it.
-#[derive(Serialize, Clone)]
+#[derive(Serialize)]
 struct HubResourceTypeInfo {
     name: String,
     /// The integration it belongs to, so a caller can total a workspace's resources per
@@ -2830,22 +2892,30 @@ async fn list_hub_resource_type_info(
         }
     };
 
-    let picks_by_name: HashMap<String, i64> =
+    let mut picks_by_name: HashMap<String, i64> =
         picks.into_iter().map(|rt| (rt.name, rt.picks)).collect();
     let index = hub_resource_types(&db, &hub_base_url)
         .await
         .unwrap_or_default();
 
-    Ok(Json(
-        index
+    let mut info: Vec<HubResourceTypeInfo> = index
+        .into_iter()
+        .map(|(name, rt)| HubResourceTypeInfo {
+            picks: picks_by_name.remove(&name).unwrap_or(0),
+            name,
+            app: rt.app,
+        })
+        .collect();
+    // What the index did not account for is a type the picks read knows and the listing does
+    // not — which is what a hub answering only the second read looks like. Its own name is
+    // the same guess a caller makes for any type the mapping misses.
+    info.extend(
+        picks_by_name
             .into_iter()
-            .map(|(name, rt)| HubResourceTypeInfo {
-                picks: picks_by_name.get(&name).copied().unwrap_or(0),
-                name,
-                app: rt.app,
-            })
-            .collect(),
-    ))
+            .map(|(name, picks)| HubResourceTypeInfo { app: name.clone(), name, picks }),
+    );
+
+    Ok(Json(info))
 }
 
 async fn get_resource_type(

frontend/src/lib/components/AppConnectInner.svelte

@@ -1025,7 +1025,15 @@
 					(x) => x.key,
 					(x) => resourceTypeDescriptions[x.key]
 				)
-			: [...items].sort((a, b) => popularity(a.key, b.key)))
+			: // Both signals are keyed by resource type, and a sandbox client is a second row
+				// against one (`salesforce_sandbox` saves a `salesforce`), so it ranks on the
+				// parent's popularity. Its own key still breaks the tie the pair then have, or
+				// the two would order arbitrarily.
+				[...items].sort(
+					(a, b) =>
+						popularity(stripSandboxSuffix(a.key), stripSandboxSuffix(b.key)) ||
+						a.key.localeCompare(b.key)
+				))
 	let rankedConnects = $derived(rank(filteredConnects))
 	let rankedConnectsManual = $derived(
 		rank(filteredConnectsManual) as typeof filteredConnectsManual | undefined