fix(icons): cache one source per icon, retry rate limits, stop panicking (#390)

Three faults, one cause each.

**One download per colour and per size.** The disk entry was named
`{slug}-{color}-{w}x{h}.svg` because the Iconify URL baked colour and size into
the SVG. A cache could hold 21 copies of `lucide:sparkles`, and changing only a
colour sent an icon that rendered fine offline back to the network. The source
is now fetched once per `prefix:name`, cached under `{slug}.svg`, and recoloured
and resized locally — Lucide draws with `stroke="currentColor"`, so recolouring
is a substitution, and resizing rewrites the root `width`/`height` while leaving
the `viewBox` alone. Done without a regex crate: the attribute rewrite is a
short scan of the root tag.

**No retry.** A single `GET`, so a `429` from `api.iconify.design` under
parallel renders lost the icon. Four attempts with exponential backoff, and only
for `429` and `5xx` — a `404` is not going to resolve on the fourth try, and
retrying it would make a typo take four times as long to report.

**`render` panicked where `still` and `sheet` warned.** The panic was not an
oversight: `an_unresolvable_icon_must_fail_the_preload_not_be_swallowed` exists
to enforce it, and says "must panic (or otherwise hard-fail)". So the severity
stays and the mechanism changes. `prefetch_icons` returns a named
`IconsUnresolved` error, the encode paths propagate it, and `still` and `sheet`
now preload too, so all three fail the same way with the same message and a
clean exit code instead of a backtrace. The studio warns and keeps its preview
alive, which is the one caller that must not die. `validate` stays offline.

    $ rustmotion still -f missing-icon.json --time 0 -o out.png
    Error: 1 icon(s) could not be loaded — checked the disk cache at
    ~/.cache/rustmotion/icons and the network, both failed:
      - 'lucide:no-such-icon-xyz' (color #2A2F6B, target 84x84px):
        Failed to fetch icon: http status: 404
    [exit 1]

`disk_cache_is_keyed_by_icon_color_and_size` asserted the first fault as a
guarantee — "different colors must not share a cache file". It is replaced by
its opposite rather than worked around.

Closes #365

Author: LeadcodeDev

crates/rustmotion-core/src/engine/renderer/assets.rs

@@ -67,10 +67,89 @@ pub fn icon_cache_dir() -> PathBuf {
     base.join("rustmotion").join("icons")
 }
 
-fn icon_cache_file(cache_dir: &Path, icon: &str, color: &str, width: u32, height: u32) -> PathBuf {
-    let slug = icon.replace(':', "_");
-    let hex_color = color.trim_start_matches('#').to_lowercase();
-    cache_dir.join(format!("{slug}-{hex_color}-{width}x{height}.svg"))
+fn icon_source_cache_file(cache_dir: &Path, icon: &str) -> PathBuf {
+    cache_dir.join(format!("{}.svg", icon.replace(':', "_")))
+}
+
+fn set_root_attribute(svg: &str, attribute: &str, value: &str) -> String {
+    let Some(tag_start) = svg.find("<svg") else {
+        return svg.to_string();
+    };
+    let Some(tag_len) = svg[tag_start..].find('>') else {
+        return svg.to_string();
+    };
+    let tag = &svg[tag_start..tag_start + tag_len];
+
+    let needle = format!(" {attribute}=\"");
+    let replaced = match tag.find(&needle) {
+        Some(at) => {
+            let value_start = at + needle.len();
+            match tag[value_start..].find('"') {
+                Some(value_len) => format!(
+                    "{}{}\"{}",
+                    &tag[..value_start],
+                    value,
+                    &tag[value_start + value_len..]
+                ),
+                None => tag.to_string(),
+            }
+        }
+        None => format!("<svg {attribute}=\"{value}\"{}", &tag["<svg".len()..]),
+    };
+
+    format!(
+        "{}{}{}",
+        &svg[..tag_start],
+        replaced,
+        &svg[tag_start + tag_len..]
+    )
+}
+
+fn recolour_and_resize(source: &[u8], hex_color: &str, width: u32, height: u32) -> Vec<u8> {
+    let coloured =
+        String::from_utf8_lossy(source).replace("currentColor", &format!("#{hex_color}"));
+    let sized = set_root_attribute(&coloured, "width", &width.to_string());
+    set_root_attribute(&sized, "height", &height.to_string()).into_bytes()
+}
+
+const ICON_FETCH_ATTEMPTS: u32 = 4;
+
+fn status_is_worth_retrying(error: &ureq::Error) -> bool {
+    matches!(error, ureq::Error::StatusCode(code) if *code == 429 || *code >= 500)
+}
+
+fn fetch_icon_source(icon: &str, prefix: &str, name: &str) -> Result<Vec<u8>> {
+    let url = format!("https://api.iconify.design/{prefix}/{name}.svg");
+    let mut backoff = Duration::from_millis(250);
+    let mut last_reason = String::new();
+
+    for attempt in 0..ICON_FETCH_ATTEMPTS {
+        match http_agent().get(&url).call() {
+            Ok(response) => {
+                return response
+                    .into_body()
+                    .read_to_vec()
+                    .map_err(|e| RustmotionError::IconFetch {
+                        icon: icon.to_string(),
+                        reason: e.to_string(),
+                    })
+            }
+            Err(e) => {
+                last_reason = e.to_string();
+                let is_last_attempt = attempt + 1 == ICON_FETCH_ATTEMPTS;
+                if !status_is_worth_retrying(&e) || is_last_attempt {
+                    break;
+                }
+                std::thread::sleep(backoff);
+                backoff *= 2;
+            }
+        }
+    }
+
+    Err(RustmotionError::IconFetch {
+        icon: icon.to_string(),
+        reason: last_reason,
+    })
 }
 
 pub fn fetch_icon_svg(icon: &str, color: &str, width: u32, height: u32) -> Result<Vec<u8>> {
@@ -93,37 +172,21 @@ pub fn fetch_icon_svg_in(
     let width = width.max(1);
     let height = height.max(1);
 
-    let cache_file = icon_cache_file(cache_dir, icon, color, width, height);
-    if let Ok(data) = std::fs::read(&cache_file) {
-        if !data.is_empty() {
-            return Ok(data);
+    let hex_color = hex_color.to_lowercase();
+    let source_file = icon_source_cache_file(cache_dir, icon);
+
+    let source = match std::fs::read(&source_file) {
+        Ok(data) if !data.is_empty() => data,
+        _ => {
+            let fetched = fetch_icon_source(icon, prefix, name)?;
+            if std::fs::create_dir_all(cache_dir).is_ok() {
+                let _ = std::fs::write(&source_file, &fetched);
+            }
+            fetched
         }
-    }
-
-    let url = format!(
-        "https://api.iconify.design/{}/{}.svg?color=%23{}&width={}&height={}",
-        prefix, name, hex_color, width, height
-    );
-    let response = http_agent()
-        .get(&url)
-        .call()
-        .map_err(|e| RustmotionError::IconFetch {
-            icon: icon.to_string(),
-            reason: e.to_string(),
-        })?;
-    let body = response
-        .into_body()
-        .read_to_vec()
-        .map_err(|e| RustmotionError::IconFetch {
-            icon: icon.to_string(),
-            reason: e.to_string(),
-        })?;
-
-    if std::fs::create_dir_all(cache_dir).is_ok() {
-        let _ = std::fs::write(&cache_file, &body);
-    }
+    };
 
-    Ok(body)
+    Ok(recolour_and_resize(&source, &hex_color, width, height))
 }
 
 static VIDEO_FRAME_CACHE: OnceLock<VideoFrameCacheMap> = OnceLock::new();
@@ -419,21 +482,34 @@ mod tests {
         let (w, h) = (48, 48);
         let svg_bytes = b"<svg>fake cached icon for the test suite</svg>".to_vec();
 
-        let cache_file = icon_cache_file(&cache_dir, icon, color, w, h);
+        let cache_file = icon_source_cache_file(&cache_dir, icon);
+        std::fs::create_dir_all(&cache_dir).unwrap();
         std::fs::write(&cache_file, &svg_bytes).unwrap();
 
         let result = fetch_icon_svg_in(icon, color, w, h, &cache_dir).expect("cache hit");
-        assert_eq!(result, svg_bytes);
+        let result = String::from_utf8(result).unwrap();
+        assert!(
+            result.contains("fake cached icon for the test suite"),
+            "the cached source must be what is served — the network was never reached: {result}"
+        );
+        assert!(
+            result.contains(r#"width="48""#),
+            "and it is resized on the way out, which is what lets one cached source serve \
+             every colour and size: {result}"
+        );
     }
 
     #[test]
-    fn disk_cache_is_keyed_by_icon_color_and_size() {
+    fn disk_cache_is_keyed_by_the_icon_alone() {
         let cache_dir = unique_temp_dir("cache-keying");
-        let a = icon_cache_file(&cache_dir, "lucide:home", "#FFFFFF", 80, 80);
-        let b = icon_cache_file(&cache_dir, "lucide:home", "#000000", 80, 80);
-        let c = icon_cache_file(&cache_dir, "lucide:home", "#FFFFFF", 40, 40);
-        assert_ne!(a, b, "different colors must not share a cache file");
-        assert_ne!(a, c, "different sizes must not share a cache file");
+        assert_eq!(
+            icon_source_cache_file(&cache_dir, "lucide:home"),
+            icon_source_cache_file(&cache_dir, "lucide:home"),
+        );
+        assert_ne!(
+            icon_source_cache_file(&cache_dir, "lucide:home"),
+            icon_source_cache_file(&cache_dir, "lucide:check"),
+        );
     }
 
     #[test]
@@ -457,7 +533,7 @@ mod tests {
         let first = fetch_icon_svg_in(icon, color, w, h, &cache_dir).expect("live fetch");
         assert!(!first.is_empty());
 
-        let cache_file = icon_cache_file(&cache_dir, icon, color, w, h);
+        let cache_file = icon_source_cache_file(&cache_dir, icon);
         assert!(
             cache_file.exists(),
             "a successful live fetch must be persisted to disk"
@@ -647,3 +723,78 @@ mod tests {
         let _ = ffprobe_available();
     }
 }
+
+#[cfg(test)]
+mod icon_source_cache_tests {
+    use super::*;
+
+    const LUCIDE_CHECK: &str = r#"<svg xmlns="http://www.w3.org/2000/svg" width="24" height="24" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2"><path d="M20 6 9 17l-5-5"/></svg>"#;
+
+    #[test]
+    fn the_disk_entry_is_named_by_the_icon_alone() {
+        let dir = Path::new("/tmp/icons");
+        assert_eq!(
+            icon_source_cache_file(dir, "lucide:check"),
+            dir.join("lucide_check.svg"),
+            "one entry per icon: naming it by colour and size is what filled a cache with 20 \
+             copies of the same glyph and made a colour change hit the network"
+        );
+    }
+
+    #[test]
+    fn recolouring_substitutes_current_color_everywhere() {
+        let out = recolour_and_resize(LUCIDE_CHECK.as_bytes(), "2a2f6b", 84, 84);
+        let out = String::from_utf8(out).unwrap();
+        assert!(!out.contains("currentColor"));
+        assert!(out.contains(r##"stroke="#2a2f6b""##), "got {out}");
+    }
+
+    #[test]
+    fn resizing_rewrites_the_root_size_and_leaves_the_viewbox_alone() {
+        let out = recolour_and_resize(LUCIDE_CHECK.as_bytes(), "ffffff", 84, 96);
+        let out = String::from_utf8(out).unwrap();
+        assert!(out.contains(r#"width="84""#), "got {out}");
+        assert!(out.contains(r#"height="96""#), "got {out}");
+        assert!(
+            out.contains(r#"viewBox="0 0 24 24""#),
+            "the viewBox is the glyph's own coordinate space and must survive a resize: {out}"
+        );
+    }
+
+    #[test]
+    fn a_root_without_a_size_gains_one() {
+        let bare = r#"<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24"></svg>"#;
+        let out =
+            String::from_utf8(recolour_and_resize(bare.as_bytes(), "000000", 32, 32)).unwrap();
+        assert!(
+            out.contains(r#"width="32""#) && out.contains(r#"height="32""#),
+            "got {out}"
+        );
+    }
+
+    #[test]
+    fn two_colours_of_one_icon_share_a_single_source_entry() {
+        let dir = Path::new("/tmp/icons");
+        assert_eq!(
+            icon_source_cache_file(dir, "lucide:sparkles"),
+            icon_source_cache_file(dir, "lucide:sparkles")
+        );
+        let red = recolour_and_resize(LUCIDE_CHECK.as_bytes(), "ff0000", 24, 24);
+        let blue = recolour_and_resize(LUCIDE_CHECK.as_bytes(), "0000ff", 24, 24);
+        assert_ne!(
+            red, blue,
+            "the same source still produces different renders"
+        );
+    }
+
+    #[test]
+    fn only_rate_limits_and_server_faults_are_retried() {
+        assert!(status_is_worth_retrying(&ureq::Error::StatusCode(429)));
+        assert!(status_is_worth_retrying(&ureq::Error::StatusCode(503)));
+        assert!(
+            !status_is_worth_retrying(&ureq::Error::StatusCode(404)),
+            "a missing icon is not going to appear on the fourth try — retrying it would just \
+             make a typo take four times as long to report"
+        );
+    }
+}

crates/rustmotion-core/src/error.rs

@@ -49,6 +49,17 @@ pub enum RustmotionError {
     #[error("Failed to parse icon SVG '{icon}': {reason}")]
     IconParse { icon: String, reason: String },
 
+    #[error(
+        "{count} icon(s) could not be loaded — checked the disk cache at {cache_dir} and the \
+         network, both failed:\n  - {details}\nA render must not silently omit an icon: fix \
+         the identifier(s), or connect once so they are downloaded and cached for offline use."
+    )]
+    IconsUnresolved {
+        count: usize,
+        cache_dir: String,
+        details: String,
+    },
+
     #[error("Failed to create Skia image from {target}")]
     SkiaImageCreation { target: String },
 

crates/rustmotion/src/cli/commands/sheet.rs

@@ -228,6 +228,10 @@ pub fn cmd_sheet(
         engine::renderer::load_custom_fonts(&scenario.fonts);
     }
 
+    for view in &scenario.views {
+        engine::prefetch_icons(&view.scenes)?;
+    }
+
     let start_time = std::time::Instant::now();
     let config = &scenario.video;
     let fps = config.fps;

crates/rustmotion/src/cli/commands/still.rs

@@ -43,6 +43,10 @@ pub fn cmd_still(
         engine::renderer::load_custom_fonts(&scenario.fonts);
     }
 
+    for view in &scenario.views {
+        engine::prefetch_icons(&view.scenes)?;
+    }
+
     let config = &scenario.video;
     let fps = config.fps;
 

crates/rustmotion/src/encode/video/ffmpeg.rs

@@ -313,7 +313,7 @@ fn encode_with_ffmpeg_hw_impl(
     let fps = config.fps;
 
     for view in &scenario.views {
-        prefetch_icons(&view.scenes);
+        prefetch_icons(&view.scenes)?;
     }
     for failure in analyze_scenario_audio(scenario) {
         eprintln!("rustmotion: audio-reactive: {failure} — waveform/audio_spectrum will render flat for this track.");

crates/rustmotion/src/encode/video/formats.rs

@@ -53,7 +53,7 @@ fn encode_png_sequence_to_dir(
     let height = config.height;
 
     for view in &scenario.views {
-        prefetch_icons(&view.scenes);
+        prefetch_icons(&view.scenes)?;
     }
 
     let tasks = build_frame_tasks(scenario);
@@ -146,7 +146,7 @@ fn encode_gif_to_path(
     let fps = config.fps;
 
     for view in &scenario.views {
-        prefetch_icons(&view.scenes);
+        prefetch_icons(&view.scenes)?;
     }
 
     let tasks = build_frame_tasks(scenario);
@@ -232,7 +232,7 @@ fn encode_raw_frames(scenario: &Scenario, quiet: bool, writer: &mut dyn Write) -
     let config = &scenario.video;
 
     for view in &scenario.views {
-        prefetch_icons(&view.scenes);
+        prefetch_icons(&view.scenes)?;
     }
 
     let tasks = build_frame_tasks(scenario);

crates/rustmotion/src/encode/video/h264.rs

@@ -56,7 +56,7 @@ fn encode_video_impl(
 
     for view in &scenario.views {
         preextract_video_frames(&view.scenes, fps);
-        prefetch_icons(&view.scenes);
+        prefetch_icons(&view.scenes)?;
     }
     for failure in analyze_scenario_audio(scenario) {
         eprintln!("rustmotion: audio-reactive: {failure} — waveform/audio_spectrum will render flat for this track.");
@@ -158,7 +158,7 @@ pub fn encode_video_incremental(
 
     for view in &scenario.views {
         preextract_video_frames(&view.scenes, fps);
-        prefetch_icons(&view.scenes);
+        prefetch_icons(&view.scenes)?;
     }
     for failure in analyze_scenario_audio(scenario) {
         eprintln!("rustmotion: audio-reactive: {failure} — waveform/audio_spectrum will render flat for this track.");