Skip to content

Commit c3f0681

Browse files
greysquirr3lgreysquirr3l
committed
fix: resolve clippy warnings for Rust 1.94
- pool.rs: restructure acquire_inner fast path to drop mutex guard before spawning disposal tasks; allow remaining false-positive lint - rest_api.rs: replace if-let/else chain with match (option_if_let_else) - cloudflare_crawl.rs: make new()/with_config() fallible, remove Default impl (expect_used prohibition)
1 parent d3c0fc3 commit c3f0681

File tree

3 files changed

+71
-53
lines changed

3 files changed

+71
-53
lines changed

‎crates/stygian-browser/src/pool.rs‎

Lines changed: 54 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -167,8 +167,12 @@ impl BrowserPool {
167167
let now = Instant::now();
168168
let active = eviction_active.load(Ordering::Relaxed);
169169

170-
let total_idle: usize =
171-
guard.shared.len() + guard.scoped.values().map(|q| q.len()).sum::<usize>();
170+
let total_idle: usize = guard.shared.len()
171+
+ guard
172+
.scoped
173+
.values()
174+
.map(std::collections::VecDeque::len)
175+
.sum::<usize>();
172176
let evict_count = if active > eviction_min {
173177
(active - eviction_min).min(total_idle)
174178
} else {
@@ -220,6 +224,7 @@ impl BrowserPool {
220224
// Remove empty scoped queues
221225
guard.scoped.retain(|_, q| !q.is_empty());
222226

227+
// Explicitly drop the guard as soon as possible to avoid holding the lock longer than needed
223228
drop(guard);
224229

225230
if evicted > 0 {
@@ -319,44 +324,56 @@ impl BrowserPool {
319324

320325
/// Shared acquisition logic. `context_id = None` reads from the shared
321326
/// queue; `Some(id)` reads from the scoped queue for that context.
327+
#[allow(clippy::significant_drop_tightening)] // guard scope is already minimal
322328
async fn acquire_inner(self: &Arc<Self>, context_id: Option<&str>) -> Result<BrowserHandle> {
323329
let acquire_timeout = self.config.pool.acquire_timeout;
324330
let active = self.active_count.load(Ordering::Relaxed);
325331
let max = self.max_size;
326332
let ctx_owned: Option<String> = context_id.map(String::from);
327333

328334
// Fast path: try idle queue first
329-
{
335+
let fast_result = {
330336
let mut guard = self.inner.lock().await;
331337
let queue = match context_id {
332338
Some(id) => guard.scoped.get_mut(id),
333339
None => Some(&mut guard.shared),
334340
};
341+
let mut healthy: Option<BrowserInstance> = None;
342+
let mut unhealthy: Vec<BrowserInstance> = Vec::new();
335343
if let Some(queue) = queue {
336344
while let Some(entry) = queue.pop_front() {
337-
if entry.instance.is_healthy_cached() {
338-
self.active_count.fetch_add(0, Ordering::Relaxed); // already counted
339-
debug!(
340-
context = context_id.unwrap_or("shared"),
341-
"Reusing idle browser (uptime={:?})",
342-
entry.instance.uptime()
343-
);
344-
return Ok(BrowserHandle::new(
345-
entry.instance,
346-
Arc::clone(self),
347-
ctx_owned,
348-
));
345+
if healthy.is_none() && entry.instance.is_healthy_cached() {
346+
healthy = Some(entry.instance);
347+
} else if !entry.instance.is_healthy_cached() {
348+
unhealthy.push(entry.instance);
349+
} else {
350+
// Healthy but we already found one — push back.
351+
queue.push_front(entry);
352+
break;
349353
}
350-
// Unhealthy idle entry — dispose in background
351-
#[cfg(feature = "metrics")]
352-
crate::metrics::METRICS.record_crash();
353-
let active_count = self.active_count.clone();
354-
tokio::spawn(async move {
355-
let _ = entry.instance.shutdown().await;
356-
active_count.fetch_sub(1, Ordering::Relaxed);
357-
});
358354
}
359355
}
356+
(healthy, unhealthy)
357+
};
358+
359+
// Dispose unhealthy entries outside the lock
360+
for instance in fast_result.1 {
361+
#[cfg(feature = "metrics")]
362+
crate::metrics::METRICS.record_crash();
363+
let active_count = self.active_count.clone();
364+
tokio::spawn(async move {
365+
let _ = instance.shutdown().await;
366+
active_count.fetch_sub(1, Ordering::Relaxed);
367+
});
368+
}
369+
370+
if let Some(instance) = fast_result.0 {
371+
debug!(
372+
context = context_id.unwrap_or("shared"),
373+
"Reusing idle browser (uptime={:?})",
374+
instance.uptime()
375+
);
376+
return Ok(BrowserHandle::new(instance, Arc::clone(self), ctx_owned));
360377
}
361378

362379
// Slow path: launch new or wait
@@ -429,8 +446,12 @@ impl BrowserPool {
429446
// Health-check before returning to idle queue
430447
if instance.is_healthy_cached() {
431448
let mut guard = self.inner.lock().await;
432-
let total_idle: usize =
433-
guard.shared.len() + guard.scoped.values().map(|q| q.len()).sum::<usize>();
449+
let total_idle: usize = guard.shared.len()
450+
+ guard
451+
.scoped
452+
.values()
453+
.map(std::collections::VecDeque::len)
454+
.sum::<usize>();
434455
if total_idle < self.max_size {
435456
let queue = match context_id {
436457
Some(id) => guard.scoped.entry(id.to_owned()).or_default(),
@@ -867,14 +888,18 @@ mod tests {
867888

868889
#[test]
869890
fn pool_inner_total_idle_calculation() {
891+
fn total_idle(inner: &PoolInner) -> usize {
892+
inner.shared.len()
893+
+ inner
894+
.scoped
895+
.values()
896+
.map(std::collections::VecDeque::len)
897+
.sum::<usize>()
898+
}
870899
let mut inner = PoolInner {
871900
shared: std::collections::VecDeque::new(),
872901
scoped: std::collections::HashMap::new(),
873902
};
874-
// Total idle across shared + scoped
875-
fn total_idle(inner: &PoolInner) -> usize {
876-
inner.shared.len() + inner.scoped.values().map(|q| q.len()).sum::<usize>()
877-
}
878903
assert_eq!(total_idle(&inner), 0);
879904

880905
// Add entries to scoped queues (without real BrowserInstance, just check sizes)

‎crates/stygian-graph/src/adapters/cloudflare_crawl.rs‎

Lines changed: 13 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
//! use serde_json::json;
2323
//!
2424
//! # tokio::runtime::Runtime::new().unwrap().block_on(async {
25-
//! let adapter = CloudflareCrawlAdapter::new();
25+
//! let adapter = CloudflareCrawlAdapter::new().unwrap();
2626
//! let input = ServiceInput {
2727
//! url: "https://docs.example.com".to_string(),
2828
//! params: json!({
@@ -112,7 +112,7 @@ impl Default for CloudflareCrawlConfig {
112112
/// use serde_json::json;
113113
///
114114
/// # tokio::runtime::Runtime::new().unwrap().block_on(async {
115-
/// let adapter = CloudflareCrawlAdapter::new();
115+
/// let adapter = CloudflareCrawlAdapter::new().unwrap();
116116
/// let input = ServiceInput {
117117
/// url: "https://docs.example.com".to_string(),
118118
/// params: json!({
@@ -136,9 +136,9 @@ impl CloudflareCrawlAdapter {
136136
///
137137
/// ```
138138
/// use stygian_graph::adapters::cloudflare_crawl::CloudflareCrawlAdapter;
139-
/// let adapter = CloudflareCrawlAdapter::new();
139+
/// let adapter = CloudflareCrawlAdapter::new().unwrap();
140140
/// ```
141-
pub fn new() -> Self {
141+
pub fn new() -> Result<Self> {
142142
Self::with_config(CloudflareCrawlConfig::default())
143143
}
144144

@@ -155,14 +155,14 @@ impl CloudflareCrawlAdapter {
155155
/// let adapter = CloudflareCrawlAdapter::with_config(CloudflareCrawlConfig {
156156
/// poll_interval: Duration::from_secs(5),
157157
/// job_timeout: Duration::from_secs(600),
158-
/// });
158+
/// }).unwrap();
159159
/// ```
160-
pub fn with_config(config: CloudflareCrawlConfig) -> Self {
160+
pub fn with_config(config: CloudflareCrawlConfig) -> Result<Self> {
161161
let client = Client::builder()
162162
.timeout(Duration::from_secs(60))
163163
.build()
164-
.expect("reqwest TLS backend failed to initialize");
165-
Self { client, config }
164+
.map_err(|e| ServiceError::Unavailable(format!("reqwest client init failed: {e}")))?;
165+
Ok(Self { client, config })
166166
}
167167

168168
// ── Private helpers ──────────────────────────────────────────────────────
@@ -326,12 +326,6 @@ impl CloudflareCrawlAdapter {
326326
}
327327
}
328328

329-
impl Default for CloudflareCrawlAdapter {
330-
fn default() -> Self {
331-
Self::new()
332-
}
333-
}
334-
335329
#[async_trait]
336330
impl ScrapingService for CloudflareCrawlAdapter {
337331
/// Submit a crawl job to Cloudflare, poll until complete, and return
@@ -357,7 +351,7 @@ impl ScrapingService for CloudflareCrawlAdapter {
357351
/// use serde_json::json;
358352
///
359353
/// # tokio::runtime::Runtime::new().unwrap().block_on(async {
360-
/// let adapter = CloudflareCrawlAdapter::new();
354+
/// let adapter = CloudflareCrawlAdapter::new().unwrap();
361355
/// let input = ServiceInput {
362356
/// url: "https://docs.example.com".to_string(),
363357
/// params: json!({
@@ -524,7 +518,7 @@ mod tests {
524518

525519
#[tokio::test]
526520
async fn execute_missing_account_id_returns_error() {
527-
let adapter = CloudflareCrawlAdapter::new();
521+
let adapter = CloudflareCrawlAdapter::new().unwrap();
528522
let input = ServiceInput {
529523
url: "https://example.com".to_string(),
530524
params: json!({ "api_token": "tok" }),
@@ -534,7 +528,7 @@ mod tests {
534528

535529
#[tokio::test]
536530
async fn execute_missing_api_token_returns_error() {
537-
let adapter = CloudflareCrawlAdapter::new();
531+
let adapter = CloudflareCrawlAdapter::new().unwrap();
538532
let input = ServiceInput {
539533
url: "https://example.com".to_string(),
540534
params: json!({ "account_id": "acc" }),
@@ -559,7 +553,8 @@ mod tests {
559553
let adapter = CloudflareCrawlAdapter::with_config(CloudflareCrawlConfig {
560554
poll_interval: Duration::from_secs(3),
561555
job_timeout: Duration::from_secs(120),
562-
});
556+
})
557+
.expect("test: client init");
563558

564559
let input = ServiceInput {
565560
url: "https://example.com".to_string(),

‎crates/stygian-graph/src/adapters/rest_api.rs‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -331,12 +331,10 @@ impl RestApiAdapter {
331331
.unwrap_or_default();
332332

333333
// body_raw takes precedence over body (raw string vs structured JSON).
334-
let body = if let Some(raw) = params["body_raw"].as_str().filter(|s| !s.is_empty()) {
335-
Some(RequestBody::Raw(raw.to_owned()))
336-
} else if !params["body"].is_null() {
337-
Some(RequestBody::Json(params["body"].clone()))
338-
} else {
339-
None
334+
let body = match params["body_raw"].as_str().filter(|s| !s.is_empty()) {
335+
Some(raw) => Some(RequestBody::Raw(raw.to_owned())),
336+
None if !params["body"].is_null() => Some(RequestBody::Json(params["body"].clone())),
337+
None => None,
340338
};
341339

342340
let accept = params["accept"]

0 commit comments

Comments
 (0)