+5

src/room.rs

Reviewed

··· 694 694 added_by, 695 695 added_at: crate::util::now_iso(), 696 696 pending: true, 697 697 + failed: None, 697 698 }; 698 699 // Send the new entry through the state machine. The state emits 699 700 // Effect::AddPlaylistEntry (for persistence) and Effect::PublishSnapshot ··· 1197 1198 added_by: None, 1198 1199 added_at: "now".into(), 1199 1200 pending: false, 1201 1201 + failed: None, 1200 1202 }], 1201 1203 ..PlaybackState::default() 1202 1204 }; ··· 1218 1220 added_by: None, 1219 1221 added_at: "now".into(), 1220 1222 pending: false, 1223 1223 + failed: None, 1221 1224 }], 1222 1225 ..PlaybackState::default() 1223 1226 }; ··· 1242 1245 added_by: None, 1243 1246 added_at: "now".into(), 1244 1247 pending: false, 1248 1248 + failed: None, 1245 1249 }, 1246 1250 PlaylistEntry { 1247 1251 id: pid(2), ··· 1253 1257 added_by: None, 1254 1258 added_at: "now".into(), 1255 1259 pending: false, 1260 1260 + failed: None, 1256 1261 }, 1257 1262 ], 1258 1263 ..PlaybackState::default()

+41 -3

src/state.rs

Reviewed

··· 320 320 added_by: entry.added_by.clone(), 321 321 added_at: entry.added_at.clone(), 322 322 pending: false, 323 323 + failed: None, 323 324 }, 324 325 // Failed or absent: keep placeholder, pending stays true. 325 326 _ => entry.clone(), ··· 520 521 entry.duration = "--:--".into(); 521 522 entry.thumbnail = None; 522 523 entry.pending = false; 524 524 + entry.failed = Some(error.to_string()); 523 525 changed = true; 524 526 } 525 527 } ··· 1540 1542 added_by: None, 1541 1543 added_at: "2026-01-01T00:00:00Z".into(), 1542 1544 pending: false, 1545 1545 + failed: None, 1543 1546 } 1544 1547 } 1545 1548 ··· 1557 1560 added_by: None, 1558 1561 added_at: "2026-01-01T00:00:00Z".into(), 1559 1562 pending: true, 1563 1563 + failed: None, 1560 1564 } 1561 1565 } 1562 1566 ··· 2112 2116 fn test_download_failed_updates_playlist_entry() { 2113 2117 // Bug fix: a failed download for a URL in the playlist must 2114 2118 // mark the entry as failed (title = "Failed to load: <error>", 2115 2115 - // pending = false). Without this, a bad URL in the playlist stays 2116 2116 - // "Loading..." forever. 2119 2119 + // pending = false, failed = Some(error)). Without this, a bad URL 2120 2120 + // in the playlist stays "Loading..." forever and the UI has no 2121 2121 + // structured signal to render a failure state. 2117 2122 let mut s = PlaybackState::default(); 2118 2123 s.transition( 2119 2124 &Event::PlaylistEntryAdded(placeholder_playlist_entry(1, "https://example.com/bad")), ··· 2134 2139 assert_eq!(entry.duration, "--:--"); 2135 2140 assert!(entry.thumbnail.is_none()); 2136 2141 assert!(!entry.pending); 2142 2142 + assert_eq!(entry.failed.as_deref(), Some("yt-dlp returned 404")); 2137 2143 // PersistMetadata so the failure title lands in media_cache and 2138 2144 // survives rehydration. 2139 2145 assert!(effects.iter().any(|e| matches!( ··· 2143 2149 && title == "Failed to load: yt-dlp returned 404" 2144 2150 ))); 2145 2151 assert!(effects.iter().any(|e| matches!(e, Effect::PublishSnapshot))); 2152 2152 + } 2153 2153 + 2154 2154 + #[test] 2155 2155 + fn test_playlist_entry_failed_field_defaults_to_none() { 2156 2156 + // A freshly added playlist entry must have `failed = None`. The 2157 2157 + // `failed` field is only set when DownloadFailed flips a pending 2158 2158 + // entry, so the default has to be explicit and the type has to be 2159 2159 + // Option<String> (not just String) so the field can be deserialized 2160 2160 + // from older rows that don't carry it. 2161 2161 + let mut s = PlaybackState::default(); 2162 2162 + s.transition( 2163 2163 + &Event::PlaylistEntryAdded(placeholder_playlist_entry(1, "https://example.com/a")), 2164 2164 + 0, 2165 2165 + ); 2166 2166 + assert!(s.playlist[0].failed.is_none()); 2146 2167 } 2147 2168 2148 2169 #[test] ··· 2380 2401 ); 2381 2402 // Cross-state invariant: a Failed cache must not leave 2382 2403 // any matching playlist entries in pending state. They 2383 2383 - // should be flipped to the failure title. 2404 2404 + // should be flipped to the failure title and the 2405 2405 + // `failed` field should carry the error. 2384 2406 for entry in &state.playlist { 2385 2407 if entry.url == url { 2386 2408 assert!( ··· 2391 2413 entry.title.starts_with("Failed to load: "), 2392 2414 "DownloadFailed({url}) did not set failure title; got: {}", 2393 2415 entry.title, 2416 2416 + ); 2417 2417 + assert_eq!( 2418 2418 + entry.failed.as_deref(), 2419 2419 + Some(format!("err-{n}").as_str()), 2420 2420 + "DownloadFailed({url}) did not set failed reason; got: {:?}", 2421 2421 + entry.failed, 2394 2422 ); 2395 2423 } 2396 2424 } ··· 2797 2825 entry.title, 2798 2826 format!("Failed to load: {err}"), 2799 2827 "non-pending Failed entry title mismatch for {url}", 2828 2828 + ); 2829 2829 + assert_eq!( 2830 2830 + entry.failed.as_deref(), 2831 2831 + Some(err.as_str()), 2832 2832 + "non-pending Failed entry `failed` field mismatch for {url}", 2833 2833 + ); 2834 2834 + } else { 2835 2835 + assert!( 2836 2836 + entry.failed.is_none(), 2837 2837 + "non-pending Resolved playlist entry has a `failed` reason: {entry:?}", 2800 2838 ); 2801 2839 } 2802 2840 }

+141 -10

src/store.rs

Reviewed

··· 36 36 pub added_at: String, 37 37 /// Derived from `media_cache.state` at load time. 38 38 pub pending: bool, 39 39 + /// `Some(reason)` if the URL failed to resolve. Mutually exclusive with 40 40 + /// `pending: true` — a failed entry has `pending: false` so the UI can 41 41 + /// distinguish it from a still-loading placeholder. The reason is the 42 42 + /// same string the state machine wrote into `media_cache.title` as 43 43 + /// `"Failed to load: <reason>"`; the prefix is stripped during snapshot 44 44 + /// load so consumers see the raw reason. 45 45 + pub failed: Option<String>, 39 46 } 40 47 41 48 /// Full snapshot of a room's state loaded from the store. ··· 135 142 added_by: Option<String>, 136 143 added_at: String, 137 144 pending: bool, 145 145 + /// Newly-pushed entries are never failed; the field is set later by 146 146 + /// state-machine effects that mutate the playlist in place. Kept in the 147 147 + /// in-memory struct so the public `PlaylistEntry` shape is preserved on 148 148 + /// snapshot. 149 149 + failed: Option<String>, 138 150 } 139 151 140 152 impl From<InMemoryPlaylistEntry> for PlaylistEntry { ··· 149 161 added_by: e.added_by, 150 162 added_at: e.added_at, 151 163 pending: e.pending, 164 164 + failed: e.failed, 152 165 } 153 166 } 154 167 } ··· 313 326 added_by: None, 314 327 added_at: crate::util::now_iso(), 315 328 pending: true, 329 329 + failed: None, 316 330 }); 317 331 } 318 332 Effect::RemovePlaylistEntry(id) => { ··· 922 936 .await?; 923 937 924 938 let pending_from_state = |state: &str| state == "pending"; 939 939 + // The failure state is encoded in the cached title (set by 940 940 + // `Effect::PersistMetadata` from `handle_download_failed`). Strip the 941 941 + // prefix here so the public `PlaylistEntry.failed` field matches the 942 942 + // in-memory shape — consumers shouldn't have to parse the title. 943 943 + let failed_from_title = |title: &str| -> Option<String> { 944 944 + title 945 945 + .strip_prefix("Failed to load: ") 946 946 + .map(|rest| rest.to_string()) 947 947 + }; 925 948 926 949 let queue: Vec<QueuedTrack> = queue_rows 927 950 .into_iter() ··· 969 992 970 993 let playlist: Vec<PlaylistEntry> = playlist_rows 971 994 .into_iter() 972 972 - .map(|r| PlaylistEntry { 973 973 - id: parse_playlist_id(&r.id), 974 974 - url: r.url, 975 975 - title: r.title, 976 976 - duration: r.duration, 977 977 - thumbnail: r.thumbnail, 978 978 - source: r.source, 979 979 - added_by: r.added_by, 980 980 - added_at: r.added_at, 981 981 - pending: pending_from_state(&r.state), 995 995 + .map(|r| { 996 996 + let failed = failed_from_title(&r.title); 997 997 + PlaylistEntry { 998 998 + id: parse_playlist_id(&r.id), 999 999 + url: r.url, 1000 1000 + title: r.title, 1001 1001 + duration: r.duration, 1002 1002 + thumbnail: r.thumbnail, 1003 1003 + source: r.source, 1004 1004 + added_by: r.added_by, 1005 1005 + added_at: r.added_at, 1006 1006 + pending: pending_from_state(&r.state), 1007 1007 + failed, 1008 1008 + } 982 1009 }) 983 1010 .collect(); 984 1011 ··· 1708 1735 assert_eq!(entry.duration, "4:20"); 1709 1736 assert_eq!(entry.thumbnail, Some("https://img/x.jpg".into())); 1710 1737 assert!(!entry.pending); 1738 1738 + } 1739 1739 + 1740 1740 + #[tokio::test] 1741 1741 + async fn sqlite_failed_playlist_entry_rehydrates_with_failed_field() { 1742 1742 + // A playlist entry whose media_cache row was rewritten to a 1743 1743 + // "Failed to load: <err>" title (via Effect::PersistMetadata from 1744 1744 + // handle_download_failed) must rehydrate with `failed = Some(err)`. 1745 1745 + // The DB schema doesn't have a `failed` column — the title prefix is 1746 1746 + // the only signal — so the snapshot loader is the place this 1747 1747 + // contract is enforced. 1748 1748 + let store = sqlite_test_store().await; 1749 1749 + store.create_room("r", "R").await.unwrap(); 1750 1750 + 1751 1751 + let entry_id = PlaylistEntryId(uuid::Uuid::from_u128(1)); 1752 1752 + store 1753 1753 + .persist( 1754 1754 + "r", 1755 1755 + &[Effect::AddPlaylistEntry { 1756 1756 + id: entry_id, 1757 1757 + url: "https://example.com/dead".into(), 1758 1758 + title: "Loading...".into(), 1759 1759 + duration: "--:--".into(), 1760 1760 + thumbnail: None, 1761 1761 + source: "ytdlp".into(), 1762 1762 + }], 1763 1763 + ) 1764 1764 + .await 1765 1765 + .unwrap(); 1766 1766 + 1767 1767 + store 1768 1768 + .persist( 1769 1769 + "r", 1770 1770 + &[Effect::PersistMetadata { 1771 1771 + item_id: TrackId::nil(), 1772 1772 + url: "https://example.com/dead".into(), 1773 1773 + title: "Failed to load: yt-dlp returned 404".into(), 1774 1774 + duration: "--:--".into(), 1775 1775 + thumbnail: None, 1776 1776 + source: "ytdlp".into(), 1777 1777 + }], 1778 1778 + ) 1779 1779 + .await 1780 1780 + .unwrap(); 1781 1781 + 1782 1782 + let snap = store.load("r").await.unwrap().unwrap(); 1783 1783 + assert_eq!(snap.playlist.len(), 1); 1784 1784 + let entry = &snap.playlist[0]; 1785 1785 + assert_eq!(entry.title, "Failed to load: yt-dlp returned 404"); 1786 1786 + assert!(!entry.pending); 1787 1787 + assert_eq!( 1788 1788 + entry.failed.as_deref(), 1789 1789 + Some("yt-dlp returned 404"), 1790 1790 + "snapshot loader must strip the title prefix into the `failed` field", 1791 1791 + ); 1792 1792 + } 1793 1793 + 1794 1794 + #[tokio::test] 1795 1795 + async fn sqlite_resolved_playlist_entry_rehydrates_with_failed_none() { 1796 1796 + // A playlist entry with a non-failure title must rehydrate with 1797 1797 + // `failed = None`. The snapshot loader is prefix-match based, so 1798 1798 + // a title that doesn't start with "Failed to load: " must not 1799 1799 + // surface a stale `failed` reason. 1800 1800 + let store = sqlite_test_store().await; 1801 1801 + store.create_room("r", "R").await.unwrap(); 1802 1802 + 1803 1803 + let entry_id = PlaylistEntryId(uuid::Uuid::from_u128(1)); 1804 1804 + store 1805 1805 + .persist( 1806 1806 + "r", 1807 1807 + &[Effect::AddPlaylistEntry { 1808 1808 + id: entry_id, 1809 1809 + url: "https://example.com/ok".into(), 1810 1810 + title: "Loading...".into(), 1811 1811 + duration: "--:--".into(), 1812 1812 + thumbnail: None, 1813 1813 + source: "ytdlp".into(), 1814 1814 + }], 1815 1815 + ) 1816 1816 + .await 1817 1817 + .unwrap(); 1818 1818 + 1819 1819 + store 1820 1820 + .persist( 1821 1821 + "r", 1822 1822 + &[Effect::PersistMetadata { 1823 1823 + item_id: TrackId::nil(), 1824 1824 + url: "https://example.com/ok".into(), 1825 1825 + title: "Resolved Title".into(), 1826 1826 + duration: "1:23".into(), 1827 1827 + thumbnail: None, 1828 1828 + source: "ytdlp".into(), 1829 1829 + }], 1830 1830 + ) 1831 1831 + .await 1832 1832 + .unwrap(); 1833 1833 + 1834 1834 + let snap = store.load("r").await.unwrap().unwrap(); 1835 1835 + let entry = &snap.playlist[0]; 1836 1836 + assert_eq!(entry.title, "Resolved Title"); 1837 1837 + assert!(!entry.pending); 1838 1838 + assert!( 1839 1839 + entry.failed.is_none(), 1840 1840 + "resolved entry must not have a `failed` reason: {entry:?}", 1841 1841 + ); 1711 1842 } 1712 1843 1713 1844 #[tokio::test]