fix: add fallback logic for CDN urls (#1524)

CHANGELOG.md

@@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - [core] MSRV is now 1.81 (breaking)
 - [core] AP connect and handshake have a combined 5 second timeout.
+- [core] `stream_from_cdn` now accepts the URL as a `&str` instead of `CdnUrl` (breaking)
 - [connect] Replaced `has_volume_ctrl` with `disable_volume` in `ConnectConfig` (breaking)
 - [connect] Changed `initial_volume` from `Option<u16>` to `u16` in `ConnectConfig` (breaking)
 - [connect] Replaced `SpircLoadCommand` with `LoadRequest`, `LoadRequestOptions` and `LoadContextOptions` (breaking)
@@ -30,6 +31,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - [playback] Add `track` field to `PlayerEvent::RepeatChanged` (breaking)
 - [playback] Add `PlayerEvent::PositionChanged` event to notify about the current playback position
 - [core] Add `request_with_options` and `request_with_protobuf_and_options` to `SpClient`
+- [core] Add `try_get_urls` to `CdnUrl`
 - [oauth] Add `OAuthClient` and `OAuthClientBuilder` structs to achieve a more customizable login process
 
 ### Fixed
@@ -48,10 +50,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - [connect] Handle transfer of playback with empty "uri" field
 - [connect] Correctly apply playing/paused state when transferring playback
 - [player] Saturate invalid seek positions to track duration
+- [audio] Fall back to other URLs in case of a failure when downloading from CDN
 
 ### Deprecated
 
 - [oauth] `get_access_token()` function marked for deprecation
+- [core] `try_get_url()` function marked for deprecation
 
 ### Removed
 

audio/src/fetch/mod.rs

@@ -306,7 +306,7 @@ struct AudioFileDownloadStatus {
 }
 
 struct AudioFileShared {
-    cdn_url: CdnUrl,
+    cdn_url: String,
     file_size: usize,
     bytes_per_second: usize,
     cond: Condvar,
@@ -426,25 +426,46 @@ impl AudioFileStreaming {
     ) -> Result<AudioFileStreaming, Error> {
         let cdn_url = CdnUrl::new(file_id).resolve_audio(&session).await?;
 
-        if let Ok(url) = cdn_url.try_get_url() {
-            trace!("Streaming from {}", url);
-        }
-
         let minimum_download_size = AudioFetchParams::get().minimum_download_size;
 
-        // When the audio file is really small, this `download_size` may turn out to be
+        let mut response_streamer_url = None;
-        // larger than the audio file we're going to stream later on. This is OK; requesting
+        let urls = cdn_url.try_get_urls()?;
-        // `Content-Range` > `Content-Length` will return the complete file with status code
+        for url in &urls {
-        // 206 Partial Content.
+            // When the audio file is really small, this `download_size` may turn out to be
-        let mut streamer =
+            // larger than the audio file we're going to stream later on. This is OK; requesting
-            session
+            // `Content-Range` > `Content-Length` will return the complete file with status code
-                .spclient()
+            // 206 Partial Content.
-                .stream_from_cdn(&cdn_url, 0, minimum_download_size)?;
+            let mut streamer =
+                session
+                    .spclient()
+                    .stream_from_cdn(*url, 0, minimum_download_size)?;
 
-        // Get the first chunk with the headers to get the file size.
+            // Get the first chunk with the headers to get the file size.
-        // The remainder of that chunk with possibly also a response body is then
+            // The remainder of that chunk with possibly also a response body is then
-        // further processed in `audio_file_fetch`.
+            // further processed in `audio_file_fetch`.
-        let response = streamer.next().await.ok_or(AudioFileError::NoData)??;
+            let streamer_result = tokio::time::timeout(Duration::from_secs(10), streamer.next())
+                .await
+                .map_err(|_| AudioFileError::WaitTimeout.into())
+                .and_then(|x| x.ok_or_else(|| AudioFileError::NoData.into()))
+                .and_then(|x| x.map_err(Error::from));
+
+            match streamer_result {
+                Ok(r) => {
+                    response_streamer_url = Some((r, streamer, url));
+                    break;
+                }
+                Err(e) => warn!("Fetching {url} failed with error {e:?}, trying next"),
+            }
+        }
+
+        let Some((response, streamer, url)) = response_streamer_url else {
+            return Err(Error::unavailable(format!(
+                "{} URLs failed, none left to try",
+                urls.len()
+            )));
+        };
+
+        trace!("Streaming from {}", url);
 
         let code = response.status();
         if code != StatusCode::PARTIAL_CONTENT {
@@ -473,7 +494,7 @@ impl AudioFileStreaming {
         };
 
         let shared = Arc::new(AudioFileShared {
-            cdn_url,
+            cdn_url: url.to_string(),
             file_size,
             bytes_per_second,
             cond: Condvar::new(),

core/src/cdn_url.rs

@@ -78,6 +78,7 @@ impl CdnUrl {
         Ok(cdn_url)
     }
 
+    #[deprecated = "This function only returns the first valid URL. Use try_get_urls instead, which allows for fallback logic."]
     pub fn try_get_url(&self) -> Result<&str, Error> {
         if self.urls.is_empty() {
             return Err(CdnUrlError::Unresolved.into());
@@ -95,6 +96,34 @@ impl CdnUrl {
             Err(CdnUrlError::Expired.into())
         }
     }
+
+    pub fn try_get_urls(&self) -> Result<Vec<&str>, Error> {
+        if self.urls.is_empty() {
+            return Err(CdnUrlError::Unresolved.into());
+        }
+
+        let now = Date::now_utc();
+        let urls: Vec<&str> = self
+            .urls
+            .iter()
+            .filter_map(|MaybeExpiringUrl(url, expiry)| match *expiry {
+                Some(expiry) => {
+                    if now < expiry {
+                        Some(url.as_str())
+                    } else {
+                        None
+                    }
+                }
+                None => Some(url.as_str()),
+            })
+            .collect();
+
+        if urls.is_empty() {
+            Err(CdnUrlError::Expired.into())
+        } else {
+            Ok(urls)
+        }
+    }
 }
 
 impl TryFrom<CdnUrlMessage> for MaybeExpiringUrls {

core/src/spclient.rs

@@ -6,7 +6,6 @@ use std::{
 use crate::config::{os_version, OS};
 use crate::{
     apresolve::SocketAddress,
-    cdn_url::CdnUrl,
     config::SessionConfig,
     error::ErrorKind,
     protocol::{
@@ -27,7 +26,7 @@ use crate::{
 use bytes::Bytes;
 use data_encoding::HEXUPPER_PERMISSIVE;
 use futures_util::future::IntoStream;
-use http::header::HeaderValue;
+use http::{header::HeaderValue, Uri};
 use hyper::{
     header::{HeaderName, ACCEPT, AUTHORIZATION, CONTENT_TYPE, RANGE},
     HeaderMap, Method, Request,
@@ -730,16 +729,19 @@ impl SpClient {
         self.request(&Method::GET, &endpoint, None, None).await
     }
 
-    pub fn stream_from_cdn(
+    pub fn stream_from_cdn<U>(
         &self,
-        cdn_url: &CdnUrl,
+        cdn_url: U,
         offset: usize,
         length: usize,
-    ) -> Result<IntoStream<ResponseFuture>, Error> {
+    ) -> Result<IntoStream<ResponseFuture>, Error>
-        let url = cdn_url.try_get_url()?;
+    where
+        U: TryInto<Uri>,
+        <U as TryInto<Uri>>::Error: Into<http::Error>,
+    {
         let req = Request::builder()
             .method(&Method::GET)
-            .uri(url)
+            .uri(cdn_url)
             .header(
                 RANGE,
                 HeaderValue::from_str(&format!("bytes={}-{}", offset, offset + length - 1))?,