From 3a700f0020afca40b80cfe97f32e583f7ac9c1cf Mon Sep 17 00:00:00 2001 From: "./lemon.sh" Date: Fri, 8 Aug 2025 16:32:20 +0200 Subject: [PATCH] fix: add fallback logic for CDN urls (#1524) --- CHANGELOG.md | 4 +++ audio/src/fetch/mod.rs | 57 +++++++++++++++++++++++++++++------------- core/src/cdn_url.rs | 29 +++++++++++++++++++++ core/src/spclient.rs | 16 ++++++------ 4 files changed, 81 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fdac644d..560de2b9 100644 --- a/CHANGELOG.md +++ b/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` 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 diff --git a/audio/src/fetch/mod.rs b/audio/src/fetch/mod.rs index d1617106..414ce3cf 100644 --- a/audio/src/fetch/mod.rs +++ b/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 { 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 - // larger than the audio file we're going to stream later on. This is OK; requesting - // `Content-Range` > `Content-Length` will return the complete file with status code - // 206 Partial Content. - let mut streamer = - session - .spclient() - .stream_from_cdn(&cdn_url, 0, minimum_download_size)?; + let mut response_streamer_url = None; + let urls = cdn_url.try_get_urls()?; + for url in &urls { + // When the audio file is really small, this `download_size` may turn out to be + // larger than the audio file we're going to stream later on. This is OK; requesting + // `Content-Range` > `Content-Length` will return the complete file with status code + // 206 Partial Content. + 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. - // The remainder of that chunk with possibly also a response body is then - // further processed in `audio_file_fetch`. - let response = streamer.next().await.ok_or(AudioFileError::NoData)??; + // 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 + // further processed in `audio_file_fetch`. + 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(), diff --git a/core/src/cdn_url.rs b/core/src/cdn_url.rs index b6b7ecfc..44f4488f 100644 --- a/core/src/cdn_url.rs +++ b/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, 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 for MaybeExpiringUrls { diff --git a/core/src/spclient.rs b/core/src/spclient.rs index 4377d406..87a6098c 100644 --- a/core/src/spclient.rs +++ b/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( &self, - cdn_url: &CdnUrl, + cdn_url: U, offset: usize, length: usize, - ) -> Result, Error> { - let url = cdn_url.try_get_url()?; + ) -> Result, Error> + where + U: TryInto, + >::Error: Into, + { let req = Request::builder() .method(&Method::GET) - .uri(url) + .uri(cdn_url) .header( RANGE, HeaderValue::from_str(&format!("bytes={}-{}", offset, offset + length - 1))?,