fix(music-link): give the Tidal/proxy fast path a timeout and a shared client
Both requests built bare reqwest clients with no timeout; a stalled connection parked the fast path forever and the Odesli fallback, which only runs afterwards, never got its turn. One OnceLock client with the same 10 s / 5 s budget as odesli.rs. cc
This commit is contained in:
3 files changed
+66
-12
No files matched your search
Generated
+1
@@ -4316,6 +4316,7 @@ dependencies = [
|
|||||||
"qbz-models",
|
"qbz-models",
|
||||||
"qbz-qobuz",
|
"qbz-qobuz",
|
||||||
"reqwest",
|
"reqwest",
|
||||||
|
"rustls",
|
||||||
"serde",
|
"serde",
|
||||||
"serde_json",
|
"serde_json",
|
||||||
"thiserror 2.0.20",
|
"thiserror 2.0.20",
|
||||||
|
|||||||
@@ -25,3 +25,8 @@ log = { workspace = true }
|
|||||||
|
|
||||||
# Error handling
|
# Error handling
|
||||||
thiserror = { workspace = true }
|
thiserror = { workspace = true }
|
||||||
|
|
||||||
|
[dev-dependencies]
|
||||||
|
# The timeout test builds a reqwest client, which panics with "No provider
|
||||||
|
# set" until a process-level rustls CryptoProvider exists (same as qbz-qobuz).
|
||||||
|
rustls = { version = "0.23", features = ["aws_lc_rs"] }
|
||||||
@@ -10,6 +10,30 @@ use crate::detection::{spotify, MusicProvider};
|
|||||||
/// URL hardcoded in the `src-tauri` original. No API keys are embedded here.
|
/// URL hardcoded in the `src-tauri` original. No API keys are embedded here.
|
||||||
const QBZ_PROXY_BASE: &str = "https://qbz-api-proxy.blitzkriegfc.workers.dev";
|
const QBZ_PROXY_BASE: &str = "https://qbz-api-proxy.blitzkriegfc.workers.dev";
|
||||||
|
|
||||||
|
/// Same budget as `odesli.rs` (`REQUEST_TIMEOUT`). A bare
|
||||||
|
/// `reqwest::Client::new()` has NO timeout, so a black-holed connection hung
|
||||||
|
/// the fast path forever — and since `lib.rs` only falls back to Odesli after
|
||||||
|
/// the fast path returns, the link never resolved.
|
||||||
|
const REQUEST_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10);
|
||||||
|
const CONNECT_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(5);
|
||||||
|
|
||||||
|
/// Build the fast-path client with a total deadline of `total`.
|
||||||
|
pub(crate) fn build_client(total: std::time::Duration) -> reqwest::Client {
|
||||||
|
reqwest::Client::builder()
|
||||||
|
.timeout(total)
|
||||||
|
.connect_timeout(CONNECT_TIMEOUT.min(total))
|
||||||
|
.user_agent("QBZ/1.0.0")
|
||||||
|
.build()
|
||||||
|
.expect("reqwest client")
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The one shared client (connection pool + timeouts) for the direct
|
||||||
|
/// platform calls and the QBZ proxy.
|
||||||
|
fn client() -> &'static reqwest::Client {
|
||||||
|
static CLIENT: std::sync::OnceLock<reqwest::Client> = std::sync::OnceLock::new();
|
||||||
|
CLIENT.get_or_init(|| build_client(REQUEST_TIMEOUT))
|
||||||
|
}
|
||||||
|
|
||||||
/// Try to get title+artist directly from the platform API.
|
/// Try to get title+artist directly from the platform API.
|
||||||
/// Returns None if the platform isn't supported or the request fails.
|
/// Returns None if the platform isn't supported or the request fails.
|
||||||
pub(crate) async fn try_direct_platform_metadata(
|
pub(crate) async fn try_direct_platform_metadata(
|
||||||
@@ -98,7 +122,7 @@ async fn try_tidal_metadata(url: &str, is_track: bool) -> Option<(String, String
|
|||||||
);
|
);
|
||||||
|
|
||||||
log::debug!("Link resolver: Tidal direct API: {}", api_url);
|
log::debug!("Link resolver: Tidal direct API: {}", api_url);
|
||||||
let data: serde_json::Value = reqwest::Client::new()
|
let data: serde_json::Value = client()
|
||||||
.get(&api_url)
|
.get(&api_url)
|
||||||
.header("Authorization", format!("Bearer {}", token))
|
.header("Authorization", format!("Bearer {}", token))
|
||||||
.send()
|
.send()
|
||||||
@@ -134,17 +158,7 @@ async fn try_tidal_metadata(url: &str, is_track: bool) -> Option<(String, String
|
|||||||
/// Get an OAuth token from the QBZ proxy for the given platform.
|
/// Get an OAuth token from the QBZ proxy for the given platform.
|
||||||
async fn get_proxy_token(platform: &str) -> Option<String> {
|
async fn get_proxy_token(platform: &str) -> Option<String> {
|
||||||
let url = format!("{}/{}/token", QBZ_PROXY_BASE, platform);
|
let url = format!("{}/{}/token", QBZ_PROXY_BASE, platform);
|
||||||
let data: serde_json::Value = reqwest::Client::builder()
|
let data: serde_json::Value = client()
|
||||||
.default_headers({
|
|
||||||
let mut h = reqwest::header::HeaderMap::new();
|
|
||||||
h.insert(
|
|
||||||
reqwest::header::USER_AGENT,
|
|
||||||
reqwest::header::HeaderValue::from_static("QBZ/1.0.0"),
|
|
||||||
);
|
|
||||||
h
|
|
||||||
})
|
|
||||||
.build()
|
|
||||||
.ok()?
|
|
||||||
.get(&url)
|
.get(&url)
|
||||||
.send()
|
.send()
|
||||||
.await
|
.await
|
||||||
@@ -156,3 +170,37 @@ async fn get_proxy_token(platform: &str) -> Option<String> {
|
|||||||
.and_then(|v| v.as_str())
|
.and_then(|v| v.as_str())
|
||||||
.map(|v| v.to_string())
|
.map(|v| v.to_string())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use std::time::{Duration, Instant};
|
||||||
|
|
||||||
|
/// A silent server (accepts, never answers) must not park the fast path:
|
||||||
|
/// the Odesli fallback only runs after it returns.
|
||||||
|
#[tokio::test]
|
||||||
|
async fn fast_path_client_gives_up_on_a_silent_server() {
|
||||||
|
let _ = rustls::crypto::aws_lc_rs::default_provider().install_default();
|
||||||
|
let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
|
||||||
|
let url = format!("http://{}/tidal/token", listener.local_addr().unwrap());
|
||||||
|
let hold = tokio::spawn(async move {
|
||||||
|
let (socket, _) = listener.accept().await.unwrap();
|
||||||
|
tokio::time::sleep(Duration::from_secs(30)).await;
|
||||||
|
drop(socket);
|
||||||
|
});
|
||||||
|
|
||||||
|
let started = Instant::now();
|
||||||
|
let err = super::build_client(Duration::from_millis(400))
|
||||||
|
.get(&url)
|
||||||
|
.send()
|
||||||
|
.await
|
||||||
|
.unwrap_err();
|
||||||
|
|
||||||
|
assert!(err.is_timeout(), "expected a timeout, got: {err}");
|
||||||
|
assert!(
|
||||||
|
started.elapsed() < Duration::from_secs(5),
|
||||||
|
"took {:?}",
|
||||||
|
started.elapsed()
|
||||||
|
);
|
||||||
|
hold.abort();
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in new issue
Block a user