From b37a6f2624859245525a0d91760ce9572acd9358 Mon Sep 17 00:00:00 2001 From: Ilia Agafonov Date: Fri, 8 May 2026 04:22:41 +0700 Subject: [PATCH 1/2] :sparkles: Add tiered logging and HTTP request helper - New `logging` module with `warn!`/`info!`/`debug!` macros driven by the existing `verbose: u8` flag (0=silent, 1=warn, 2=info, 3=debug). Zero-overhead at default verbosity: macros short-circuit before `format!` and the HTTP helper skips timing/body reads. - Live mode buffers logs in a 1000-line ring buffer and flushes them to stderr via an RAII `CaptureGuard` after the alt-screen is restored, so log lines never corrupt the UI. - New `weather::http::get_json` centralizes HTTP send + JSON parse with request observability (INFO: provider/op/status/duration; DEBUG: full URL and truncated response body). All seven providers and the OpenUV helper migrated onto it. - Replace ad-hoc `eprintln!` in `app.rs` and `openuv.rs` with `warn!`. Add deduped missing-translation warnings (one per key per process) and cache hit/miss + enrich-action info logs. - Fix latent bug: a transient OpenUV failure no longer aborts the weather fetch; the error is logged as a warning and `uv_index` stays `None`. `enrich` now returns `()` since it can no longer fail. - README and CHANGELOG updated with `-v`/`-vv`/`-vvv` usage and the new user-visible behavior. --- CHANGELOG.md | 20 ++ README.md | 22 +- src/app.rs | 9 +- src/cache.rs | 2 + src/display/translations.rs | 53 +++- src/live.rs | 10 +- src/logging.rs | 291 ++++++++++++++++++ src/main.rs | 1 + src/weather/enrich.rs | 55 ++-- src/weather/http.rs | 203 ++++++++++++ src/weather/mod.rs | 3 +- src/weather/openuv.rs | 32 +- src/weather/providers/open_meteo.rs | 35 +-- src/weather/providers/open_weather_map.rs | 17 +- src/weather/providers/tomorrow_io.rs | 10 +- src/weather/providers/weather_api.rs | 10 +- src/weather/providers/weather_bit.rs | 16 +- src/weather/providers/world_weather_online.rs | 10 +- src/weather/providers/yr.rs | 17 +- 19 files changed, 715 insertions(+), 101 deletions(-) create mode 100644 src/logging.rs create mode 100644 src/weather/http.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 1853aac..8f82722 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,26 @@ All notable changes to this project will be documented in this file. See [Keep a Changelog](https://keepachangelog.com/) for details. This project adheres to [Semantic Versioning](https://semver.org/). +## [Unreleased] + +### Added + +- Tiered diagnostic logging via `-v` / `-vv` / `-vvv` (or `verbose = 0..=3` in the config file). All logs go to stderr, + so they don't pollute `--format=json` output piped through `jq` or similar tools. + - `-v` shows warnings: when a provider fails and we fall back to the next one, when OpenUV is unavailable, or when + a translation key is missing. + - `-vv` adds one-line summaries of every HTTP request (provider, operation, status, latency) and geocoding cache + hits/misses. URLs and API keys are not shown at this level. + - `-vvv` adds full request URLs and truncated response bodies for debugging. URLs at this level include API keys — + don't share captured output verbatim. +- In live mode, log lines produced inside the alternate screen are buffered and flushed to stderr after you exit, so + the UI stays clean during the session and you still see what happened on quit. + +### Fixed + +- A transient OpenUV failure no longer aborts the weather fetch. UV index is silently omitted (with a `-v` warning if + enabled) and the rest of the data still displays normally. + ## [0.5.0] - 2026-05-06 ### Added diff --git a/README.md b/README.md index 109f0da..48c5a88 100644 --- a/README.md +++ b/README.md @@ -151,7 +151,18 @@ use_geocoding_cache = false #### Verbosity level -Verbosity level can be set with `verbose` option (0 - no verbose, 1 - show errors) +`rustormy` writes diagnostic logs to stderr. The level is set via `verbose` in the config file or by repeating `-v` +on the command line: + +| Level | Flag | What you'll see | +|-------|---------|------------------------------------------------------------------------------| +| 0 | (none) | Silent (only fatal errors). | +| 1 | `-v` | Warnings: provider fallback, OpenUV failures, missing translations. | +| 2 | `-vv` | + Per-request summaries (provider, operation, HTTP status, latency) and cache hits/misses. | +| 3 | `-vvv` | + Full request URLs (including API keys) and truncated response bodies. | + +Logs always go to stderr, so `2>/dev/null` keeps stdout clean for piping. In live mode, logs produced inside the +alternate screen are buffered and flushed to stderr after you exit, so the UI stays clean during the session. ```toml verbose = 0 @@ -302,6 +313,15 @@ Options: ![Night mode icons](.github/assets/night.png) ![JSON output: `rustormy -c Ajax -o json`](.github/assets/json.png) +### Diagnostic output + +```sh +rustormy -c London -v # warnings only (e.g. when a provider fails and we fall back) +rustormy -c London -vv # one-line per-request summaries and cache hits/misses +rustormy -c London -vvv # full request URLs and response bodies (for debugging only) +rustormy -c London -o json -vv 2>/dev/null | jq . # logs go to stderr; stdout stays pure JSON +``` + ## License This project is licensed under the MIT License. See the [LICENSE](LICENSE) file for details. diff --git a/src/app.rs b/src/app.rs index 7abc3de..b7d2203 100644 --- a/src/app.rs +++ b/src/app.rs @@ -17,6 +17,9 @@ pub struct App { impl App { pub fn new() -> Result { let mut config = Config::new(Cli::new())?; + if !config.live_mode() { + crate::logging::init(config.verbose(), config.format().use_colors); + } let client = Client::builder() .user_agent(concat!("rustormy/", env!("CARGO_PKG_VERSION"))) .timeout(Duration::from_secs(config.connect_timeout())) @@ -43,15 +46,13 @@ impl App { loop { match self.provider.get_weather(&self.client, &self.config) { Ok(mut weather) => { - enrich(&mut weather, &self.client, &self.config)?; + enrich(&mut weather, &self.client, &self.config); return Ok(weather); } Err(error) => match error { RustormyError::ApiReturnedError(_) | RustormyError::HttpRequestFailed(_) => { let p: Provider = (&self.provider).into(); - if self.config.verbose() >= 1 { - eprintln!("Provider {p:?} failed: {error:?}"); - } + crate::warn!("Provider {p:?} failed: {error}"); let Some(next) = self.config.take_next_provider() else { return Err(error); }; diff --git a/src/cache.rs b/src/cache.rs index 9e98174..aebd8b3 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -39,8 +39,10 @@ pub fn get_cached_location( if cache_path.exists() { let location: Location = serde_json::from_reader(File::open(cache_path)?)?; + crate::info!("cache hit \"{city}\""); Ok(Some(location)) } else { + crate::info!("cache miss \"{city}\""); Ok(None) } } diff --git a/src/display/translations.rs b/src/display/translations.rs index 8b2fae7..bc614b6 100644 --- a/src/display/translations.rs +++ b/src/display/translations.rs @@ -1,6 +1,6 @@ use crate::models::Language; -use std::collections::HashMap; -use std::sync::LazyLock; +use std::collections::{HashMap, HashSet}; +use std::sync::{LazyLock, Mutex, OnceLock}; macro_rules! translations { ($($key:expr => { @@ -415,10 +415,53 @@ static TRANSLATIONS: LazyLock &'static Mutex> { + static SET: OnceLock>> = OnceLock::new(); + SET.get_or_init(|| Mutex::new(HashSet::new())) +} + pub fn ll(lang: Language, key: &'static str) -> &'static str { - TRANSLATIONS + if let Some(translated) = TRANSLATIONS .get(lang.code()) .and_then(|translations| translations.get(key)) - // TODO: Add logging for missing translations - .unwrap_or(&key) + { + return translated; + } + let mut seen = match missing_keys_set().lock() { + Ok(g) => g, + Err(p) => p.into_inner(), + }; + if seen.insert(key) { + crate::warn!("missing translation for key: {key}"); + } + key +} + +#[cfg(test)] +pub(crate) fn missing_keys_seen(key: &'static str) -> bool { + missing_keys_set() + .lock() + .map(|s| s.contains(key)) + .unwrap_or(false) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn missing_keys_are_logged_only_once() { + let key = "test_missing_key_dedup"; + let _ = ll(Language::English, key); + let _ = ll(Language::English, key); + let _ = ll(Language::English, key); + assert!(super::missing_keys_seen(key)); + } + + #[test] + fn known_keys_do_not_register_as_missing() { + let key = "Clear"; + let _ = ll(Language::English, key); + assert!(!super::missing_keys_seen(key)); + } } diff --git a/src/live.rs b/src/live.rs index fd9953d..60426bd 100644 --- a/src/live.rs +++ b/src/live.rs @@ -66,12 +66,20 @@ pub fn render( } pub fn run(app: &mut App) -> Result<(), RustormyError> { + let level = app.config().verbose(); + let use_colors = app.config().format().use_colors; + let _capture = crate::logging::init_with_capture(level, use_colors); + // First fetch runs before entering the alt screen so the user's // existing terminal contents stay visible during the initial API call. let mut weather = app.fetch_with_fallback()?; let mut now = Local::now(); - let _guard = TerminalGuard::enter()?; + // Drain any logs produced during the first fetch directly to stderr + // so they remain in the user's scrollback after the alt-screen exits. + crate::logging::flush_capture(); + + let _terminal = TerminalGuard::enter()?; let mut stdout = io::stdout(); loop { diff --git a/src/logging.rs b/src/logging.rs new file mode 100644 index 0000000..503c14c --- /dev/null +++ b/src/logging.rs @@ -0,0 +1,291 @@ +use std::collections::VecDeque; +use std::io::{self, Write}; +use std::sync::{Mutex, OnceLock}; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Level { + Warn, + Info, + Debug, +} + +impl Level { + pub fn label(self) -> &'static str { + match self { + Self::Warn => "WARN", + Self::Info => "INFO", + Self::Debug => "DEBUG", + } + } + + pub fn threshold(self) -> u8 { + match self { + Self::Warn => 1, + Self::Info => 2, + Self::Debug => 3, + } + } + + fn ansi_code(self) -> &'static str { + match self { + Self::Warn => "\x1b[33m", + Self::Info => "\x1b[36m", + Self::Debug => "\x1b[90m", + } + } +} + +pub struct Capture { + lines: VecDeque, + capacity: usize, + dropped: usize, +} + +pub struct DrainedCapture { + pub lines: Vec, + pub dropped: usize, +} + +impl Capture { + pub fn new(capacity: usize) -> Self { + Self { + lines: VecDeque::with_capacity(capacity), + capacity, + dropped: 0, + } + } + + pub fn push(&mut self, line: String) { + if self.lines.len() == self.capacity { + self.lines.pop_front(); + self.dropped += 1; + } + self.lines.push_back(line); + } + + pub fn drain(&mut self) -> DrainedCapture { + let lines = self.lines.drain(..).collect(); + let dropped = self.dropped; + self.dropped = 0; + DrainedCapture { lines, dropped } + } +} + +pub struct LoggerState { + pub level: u8, + pub use_colors: bool, + pub capture: Option>, +} + +static LOGGER: OnceLock = OnceLock::new(); + +const CAPTURE_CAPACITY: usize = 1000; + +pub fn state() -> Option<&'static LoggerState> { + LOGGER.get() +} + +pub fn init(level: u8, use_colors: bool) { + let _ = LOGGER.set(LoggerState { + level: clamp_level(level), + use_colors, + capture: None, + }); +} + +pub fn init_with_capture(level: u8, use_colors: bool) -> CaptureGuard { + let _ = LOGGER.set(LoggerState { + level: clamp_level(level), + use_colors, + capture: Some(Mutex::new(Capture::new(CAPTURE_CAPACITY))), + }); + CaptureGuard { _private: () } +} + +pub struct CaptureGuard { + _private: (), +} + +impl Drop for CaptureGuard { + fn drop(&mut self) { + flush_capture(); + } +} + +pub fn flush_capture() { + let Some(state) = LOGGER.get() else { return }; + let Some(mutex) = state.capture.as_ref() else { + return; + }; + let drained = match mutex.lock() { + Ok(mut guard) => guard.drain(), + Err(poisoned) => poisoned.into_inner().drain(), + }; + let mut stderr = io::stderr().lock(); + if drained.dropped > 0 { + let header = format_line( + state, + Level::Info, + &format!( + "(capture buffer overflow: {} earlier lines dropped)", + drained.dropped + ), + ); + let _ = writeln!(stderr, "{header}"); + } + for line in drained.lines { + let _ = writeln!(stderr, "{line}"); + } +} + +pub(crate) fn clamp_level(level: u8) -> u8 { + level.min(Level::Debug.threshold()) +} + +pub(crate) fn level_passes(state_level: u8, line_level: Level) -> bool { + state_level >= line_level.threshold() +} + +pub(crate) fn format_line(state: &LoggerState, level: Level, message: &str) -> String { + if state.use_colors { + format!( + "{}[{}]\x1b[0m {}", + level.ansi_code(), + level.label(), + message + ) + } else { + format!("[{}] {}", level.label(), message) + } +} + +pub(crate) fn emit(state: &'static LoggerState, level: Level, message: &str) { + let line = format_line(state, level, message); + if let Some(mutex) = state.capture.as_ref() + && let Ok(mut guard) = mutex.lock() + { + guard.push(line); + return; + } + let _ = writeln!(io::stderr().lock(), "{line}"); +} + +#[macro_export] +macro_rules! warn { + ($($arg:tt)*) => {{ + if let Some(state) = $crate::logging::state() + && $crate::logging::level_passes(state.level, $crate::logging::Level::Warn) + { + $crate::logging::emit(state, $crate::logging::Level::Warn, &format!($($arg)*)); + } + }}; +} + +#[macro_export] +macro_rules! info { + ($($arg:tt)*) => {{ + if let Some(state) = $crate::logging::state() + && $crate::logging::level_passes(state.level, $crate::logging::Level::Info) + { + $crate::logging::emit(state, $crate::logging::Level::Info, &format!($($arg)*)); + } + }}; +} + +#[macro_export] +macro_rules! debug { + ($($arg:tt)*) => {{ + if let Some(state) = $crate::logging::state() + && $crate::logging::level_passes(state.level, $crate::logging::Level::Debug) + { + $crate::logging::emit(state, $crate::logging::Level::Debug, &format!($($arg)*)); + } + }}; +} + +#[cfg(test)] +mod tests { + use super::*; + + fn render(level_threshold: u8, line_level: Level, message: &str, use_colors: bool) -> String { + let state = LoggerState { + level: level_threshold, + use_colors, + capture: None, + }; + format_line(&state, line_level, message) + } + + #[test] + fn formats_warn_without_colors() { + let out = render(1, Level::Warn, "boom", false); + assert_eq!(out, "[WARN] boom"); + } + + #[test] + fn formats_info_without_colors() { + let out = render(2, Level::Info, "ok", false); + assert_eq!(out, "[INFO] ok"); + } + + #[test] + fn colored_warn_contains_ansi() { + let out = render(1, Level::Warn, "boom", true); + assert!(out.contains("\x1b["), "expected ANSI escape, got {out:?}"); + assert!(out.contains("[WARN]")); + assert!(out.contains("boom")); + } + + #[test] + fn level_passes_returns_true_for_equal_or_lower() { + assert!(level_passes(1, Level::Warn)); + assert!(!level_passes(1, Level::Info)); + assert!(!level_passes(0, Level::Warn)); + assert!(level_passes(3, Level::Debug)); + assert!(!level_passes(2, Level::Debug)); + } + + #[test] + fn level_clamping_in_init_state() { + assert_eq!(clamp_level(0), 0); + assert_eq!(clamp_level(1), 1); + assert_eq!(clamp_level(3), 3); + assert_eq!(clamp_level(99), 3); + } + + #[test] + fn capture_records_lines_when_active() { + let mut cap = Capture::new(1000); + cap.push("[WARN] one".to_string()); + cap.push("[INFO] two".to_string()); + let drained = cap.drain(); + assert_eq!(drained.lines, vec!["[WARN] one", "[INFO] two"]); + assert_eq!(drained.dropped, 0); + } + + #[test] + fn capture_drops_oldest_when_full() { + let mut cap = Capture::new(3); + cap.push("a".to_string()); + cap.push("b".to_string()); + cap.push("c".to_string()); + cap.push("d".to_string()); + cap.push("e".to_string()); + let drained = cap.drain(); + assert_eq!(drained.lines, vec!["c", "d", "e"]); + assert_eq!(drained.dropped, 2); + } + + #[test] + fn capture_drain_resets_state() { + let mut cap = Capture::new(2); + cap.push("x".to_string()); + cap.push("y".to_string()); + cap.push("z".to_string()); + let _ = cap.drain(); + cap.push("after".to_string()); + let drained = cap.drain(); + assert_eq!(drained.lines, vec!["after"]); + assert_eq!(drained.dropped, 0); + } +} diff --git a/src/main.rs b/src/main.rs index 1043e44..e0e6644 100644 --- a/src/main.rs +++ b/src/main.rs @@ -4,6 +4,7 @@ mod config; mod display; mod errors; mod live; +mod logging; mod models; #[cfg(test)] mod tests; diff --git a/src/weather/enrich.rs b/src/weather/enrich.rs index 9b6c464..e4bfd9d 100644 --- a/src/weather/enrich.rs +++ b/src/weather/enrich.rs @@ -1,23 +1,33 @@ use crate::config::Config; -use crate::errors::RustormyError; use crate::models::Weather; use crate::weather::openuv::get_uv_index; use crate::weather::sun; use chrono::Utc; use reqwest::blocking::Client; -pub fn enrich( - weather: &mut Weather, - client: &Client, - config: &Config, -) -> Result<(), RustormyError> { +pub fn enrich(weather: &mut Weather, client: &Client, config: &Config) { if weather.is_day.is_none() { weather.is_day = Some(sun::is_daytime(&weather.location, Utc::now())); + crate::info!("enrich: filled is_day from solar altitude"); } - if weather.uv_index.is_none() && !config.api_keys().open_uv.is_empty() { - weather.uv_index = get_uv_index(client, config, &weather.location)?; + if weather.uv_index.is_none() { + if config.api_keys().open_uv.is_empty() { + crate::info!("enrich: OpenUV skipped (no api key)"); + } else { + match get_uv_index(client, config, &weather.location) { + Ok(Some(uv)) => { + weather.uv_index = Some(uv); + crate::info!("enrich: fetched UV index from OpenUV"); + } + Ok(None) => { + crate::info!("enrich: OpenUV returned no UV value"); + } + Err(error) => { + crate::warn!("Failed to fetch UV index: {error}"); + } + } + } } - Ok(()) } #[cfg(test)] @@ -53,7 +63,7 @@ mod tests { assert!(weather.is_day.is_none()); let client = Client::new(); let config = Config::default(); - enrich(&mut weather, &client, &config).expect("enrich should succeed"); + enrich(&mut weather, &client, &config); assert!(weather.is_day.is_some(), "is_day should be populated"); } @@ -63,7 +73,7 @@ mod tests { weather.is_day = Some(false); let client = Client::new(); let config = Config::default(); - enrich(&mut weather, &client, &config).expect("enrich should succeed"); + enrich(&mut weather, &client, &config); assert_eq!( weather.is_day, Some(false), @@ -77,7 +87,7 @@ mod tests { weather.is_day = Some(true); let client = Client::new(); let config = Config::default(); - enrich(&mut weather, &client, &config).expect("enrich should succeed"); + enrich(&mut weather, &client, &config); assert_eq!( weather.is_day, Some(true), @@ -90,22 +100,31 @@ mod tests { let mut weather = make_weather(); let client = Client::new(); let config = Config::default(); - enrich(&mut weather, &client, &config).expect("enrich should succeed"); + enrich(&mut weather, &client, &config); assert_eq!(weather.uv_index, None, "no UV when key is empty"); } + #[test] + fn openuv_failure_does_not_break_enrich() { + let mut weather = make_weather(); + let client = Client::builder() + .timeout(std::time::Duration::from_millis(50)) + .build() + .unwrap(); + let mut config = Config::default(); + config.api_keys_mut().open_uv = "definitely-not-a-real-key".to_string(); + enrich(&mut weather, &client, &config); + assert!(weather.uv_index.is_none()); + } + #[test] fn does_not_overwrite_uv_when_set() { let mut weather = make_weather(); weather.uv_index = Some(4.2); let client = Client::new(); let mut config = Config::default(); - // Set a non-empty OpenUV key so the empty-key short-circuit does - // not fire. The is_none() guard on uv_index should prevent the - // OpenUV HTTP call from happening — if it did fire, the fake key - // would produce an error and this test would fail. config.api_keys_mut().open_uv = "fake-key".to_string(); - enrich(&mut weather, &client, &config).expect("enrich should succeed"); + enrich(&mut weather, &client, &config); assert_eq!(weather.uv_index, Some(4.2)); } } diff --git a/src/weather/http.rs b/src/weather/http.rs new file mode 100644 index 0000000..7fef529 --- /dev/null +++ b/src/weather/http.rs @@ -0,0 +1,203 @@ +use crate::config::Config; +use crate::errors::RustormyError; +use crate::logging::{Level, level_passes, state}; +use crate::models::{Location, Provider}; +use reqwest::blocking::RequestBuilder; +use serde::de::DeserializeOwned; +use std::fmt; +use std::time::Instant; + +const BODY_LOG_LIMIT: usize = 1024; +const TRUNCATED_MARKER: &str = "…[truncated]"; + +/// Describes the HTTP call for logging purposes. +/// +/// Built once per call via the `geocode` / `weather_at` / `weather_for` / `uv_at` +/// constructors and passed to [`get_json`]. `Display` is only invoked when an +/// info-level log actually fires, so providers don't allocate label strings at +/// default verbosity. +#[derive(Debug, Clone, Copy)] +pub enum Op<'a> { + Geocode { + provider: Provider, + city: &'a str, + }, + WeatherAtCity { + provider: Provider, + city: &'a str, + }, + WeatherAtCoords { + provider: Provider, + lat: f64, + lon: f64, + }, + Uv { + lat: f64, + lon: f64, + }, +} + +impl<'a> Op<'a> { + pub const fn geocode(provider: Provider, city: &'a str) -> Self { + Self::Geocode { provider, city } + } + + pub fn weather_for(provider: Provider, config: &'a Config) -> Self { + debug_assert!( + config.coordinates().is_some() || config.city().is_some(), + "Op::weather_for called with neither coordinates nor city in config", + ); + if let Some((lat, lon)) = config.coordinates() { + Self::WeatherAtCoords { provider, lat, lon } + } else { + Self::WeatherAtCity { + provider, + city: config.city().unwrap_or(""), + } + } + } +} + +impl Op<'static> { + pub fn weather_at(provider: Provider, location: &Location) -> Self { + Self::WeatherAtCoords { + provider, + lat: location.latitude, + lon: location.longitude, + } + } + + pub fn uv_at(location: &Location) -> Self { + Self::Uv { + lat: location.latitude, + lon: location.longitude, + } + } +} + +impl fmt::Display for Op<'_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Geocode { provider, city } => { + write!(f, "provider:{provider:?} geocode \"{city}\"") + } + Self::WeatherAtCity { provider, city } => { + write!(f, "provider:{provider:?} weather \"{city}\"") + } + Self::WeatherAtCoords { provider, lat, lon } => { + write!(f, "provider:{provider:?} weather {lat:.2},{lon:.2}") + } + Self::Uv { lat, lon } => { + write!(f, "provider:OpenUV uv {lat:.2},{lon:.2}") + } + } + } +} + +pub fn get_json(request: RequestBuilder, op: Op<'_>) -> Result +where + T: DeserializeOwned, +{ + let level = state().map_or(0, |s| s.level); + + if level_passes(level, Level::Debug) + && let Some(url) = extract_url_for_log(&request) + { + crate::debug!("GET {url}"); + } + + let start = level_passes(level, Level::Info).then(Instant::now); + let response = request.send()?; + let status = response.status(); + + let parsed: T = if level_passes(level, Level::Debug) { + let text = response.text()?; + crate::debug!("response body: {}", truncate_for_log(&text, BODY_LOG_LIMIT)); + serde_json::from_str(&text)? + } else { + response.json()? + }; + + if let Some(s) = start { + let ms = s.elapsed().as_millis(); + crate::info!("{op} → {} in {}ms", status.as_u16(), ms); + } + + Ok(parsed) +} + +fn extract_url_for_log(request: &RequestBuilder) -> Option { + request + .try_clone() + .and_then(|c| c.build().ok()) + .map(|r| r.url().to_string()) +} + +fn truncate_for_log(s: &str, byte_limit: usize) -> String { + if s.len() <= byte_limit { + return s.to_string(); + } + let mut cut = byte_limit; + while cut > 0 && !s.is_char_boundary(cut) { + cut -= 1; + } + let mut out = String::with_capacity(cut + TRUNCATED_MARKER.len()); + out.push_str(&s[..cut]); + out.push_str(TRUNCATED_MARKER); + out +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn truncate_short_string_is_unchanged() { + assert_eq!(truncate_for_log("hello", 1024), "hello"); + } + + #[test] + fn truncate_long_string_marks_truncation() { + let s = "x".repeat(2048); + let out = truncate_for_log(&s, 1024); + assert!(out.starts_with(&"x".repeat(1024))); + assert!(out.ends_with("…[truncated]")); + assert_eq!(out.len(), 1024 + "…[truncated]".len()); + } + + #[test] + fn truncate_handles_utf8_boundaries() { + let s = "абв"; + let out = truncate_for_log(s, 3); + assert!(out.starts_with('а')); + assert!(out.ends_with("…[truncated]")); + } + + #[test] + fn op_display_geocode() { + let op = Op::geocode(Provider::OpenMeteo, "Lisbon"); + assert_eq!(op.to_string(), "provider:OpenMeteo geocode \"Lisbon\""); + } + + #[test] + fn op_display_weather_at() { + let location = Location { + name: "Lisbon".to_string(), + latitude: 38.7223, + longitude: -9.1393, + }; + let op = Op::weather_at(Provider::OpenMeteo, &location); + assert_eq!(op.to_string(), "provider:OpenMeteo weather 38.72,-9.14"); + } + + #[test] + fn op_display_uv_at() { + let location = Location { + name: "Lisbon".to_string(), + latitude: 38.7223, + longitude: -9.1393, + }; + let op = Op::uv_at(&location); + assert_eq!(op.to_string(), "provider:OpenUV uv 38.72,-9.14"); + } +} diff --git a/src/weather/mod.rs b/src/weather/mod.rs index be1f8a5..e066660 100644 --- a/src/weather/mod.rs +++ b/src/weather/mod.rs @@ -2,7 +2,6 @@ use crate::config::Config; use crate::errors::RustormyError; use crate::models::{Location, Weather}; use enum_dispatch::enum_dispatch; -pub use providers::GetWeatherProvider; use reqwest::blocking::Client; #[enum_dispatch] @@ -46,9 +45,11 @@ pub trait LookUpCity { } mod enrich; +pub(crate) mod http; mod openuv; mod providers; mod sun; pub mod tools; pub use enrich::enrich; +pub use providers::GetWeatherProvider; diff --git a/src/weather/openuv.rs b/src/weather/openuv.rs index b0d197d..ed6ac2d 100644 --- a/src/weather/openuv.rs +++ b/src/weather/openuv.rs @@ -1,3 +1,4 @@ +use super::http; use crate::config::Config; use crate::errors::RustormyError; use crate::models::Location; @@ -93,15 +94,11 @@ enum UvResponse { } impl UvResponse { - fn into_uv_index(self, config: &Config) -> Option { + fn into_uv_index(self) -> Option { match self { Self::Ok { result } => Some((result.uv * 10.).round() / 10.), Self::Err { message } => { - if config.verbose() >= 1 { - // TODO: proper logging - eprintln!("OpenUV API error: {message}"); - } - + crate::warn!("OpenUV API error: {message}"); None } } @@ -130,18 +127,19 @@ pub fn get_uv_index( return Ok(None); } let params = UvRequestParams::new(location); - let response = client - .get(OPEN_UV_API_URL) - .query(¶ms) - .header("x-access-token", &config.api_keys().open_uv) - .send()?; - Ok(response.json::()?.into_uv_index(config)) + let response: UvResponse = http::get_json( + client + .get(OPEN_UV_API_URL) + .query(¶ms) + .header("x-access-token", &config.api_keys().open_uv), + http::Op::uv_at(location), + )?; + Ok(response.into_uv_index()) } #[cfg(test)] mod tests { use super::UvResponse; - use crate::config::Config; const TEST_API_RESPONSE: &str = include_str!("../../tests/data/openuv_response.json"); @@ -149,18 +147,14 @@ mod tests { fn test_openuv_error_response_returns_no_uv_index() { let response: UvResponse = serde_json::from_str(r#"{"error":"Daily API quota exceeded."}"#) .expect("OpenUV error payload should deserialize"); - let config = Config::default(); - - assert_eq!(response.into_uv_index(&config), None); + assert_eq!(response.into_uv_index(), None); } #[test] fn test_openuv_valid_response_returns_uv_index() { let response: UvResponse = serde_json::from_str(TEST_API_RESPONSE).expect("Failed to parse JSON"); - let config = Config::default(); - - assert_eq!(response.into_uv_index(&config), Some(4.4)); + assert_eq!(response.into_uv_index(), Some(4.4)); } #[test] diff --git a/src/weather/providers/open_meteo.rs b/src/weather/providers/open_meteo.rs index 0215794..83b9af5 100644 --- a/src/weather/providers/open_meteo.rs +++ b/src/weather/providers/open_meteo.rs @@ -1,8 +1,8 @@ use crate::config::Config; use crate::display::translations::ll; use crate::errors::RustormyError; -use crate::models::{Language, Location, Units, Weather, WeatherConditionIcon}; -use crate::weather::{GetWeather, LookUpCity, tools}; +use crate::models::{Language, Location, Provider, Units, Weather, WeatherConditionIcon}; +use crate::weather::{GetWeather, LookUpCity, http, tools}; use reqwest::blocking::Client; use serde::{Deserialize, Serialize}; @@ -218,29 +218,26 @@ impl LookUpCity for OpenMeteo { let city = config.city().ok_or(RustormyError::NoLocationProvided)?; let request = GeocodingRequest::new(city, config.language()); - let response = client.get(GEO_API_URL).query(&request).send()?; - let data = response - .json::>()? - .into_result()?; - - let location = data - .into_location() - .ok_or_else(|| RustormyError::CityNotFound(city.to_string()))?; - - Ok(location) + let data = http::get_json::>( + client.get(GEO_API_URL).query(&request), + http::Op::geocode(Provider::OpenMeteo, city), + )? + .into_result()?; + + data.into_location() + .ok_or_else(|| RustormyError::CityNotFound(city.to_string())) } } impl GetWeather for OpenMeteo { fn get_weather(&self, client: &Client, config: &Config) -> Result { let location = self.get_location(client, config)?; - let response = client - .get(WEATHER_API_URL) - .query(&WeatherAPIRequest::new(&location, config)) - .send()?; - let data = response - .json::>()? - .into_result()?; + let request = WeatherAPIRequest::new(&location, config); + let data = http::get_json::>( + client.get(WEATHER_API_URL).query(&request), + http::Op::weather_at(Provider::OpenMeteo, &location), + )? + .into_result()?; Ok(data.into_weather(config, &location)) } diff --git a/src/weather/providers/open_weather_map.rs b/src/weather/providers/open_weather_map.rs index a073429..2f6750c 100644 --- a/src/weather/providers/open_weather_map.rs +++ b/src/weather/providers/open_weather_map.rs @@ -1,8 +1,8 @@ use crate::config::Config; use crate::display::translations::ll; use crate::errors::RustormyError; -use crate::models::{Language, Location, Units, Weather, WeatherConditionIcon}; -use crate::weather::{GetWeather, LookUpCity, tools}; +use crate::models::{Language, Location, Provider, Units, Weather, WeatherConditionIcon}; +use crate::weather::{GetWeather, LookUpCity, http, tools}; use capitalize::Capitalize; use reqwest::blocking::Client; @@ -198,8 +198,10 @@ impl LookUpCity for OpenWeatherMap { let city = config.city().ok_or(RustormyError::NoLocationProvided)?; let request = GeocodingApiRequest::new(city, config); - let response = client.get(GEO_API_URL).query(&request).send()?; - let data: GeocodingApiResponse = response.json()?; + let data: GeocodingApiResponse = http::get_json( + client.get(GEO_API_URL).query(&request), + http::Op::geocode(Provider::OpenWeatherMap, city), + )?; if let GeocodingApiResponse::Err { message } = data { return Err(RustormyError::ApiReturnedError(message)); @@ -217,9 +219,10 @@ impl GetWeather for OpenWeatherMap { let location = self.get_location(client, config)?; let request = WeatherAPIRequest::new(&location, config); - let response = client.get(WEATHER_API_URL).query(&request).send()?; - - let response: WeatherApiResponse = response.json()?; + let response: WeatherApiResponse = http::get_json( + client.get(WEATHER_API_URL).query(&request), + http::Op::weather_at(Provider::OpenWeatherMap, &location), + )?; match response { WeatherApiResponse::Err { message } => Err(RustormyError::ApiReturnedError(message)), WeatherApiResponse::Ok(data) => Ok(data.into_weather(config, &location)), diff --git a/src/weather/providers/tomorrow_io.rs b/src/weather/providers/tomorrow_io.rs index 697ebdb..5decb94 100644 --- a/src/weather/providers/tomorrow_io.rs +++ b/src/weather/providers/tomorrow_io.rs @@ -1,8 +1,8 @@ use crate::config::Config; use crate::display::translations::ll; use crate::errors::RustormyError; -use crate::models::{Language, Location, Units, Weather, WeatherConditionIcon}; -use crate::weather::GetWeather; +use crate::models::{Language, Location, Provider, Units, Weather, WeatherConditionIcon}; +use crate::weather::{GetWeather, http}; use reqwest::blocking::Client; const REALTIME_API_URL: &str = "https://api.tomorrow.io/v4/weather/realtime"; @@ -191,8 +191,10 @@ impl WeatherResponse { impl GetWeather for TomorrowIo { fn get_weather(&self, client: &Client, config: &Config) -> Result { let request = WeatherRequestParams::new(config); - let response = client.get(REALTIME_API_URL).query(&request).send()?; - let data: WeatherResponse = response.json()?; + let data: WeatherResponse = http::get_json( + client.get(REALTIME_API_URL).query(&request), + http::Op::weather_for(Provider::TomorrowIo, config), + )?; data.into_weather(config) } diff --git a/src/weather/providers/weather_api.rs b/src/weather/providers/weather_api.rs index e297108..30eb903 100644 --- a/src/weather/providers/weather_api.rs +++ b/src/weather/providers/weather_api.rs @@ -1,7 +1,7 @@ use crate::config::Config; use crate::errors::RustormyError; -use crate::models::{Location, Units, Weather, WeatherConditionIcon}; -use crate::weather::{GetWeather, tools}; +use crate::models::{Location, Provider, Units, Weather, WeatherConditionIcon}; +use crate::weather::{GetWeather, http, tools}; use reqwest::blocking::Client; const WEATHER_API_URL: &str = "https://api.weatherapi.com/v1/current.json"; @@ -229,8 +229,10 @@ struct WeatherApiCondition { impl GetWeather for WeatherApi { fn get_weather(&self, client: &Client, config: &Config) -> Result { let request = WeatherApiRequest::new(config); - let response = client.get(WEATHER_API_URL).query(&request).send()?; - let data: WeatherApiResponse = response.json()?; + let data: WeatherApiResponse = http::get_json( + client.get(WEATHER_API_URL).query(&request), + http::Op::weather_for(Provider::WeatherApi, config), + )?; match data { WeatherApiResponse::Ok(data) => Ok(data.into_weather(config)), WeatherApiResponse::Err { error } => Err(RustormyError::ApiReturnedError(format!( diff --git a/src/weather/providers/weather_bit.rs b/src/weather/providers/weather_bit.rs index 1cd7976..d7f3788 100644 --- a/src/weather/providers/weather_bit.rs +++ b/src/weather/providers/weather_bit.rs @@ -1,7 +1,7 @@ use crate::config::Config; use crate::errors::RustormyError; -use crate::models::{Location, Units, Weather, WeatherConditionIcon}; -use crate::weather::{GetWeather, LookUpCity, tools}; +use crate::models::{Location, Provider, Units, Weather, WeatherConditionIcon}; +use crate::weather::{GetWeather, LookUpCity, http, tools}; use reqwest::blocking::Client; const GEOCODING_API_URL: &str = "https://api.weatherbit.io/v2.0/geocode"; @@ -58,8 +58,10 @@ impl LookUpCity for WeatherBit { config: &Config, ) -> Result { let request = GeocodingApiRequest::new(config)?; - let response = client.get(GEOCODING_API_URL).query(&request).send()?; - let data: GeocodingApiResponse = response.json()?; + let data: GeocodingApiResponse = http::get_json( + client.get(GEOCODING_API_URL).query(&request), + http::Op::geocode(Provider::WeatherBit, config.city().unwrap_or("")), + )?; match data { GeocodingApiResponse::Err { error } => Err(RustormyError::ApiReturnedError(error)), GeocodingApiResponse::Ok(data) => Ok(data.into_location()), @@ -189,8 +191,10 @@ impl GetWeather for WeatherBit { fn get_weather(&self, client: &Client, config: &Config) -> Result { let location = self.get_location(client, config)?; let request = WeatherAPIRequest::new(&location, config); - let response = client.get(WEATHER_API_URL).query(&request).send()?; - let data: WeatherApiResponse = response.json()?; + let data: WeatherApiResponse = http::get_json( + client.get(WEATHER_API_URL).query(&request), + http::Op::weather_at(Provider::WeatherBit, &location), + )?; match data { WeatherApiResponse::Err { error } => Err(RustormyError::ApiReturnedError(error)), WeatherApiResponse::Ok { count, data } => { diff --git a/src/weather/providers/world_weather_online.rs b/src/weather/providers/world_weather_online.rs index 2e4da3b..40b281d 100644 --- a/src/weather/providers/world_weather_online.rs +++ b/src/weather/providers/world_weather_online.rs @@ -1,7 +1,7 @@ use crate::config::Config; use crate::errors::RustormyError; -use crate::models::{Language, Location, Units, Weather, WeatherConditionIcon}; -use crate::weather::{GetWeather, tools}; +use crate::models::{Language, Location, Provider, Units, Weather, WeatherConditionIcon}; +use crate::weather::{GetWeather, http, tools}; use reqwest::blocking::Client; const WWO_API_URL: &str = "https://api.worldweatheronline.com/premium/v1/weather.ashx"; @@ -291,8 +291,10 @@ pub struct WorldWeatherOnline {} impl GetWeather for WorldWeatherOnline { fn get_weather(&self, client: &Client, config: &Config) -> Result { let params = WwoRequestParams::new(config); - let response = client.get(WWO_API_URL).query(¶ms).send()?; - let response: WwoResponse = response.json()?; + let response: WwoResponse = http::get_json( + client.get(WWO_API_URL).query(¶ms), + http::Op::weather_for(Provider::WorldWeatherOnline, config), + )?; match response { WwoResponse::Ok { data } => data.into_weather(config), diff --git a/src/weather/providers/yr.rs b/src/weather/providers/yr.rs index e7fc7b5..44854d8 100644 --- a/src/weather/providers/yr.rs +++ b/src/weather/providers/yr.rs @@ -2,9 +2,9 @@ use super::open_meteo::OpenMeteo; use crate::config::Config; use crate::display::translations::ll; use crate::errors::RustormyError; -use crate::models::{Language, Location, Weather, WeatherConditionIcon}; +use crate::models::{Language, Location, Provider, Weather, WeatherConditionIcon}; use crate::weather::tools::{apparent_temperature, dew_point}; -use crate::weather::{GetWeather, LookUpCity}; +use crate::weather::{GetWeather, LookUpCity, http}; use reqwest::blocking::Client; use serde::{Deserialize, Serialize}; @@ -150,12 +150,13 @@ impl YrResponse { impl GetWeather for Yr { fn get_weather(&self, client: &Client, config: &Config) -> Result { let location = get_location(client, config)?; - let response = client - .get(YR_API_URL) - .query(&YrRequest::new(&location)) - .header("User-Agent", YR_USER_AGENT) - .send()?; - let data: YrResponse = response.json()?; + let data: YrResponse = http::get_json( + client + .get(YR_API_URL) + .query(&YrRequest::new(&location)) + .header("User-Agent", YR_USER_AGENT), + http::Op::weather_at(Provider::Yr, &location), + )?; data.into_weather(config, &location) } } From d2dcdc9438874b12981a1214022a32993fb4b8bb Mon Sep 17 00:00:00 2001 From: Ilia Agafonov Date: Fri, 8 May 2026 04:37:49 +0700 Subject: [PATCH 2/2] :white_check_mark: Apply clippy suggestions --- src/display/translations.rs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/display/translations.rs b/src/display/translations.rs index bc614b6..fd89ad3 100644 --- a/src/display/translations.rs +++ b/src/display/translations.rs @@ -427,10 +427,9 @@ pub fn ll(lang: Language, key: &'static str) -> &'static str { { return translated; } - let mut seen = match missing_keys_set().lock() { - Ok(g) => g, - Err(p) => p.into_inner(), - }; + let mut seen = missing_keys_set() + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); if seen.insert(key) { crate::warn!("missing translation for key: {key}"); } @@ -439,10 +438,7 @@ pub fn ll(lang: Language, key: &'static str) -> &'static str { #[cfg(test)] pub(crate) fn missing_keys_seen(key: &'static str) -> bool { - missing_keys_set() - .lock() - .map(|s| s.contains(key)) - .unwrap_or(false) + missing_keys_set().lock().is_ok_and(|s| s.contains(key)) } #[cfg(test)]