refactor(deps): replace tokio Mutex/join!, add NetworkErrorKind enum, remove reqwest from error types

This commit is contained in:
teernisse
2026-03-06 15:22:37 -05:00
parent 3a4fc96558
commit 4d41d74ea7
4 changed files with 52 additions and 21 deletions

View File

@@ -1,6 +1,15 @@
use serde::Serialize; use serde::Serialize;
use thiserror::Error; use thiserror::Error;
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum NetworkErrorKind {
Timeout,
ConnectionRefused,
DnsResolution,
Tls,
Other,
}
#[derive(Debug, Clone, Copy, PartialEq, Eq)] #[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum ErrorCode { pub enum ErrorCode {
ConfigNotFound, ConfigNotFound,
@@ -99,8 +108,8 @@ pub enum LoreError {
#[error("Cannot connect to GitLab at {base_url}")] #[error("Cannot connect to GitLab at {base_url}")]
GitLabNetworkError { GitLabNetworkError {
base_url: String, base_url: String,
#[source] kind: NetworkErrorKind,
source: Option<reqwest::Error>, detail: Option<String>,
}, },
#[error( #[error(
@@ -122,9 +131,6 @@ pub enum LoreError {
#[error("Database error: {0}")] #[error("Database error: {0}")]
Database(#[from] rusqlite::Error), Database(#[from] rusqlite::Error),
#[error("HTTP error: {0}")]
Http(#[from] reqwest::Error),
#[error("JSON error: {0}")] #[error("JSON error: {0}")]
Json(#[from] serde_json::Error), Json(#[from] serde_json::Error),
@@ -146,8 +152,7 @@ pub enum LoreError {
#[error("Cannot connect to Ollama at {base_url}. Is it running?")] #[error("Cannot connect to Ollama at {base_url}. Is it running?")]
OllamaUnavailable { OllamaUnavailable {
base_url: String, base_url: String,
#[source] detail: Option<String>,
source: Option<reqwest::Error>,
}, },
#[error("Ollama model '{model}' not found. Run: ollama pull {model}")] #[error("Ollama model '{model}' not found. Run: ollama pull {model}")]
@@ -187,7 +192,6 @@ impl LoreError {
ErrorCode::DatabaseError ErrorCode::DatabaseError
} }
} }
Self::Http(_) => ErrorCode::GitLabNetworkError,
Self::Json(_) => ErrorCode::InternalError, Self::Json(_) => ErrorCode::InternalError,
Self::Io(_) => ErrorCode::IoError, Self::Io(_) => ErrorCode::IoError,
Self::Transform(_) => ErrorCode::TransformError, Self::Transform(_) => ErrorCode::TransformError,
@@ -238,7 +242,6 @@ impl LoreError {
Some("Check database file permissions.\n\n Example:\n lore doctor") Some("Check database file permissions.\n\n Example:\n lore doctor")
} }
} }
Self::Http(_) => Some("Check network connection"),
Self::NotFound(_) => { Self::NotFound(_) => {
Some("Verify the entity exists.\n\n Example:\n lore issues\n lore mrs") Some("Verify the entity exists.\n\n Example:\n lore issues\n lore mrs")
} }

View File

@@ -75,7 +75,7 @@ impl OllamaClient {
.await .await
.map_err(|e| LoreError::OllamaUnavailable { .map_err(|e| LoreError::OllamaUnavailable {
base_url: self.config.base_url.clone(), base_url: self.config.base_url.clone(),
source: Some(e), detail: Some(format!("{e:?}")),
})?; })?;
let tags: TagsResponse = let tags: TagsResponse =
@@ -84,7 +84,7 @@ impl OllamaClient {
.await .await
.map_err(|e| LoreError::OllamaUnavailable { .map_err(|e| LoreError::OllamaUnavailable {
base_url: self.config.base_url.clone(), base_url: self.config.base_url.clone(),
source: Some(e), detail: Some(format!("{e:?}")),
})?; })?;
let model_found = tags.models.iter().any(|m| { let model_found = tags.models.iter().any(|m| {
@@ -116,7 +116,7 @@ impl OllamaClient {
.await .await
.map_err(|e| LoreError::OllamaUnavailable { .map_err(|e| LoreError::OllamaUnavailable {
base_url: self.config.base_url.clone(), base_url: self.config.base_url.clone(),
source: Some(e), detail: Some(format!("{e:?}")),
})?; })?;
let status = response.status(); let status = response.status();

View File

@@ -5,8 +5,8 @@ use reqwest::header::{ACCEPT, HeaderMap, HeaderValue};
use reqwest::{Client, Response, StatusCode}; use reqwest::{Client, Response, StatusCode};
use std::pin::Pin; use std::pin::Pin;
use std::sync::Arc; use std::sync::Arc;
use std::sync::Mutex;
use std::time::{Duration, Instant}; use std::time::{Duration, Instant};
use tokio::sync::Mutex;
use tokio::time::sleep; use tokio::time::sleep;
use tracing::{debug, warn}; use tracing::{debug, warn};
@@ -131,7 +131,16 @@ impl GitLabClient {
let mut last_response = None; let mut last_response = None;
for attempt in 0..=Self::MAX_RETRIES { for attempt in 0..=Self::MAX_RETRIES {
let delay = self.rate_limiter.lock().await.check_delay(); // SAFETY: std::sync::Mutex blocks the executor thread while held. This is safe
// because the critical section is a single Instant::now() comparison with no I/O.
// If async work is ever added inside the lock, switch to an async-aware lock.
let delay = {
let mut limiter = self
.rate_limiter
.lock()
.unwrap_or_else(|poisoned| poisoned.into_inner());
limiter.check_delay()
};
if let Some(d) = delay { if let Some(d) = delay {
sleep(d).await; sleep(d).await;
} }
@@ -146,7 +155,8 @@ impl GitLabClient {
.await .await
.map_err(|e| LoreError::GitLabNetworkError { .map_err(|e| LoreError::GitLabNetworkError {
base_url: self.base_url.clone(), base_url: self.base_url.clone(),
source: Some(e), kind: crate::core::error::NetworkErrorKind::Other,
detail: Some(format!("{e:?}")),
})?; })?;
if response.status() == StatusCode::TOO_MANY_REQUESTS && attempt < Self::MAX_RETRIES { if response.status() == StatusCode::TOO_MANY_REQUESTS && attempt < Self::MAX_RETRIES {
@@ -197,7 +207,14 @@ impl GitLabClient {
} }
status if status.is_success() => { status if status.is_success() => {
let text = response.text().await?; let text = response
.text()
.await
.map_err(|e| LoreError::GitLabNetworkError {
base_url: self.base_url.clone(),
kind: crate::core::error::NetworkErrorKind::Other,
detail: Some(format!("{e:?}")),
})?;
serde_json::from_str(&text).map_err(|e| { serde_json::from_str(&text).map_err(|e| {
let preview = if text.len() > 500 { let preview = if text.len() > 500 {
&text[..text.floor_char_boundary(500)] &text[..text.floor_char_boundary(500)]
@@ -516,7 +533,16 @@ impl GitLabClient {
let mut last_response = None; let mut last_response = None;
for attempt in 0..=Self::MAX_RETRIES { for attempt in 0..=Self::MAX_RETRIES {
let delay = self.rate_limiter.lock().await.check_delay(); // SAFETY: std::sync::Mutex blocks the executor thread while held. This is safe
// because the critical section is a single Instant::now() comparison with no I/O.
// If async work is ever added inside the lock, switch to an async-aware lock.
let delay = {
let mut limiter = self
.rate_limiter
.lock()
.unwrap_or_else(|poisoned| poisoned.into_inner());
limiter.check_delay()
};
if let Some(d) = delay { if let Some(d) = delay {
sleep(d).await; sleep(d).await;
} }
@@ -532,7 +558,8 @@ impl GitLabClient {
.await .await
.map_err(|e| LoreError::GitLabNetworkError { .map_err(|e| LoreError::GitLabNetworkError {
base_url: self.base_url.clone(), base_url: self.base_url.clone(),
source: Some(e), kind: crate::core::error::NetworkErrorKind::Other,
detail: Some(format!("{e:?}")),
})?; })?;
if response.status() == StatusCode::TOO_MANY_REQUESTS && attempt < Self::MAX_RETRIES { if response.status() == StatusCode::TOO_MANY_REQUESTS && attempt < Self::MAX_RETRIES {
@@ -726,14 +753,14 @@ impl GitLabClient {
)> { )> {
let (state_res, label_res, milestone_res) = match entity_type { let (state_res, label_res, milestone_res) = match entity_type {
"issue" => { "issue" => {
tokio::join!( futures::join!(
self.fetch_issue_state_events(gitlab_project_id, iid), self.fetch_issue_state_events(gitlab_project_id, iid),
self.fetch_issue_label_events(gitlab_project_id, iid), self.fetch_issue_label_events(gitlab_project_id, iid),
self.fetch_issue_milestone_events(gitlab_project_id, iid), self.fetch_issue_milestone_events(gitlab_project_id, iid),
) )
} }
"merge_request" => { "merge_request" => {
tokio::join!( futures::join!(
self.fetch_mr_state_events(gitlab_project_id, iid), self.fetch_mr_state_events(gitlab_project_id, iid),
self.fetch_mr_label_events(gitlab_project_id, iid), self.fetch_mr_label_events(gitlab_project_id, iid),
self.fetch_mr_milestone_events(gitlab_project_id, iid), self.fetch_mr_milestone_events(gitlab_project_id, iid),

View File

@@ -55,7 +55,8 @@ impl GraphqlClient {
.await .await
.map_err(|e| LoreError::GitLabNetworkError { .map_err(|e| LoreError::GitLabNetworkError {
base_url: self.base_url.clone(), base_url: self.base_url.clone(),
source: Some(e), kind: crate::core::error::NetworkErrorKind::Other,
detail: Some(format!("{e:?}")),
})?; })?;
let status = response.status(); let status = response.status();