diff --git a/crates/buzz-admin/src/main.rs b/crates/buzz-admin/src/main.rs index bb30ddfae..07695c4d5 100644 --- a/crates/buzz-admin/src/main.rs +++ b/crates/buzz-admin/src/main.rs @@ -25,7 +25,7 @@ use std::sync::Arc; use anyhow::Result; use buzz_core::kind::KIND_NIP43_MEMBERSHIP_LIST; use buzz_core::tenant::{relay_url_authority, TenantContext}; -use buzz_db::{Db, DbConfig}; +use buzz_db::{Db, DbConfig, PgTimeout}; use buzz_pubsub::{EventTopic, PubSubManager}; use clap::{Parser, Subcommand}; use nostr::{EventBuilder, Keys, Kind, Tag}; @@ -420,8 +420,17 @@ async fn connect_member_services() -> Result<(Db, Arc, Keys)> { async fn connect_db() -> Result { let db_url = std::env::var("DATABASE_URL") .unwrap_or_else(|_| "postgres://buzz:buzz_dev@localhost:5432/buzz".to_string()); + // No runtime caps by default. `DbConfig`'s 30s/5s are sized for the relay's + // request-serving traffic on a shared pool — an operator CLI is neither, and + // a bulk repair that outlives 30s should finish, not get cancelled. The same + // env vars still apply for an operator who wants a bound here. let db = Db::new(&DbConfig { database_url: db_url, + statement_timeout: PgTimeout::from_env_or( + "BUZZ_DB_STATEMENT_TIMEOUT", + PgTimeout::disabled(), + ), + lock_timeout: PgTimeout::from_env_or("BUZZ_DB_LOCK_TIMEOUT", PgTimeout::disabled()), ..DbConfig::default() }) .await?; diff --git a/crates/buzz-db/src/lib.rs b/crates/buzz-db/src/lib.rs index 4019d97d3..9eaa21eda 100644 --- a/crates/buzz-db/src/lib.rs +++ b/crates/buzz-db/src/lib.rs @@ -73,26 +73,168 @@ pub const RUNTIME_LOCK_TIMEOUT: &str = "5s"; pub const TIMEOUT_DISABLED: &str = "0"; /// `statement_timeout` and `lock_timeout` are `int` GUCs measured in /// milliseconds, so Postgres refuses anything larger regardless of the unit it -/// is spelled with. Callers building a [`DbConfig`] from operator input must -/// range-check against this: [`apply_runtime_connection_timeouts`] runs on every -/// pooled connection, so an unusable value fails all database access. +/// is spelled with. Private on purpose: [`PgTimeout`] is the only way to build a +/// timeout, so no caller needs to range-check by hand. /// `pg_timeout_max_millis_matches_postgres` pins it to the live server. -pub const PG_TIMEOUT_MAX_MILLIS: u128 = i32::MAX as u128; +const PG_TIMEOUT_MAX_MILLIS: u128 = i32::MAX as u128; + +/// Why a string is not a timeout Postgres would accept. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum PgTimeoutError { + /// Not an integer with an optional `us`/`ms`/`s`/`min`/`h`/`d` unit. + Malformed, + /// Well-formed, but larger than Postgres can store in an `int` GUC. + OutOfRange { + /// The value in milliseconds, for the operator-facing message. + millis: u128, + }, +} + +impl std::fmt::Display for PgTimeoutError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Malformed => write!( + f, + "expected an integer with an optional us/ms/s/min/h/d unit" + ), + Self::OutOfRange { millis } => write!( + f, + "{millis}ms exceeds the {PG_TIMEOUT_MAX_MILLIS}ms Postgres stores in an int GUC" + ), + } + } +} + +impl std::error::Error for PgTimeoutError {} + +/// A Postgres timeout that the server is known to accept. +/// +/// Parsing is the only way to build one from operator input, so a [`DbConfig`] +/// cannot carry a value that would break +/// [`apply_runtime_connection_timeouts`] — which runs on every pooled +/// connection, and would therefore fail all database access. The stored +/// spelling is canonical: the magnitude followed by the lowercased unit, since +/// Postgres' unit names are case-sensitive. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct PgTimeout(String); + +impl PgTimeout { + /// The default runtime statement timeout ([`RUNTIME_STATEMENT_TIMEOUT`]). + pub fn statement_default() -> Self { + Self(RUNTIME_STATEMENT_TIMEOUT.to_string()) + } + + /// The default runtime lock timeout ([`RUNTIME_LOCK_TIMEOUT`]). + pub fn lock_default() -> Self { + Self(RUNTIME_LOCK_TIMEOUT.to_string()) + } + + /// No limit at all — what schema migrations and one-shot operator tools + /// run with. + pub fn disabled() -> Self { + Self(TIMEOUT_DISABLED.to_string()) + } + + /// The canonical spelling, ready to hand to Postgres. + pub fn as_str(&self) -> &str { + &self.0 + } + + /// Parse operator input, falling back to `default` with a warning. + /// + /// Refusing to start on a bad timeout would be the same outage the limits + /// prevent, so a malformed or out-of-range value keeps the documented + /// default instead. `None` and blank mean "unset". + pub fn parse_or(raw: Option<&str>, default: Self) -> Self { + let Some(candidate) = raw.map(str::trim).filter(|value| !value.is_empty()) else { + return default; + }; + match candidate.parse::() { + Ok(timeout) => timeout, + Err(error) => { + tracing::warn!( + value = candidate, + default = default.as_str(), + %error, + "ignoring unusable Postgres timeout" + ); + default + } + } + } + + /// [`PgTimeout::parse_or`] against an environment variable. + pub fn from_env_or(var: &str, default: Self) -> Self { + Self::parse_or(std::env::var(var).ok().as_deref(), default) + } +} + +#[cfg(test)] +impl PgTimeout { + /// Build without validation, so a test can hand Postgres a value + /// [`FromStr`](std::str::FromStr) refuses to produce and prove the server + /// rejects it. + pub(crate) fn unchecked(raw: impl Into) -> Self { + Self(raw.into()) + } +} + +impl std::fmt::Display for PgTimeout { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(&self.0) + } +} + +impl std::str::FromStr for PgTimeout { + type Err = PgTimeoutError; + + fn from_str(raw: &str) -> std::result::Result { + let candidate = raw.trim(); + let digits = candidate.chars().take_while(char::is_ascii_digit).count(); + let (magnitude, unit) = candidate.split_at(digits); + let unit = unit.trim().to_ascii_lowercase(); + + match pg_timeout_millis(magnitude, &unit) { + Some(millis) if millis <= PG_TIMEOUT_MAX_MILLIS => { + Ok(Self(format!("{magnitude}{unit}"))) + } + Some(millis) => Err(PgTimeoutError::OutOfRange { millis }), + None => Err(PgTimeoutError::Malformed), + } + } +} + +/// Convert a Postgres timeout magnitude and unit to milliseconds, mirroring the +/// rounding Postgres applies to sub-millisecond `us` values. `None` means the +/// spelling is not something Postgres would accept. +fn pg_timeout_millis(magnitude: &str, unit: &str) -> Option { + let value = magnitude.parse::().ok()?; + match unit { + // Round half up without the `value + 500` intermediate, which overflows + // for the top 500 representable microsecond values. + "us" => Some(value / 1_000 + u128::from(value % 1_000 >= 500)), + "" | "ms" => Some(value), + "s" => value.checked_mul(1_000), + "min" => value.checked_mul(60_000), + "h" => value.checked_mul(3_600_000), + "d" => value.checked_mul(86_400_000), + _ => None, + } +} /// Apply the runtime safety limits shared by writer, reader, audit, and search -/// pools. Values are Postgres interval strings (`"30s"`, `"500ms"`), with -/// [`TIMEOUT_DISABLED`] lifting a limit entirely. +/// pools. [`PgTimeout::disabled`] lifts a limit entirely. pub async fn apply_runtime_connection_timeouts( connection: &mut PgConnection, - statement_timeout: &str, - lock_timeout: &str, + statement_timeout: &PgTimeout, + lock_timeout: &PgTimeout, ) -> std::result::Result<(), sqlx::Error> { sqlx::query( "SELECT set_config('statement_timeout', $1, false), \ set_config('lock_timeout', $2, false)", ) - .bind(statement_timeout) - .bind(lock_timeout) + .bind(statement_timeout.as_str()) + .bind(lock_timeout.as_str()) .execute(connection) .await?; Ok(()) @@ -562,12 +704,12 @@ pub struct DbConfig { pub replica_read_max_age_ms: u64, /// Postgres `statement_timeout` applied to every runtime connection. An /// operator running a backfill or working an incident can widen this without - /// a code change; [`TIMEOUT_DISABLED`] removes the cap. - pub statement_timeout: String, + /// a code change; [`PgTimeout::disabled`] removes the cap. + pub statement_timeout: PgTimeout, /// Postgres `lock_timeout` applied to every runtime connection. Bounds /// heavyweight and row lock waits only — advisory-lock waits are bounded by /// [`Self::statement_timeout`] instead. - pub lock_timeout: String, + pub lock_timeout: PgTimeout, } impl Default for DbConfig { @@ -585,8 +727,8 @@ impl Default for DbConfig { max_lifetime_secs: 1800, idle_timeout_secs: 600, replica_read_max_age_ms: 0, - statement_timeout: RUNTIME_STATEMENT_TIMEOUT.to_string(), - lock_timeout: RUNTIME_LOCK_TIMEOUT.to_string(), + statement_timeout: PgTimeout::statement_default(), + lock_timeout: PgTimeout::lock_default(), } } } @@ -6595,8 +6737,8 @@ mod tests { database_url: scratch_url, max_connections: 2, min_connections: 2, - statement_timeout: TIGHT.to_string(), - lock_timeout: TIGHT.to_string(), + statement_timeout: TIGHT.parse().expect("test timeout literal"), + lock_timeout: TIGHT.parse().expect("test timeout literal"), ..DbConfig::default() }) .await @@ -8529,10 +8671,138 @@ mod tests { db.pool.close().await; } - /// [`PG_TIMEOUT_MAX_MILLIS`] is the bound callers range-check operator input - /// against, so it must be the server's real bound: too high and an accepted - /// value still fails every `after_connect`; too low and we reject settings - /// Postgres would have taken. + /// The built-in defaults bypass parsing, so prove they would survive it — + /// otherwise a typo in a literal ships a value Postgres refuses. + #[test] + fn builtin_timeout_defaults_are_parseable() { + for default in [ + PgTimeout::statement_default(), + PgTimeout::lock_default(), + PgTimeout::disabled(), + ] { + assert_eq!( + default.as_str().parse::().as_ref(), + Ok(&default), + "{default} must round-trip through the parser" + ); + } + } + + #[test] + fn pg_timeout_accepts_postgres_spellings_and_refuses_the_rest() { + let default = PgTimeout::statement_default(); + for (accepted, canonical) in [ + ("30s", "30s"), + ("500ms", "500ms"), + ("0", "0"), + ("45S", "45s"), + ("2MIN", "2min"), + (" 10 H ", "10h"), + ] { + assert_eq!( + PgTimeout::parse_or(Some(accepted), default.clone()).as_str(), + canonical, + "{accepted} is a valid Postgres timeout" + ); + } + + // A rejected value must not reach Postgres: `after_connect` would fail + // for every connection, which is worse than the documented default. + for rejected in ["", " ", "soon", "30 seconds", "s30", "-5s", "30s;DROP"] { + assert_eq!( + PgTimeout::parse_or(Some(rejected), default.clone()), + default, + "{rejected:?} must fall back to the default" + ); + } + + assert_eq!( + PgTimeout::parse_or(None, PgTimeout::lock_default()), + PgTimeout::lock_default() + ); + } + + #[test] + fn pg_timeout_refuses_magnitudes_postgres_cannot_store() { + let default = PgTimeout::statement_default(); + // Well-formed spellings whose millisecond value exceeds the int GUC + // range. Postgres rejects these in `set_config`, which would fail every + // pool's `after_connect`. + for rejected in [ + "2147483648", + "2147483648ms", + "2147484s", + "35792min", + "597h", + "25d", + // Wider than any integer type — must fall back, not overflow. + "999999999999999999999999999999999999999999d", + "99999999999999999999999999999999999999999999999999", + // Parses as u128, so unlike the two above it reaches the unit + // conversion — where rounding must not overflow on the way to the + // range check. + &format!("{}us", u128::MAX), + &format!("{}us", u128::MAX - 499), + ] { + assert_eq!( + PgTimeout::parse_or(Some(rejected), default.clone()), + default, + "{rejected:?} is out of range for Postgres and must fall back" + ); + } + + // The boundary itself, and the same instant in every unit, stay valid. + for accepted in [ + "2147483647", + "2147483647ms", + "2147483647000us", + "2147483s", + "35791min", + "596h", + "24d", + ] { + assert_eq!( + PgTimeout::parse_or(Some(accepted), default.clone()).as_str(), + accepted, + "{accepted} is inside the Postgres range" + ); + } + } + + /// Every spelling the parser emits must be usable by Postgres. A + /// parser-only assertion missed that Postgres rejects uppercase units even + /// though validation lowercases them. + #[tokio::test] + #[ignore = "requires Postgres"] + async fn pg_timeout_canonical_spellings_are_accepted_by_postgres() { + let mut connection = PgConnection::connect(&admin_url().await) + .await + .expect("connect test Postgres"); + + for (raw, canonical) in [ + ("500US", "500us"), + ("500MS", "500ms"), + ("2S", "2s"), + ("2MIN", "2min"), + ("2H", "2h"), + ("2D", "2d"), + ("0", "0"), + (" 10 S ", "10s"), + ] { + let parsed = PgTimeout::parse_or(Some(raw), PgTimeout::statement_default()); + assert_eq!(parsed.as_str(), canonical); + apply_runtime_connection_timeouts(&mut connection, &parsed, &parsed) + .await + .unwrap_or_else(|error| panic!("{raw:?} normalized to {parsed}: {error}")); + } + + connection.close().await.expect("close test Postgres"); + } + + /// `PG_TIMEOUT_MAX_MILLIS` is the bound [`PgTimeout`] range-checks operator + /// input against, so it must be the server's real bound: too high and an + /// accepted value still fails every `after_connect`; too low and we reject + /// settings Postgres would have taken. #[tokio::test] #[ignore = "requires Postgres"] async fn pg_timeout_max_millis_matches_postgres() { @@ -8540,7 +8810,7 @@ mod tests { .await .expect("connect"); - let boundary = PG_TIMEOUT_MAX_MILLIS.to_string(); + let boundary = PgTimeout::unchecked(PG_TIMEOUT_MAX_MILLIS.to_string()); apply_runtime_connection_timeouts(&mut conn, &boundary, &boundary) .await .expect("PG_TIMEOUT_MAX_MILLIS must be settable"); @@ -8548,13 +8818,10 @@ mod tests { // Same instant spelled in a coarser unit — the conversion the caller's // range check performs must land inside the range too. let in_seconds = (PG_TIMEOUT_MAX_MILLIS / 1_000).to_string(); - apply_runtime_connection_timeouts( - &mut conn, - &format!("{in_seconds}s"), - &format!("{in_seconds}s"), - ) - .await - .expect("the boundary in seconds must be settable"); + let boundary_seconds = PgTimeout::unchecked(format!("{in_seconds}s")); + apply_runtime_connection_timeouts(&mut conn, &boundary_seconds, &boundary_seconds) + .await + .expect("the boundary in seconds must be settable"); for over in [ (PG_TIMEOUT_MAX_MILLIS + 1).to_string(), @@ -8562,9 +8829,13 @@ mod tests { format!("{}s", PG_TIMEOUT_MAX_MILLIS / 1_000 + 1), "999999999999999999999999999999999999999999d".to_string(), ] { - let err = apply_runtime_connection_timeouts(&mut conn, &over, RUNTIME_LOCK_TIMEOUT) - .await - .expect_err(&format!("{over} must be rejected by Postgres")); + let err = apply_runtime_connection_timeouts( + &mut conn, + &PgTimeout::unchecked(over.clone()), + &PgTimeout::lock_default(), + ) + .await + .expect_err(&format!("{over} must be rejected by Postgres")); let code = match &err { sqlx::Error::Database(db) => db.code().map(|c| c.to_string()), other => panic!("expected a database error, got {other:?}"), diff --git a/crates/buzz-db/src/migration.rs b/crates/buzz-db/src/migration.rs index 051a1bc16..283c2af53 100644 --- a/crates/buzz-db/src/migration.rs +++ b/crates/buzz-db/src/migration.rs @@ -73,8 +73,8 @@ async fn migrate_on_exempt_connection(connection: &mut sqlx::PgConnection) -> Re async fn lift_runtime_timeouts(connection: &mut sqlx::PgConnection) -> Result<()> { crate::apply_runtime_connection_timeouts( connection, - crate::TIMEOUT_DISABLED, - crate::TIMEOUT_DISABLED, + &crate::PgTimeout::disabled(), + &crate::PgTimeout::disabled(), ) .await?; Ok(()) @@ -1174,6 +1174,13 @@ mod tests { .expect("create public schema"); } + /// Parse a literal the tests control, so a typo in one fails loudly here + /// rather than silently falling back to the runtime default. + fn tight_timeout(raw: &str) -> crate::PgTimeout { + raw.parse::() + .expect("test timeout literal must be valid") + } + async fn applied_versions(pool: &PgPool) -> Vec { sqlx::query_scalar::<_, i64>( "SELECT version FROM _sqlx_migrations WHERE success ORDER BY version", @@ -1208,9 +1215,10 @@ mod tests { let pool = sqlx::postgres::PgPoolOptions::new() .max_connections(1) .after_connect(|connection, _meta| { - Box::pin(crate::apply_runtime_connection_timeouts( - connection, TIGHT, TIGHT, - )) + let tight = tight_timeout(TIGHT); + Box::pin(async move { + crate::apply_runtime_connection_timeouts(connection, &tight, &tight).await + }) }) .connect(&database_url) .await @@ -1336,9 +1344,10 @@ mod tests { sqlx::postgres::PgPoolOptions::new() .max_connections(2) .after_connect(move |connection, _meta| { - Box::pin(crate::apply_runtime_connection_timeouts( - connection, timeout, timeout, - )) + let timeout = tight_timeout(timeout); + Box::pin(async move { + crate::apply_runtime_connection_timeouts(connection, &timeout, &timeout).await + }) }) .connect(&database_url) .await diff --git a/crates/buzz-relay/src/config.rs b/crates/buzz-relay/src/config.rs index 07ab0a17e..567d6fcb6 100644 --- a/crates/buzz-relay/src/config.rs +++ b/crates/buzz-relay/src/config.rs @@ -3,6 +3,7 @@ use std::net::SocketAddr; use std::time::Duration; +use buzz_db::PgTimeout; use sha2::{Digest, Sha256}; use thiserror::Error; use tracing::warn; @@ -102,11 +103,11 @@ pub struct Config { /// Postgres `statement_timeout` for every runtime connection /// (`BUZZ_DB_STATEMENT_TIMEOUT`, e.g. `45s`, `500ms`, `0` to disable). /// Tunable so a backfill or an incident does not need a code change. - pub db_statement_timeout: String, + pub db_statement_timeout: PgTimeout, /// Postgres `lock_timeout` for every runtime connection /// (`BUZZ_DB_LOCK_TIMEOUT`). Schema migrations always run with both limits /// lifted — see `buzz_db::migration::run_migrations`. - pub db_lock_timeout: String, + pub db_lock_timeout: PgTimeout, /// Public WebSocket URL of this relay, advertised in NIP-11. pub relay_url: String, /// Public WebSocket URL of the dedicated device-pairing relay, when configured. @@ -306,70 +307,6 @@ fn parse_bind_addr(raw: &str) -> Result { .map_err(|e| ConfigError::InvalidBindAddr(e.to_string())) } -/// Convert a Postgres timeout magnitude and unit to milliseconds, mirroring the -/// rounding Postgres applies to sub-millisecond `us` values. `None` means the -/// spelling is not something Postgres would accept. -fn pg_timeout_millis(magnitude: &str, unit: &str) -> Option { - let value = magnitude.parse::().ok()?; - match unit { - // Round half up without the `value + 500` intermediate, which overflows - // for the top 500 representable microsecond values. - "us" => Some(value / 1_000 + u128::from(value % 1_000 >= 500)), - "" | "ms" => Some(value), - "s" => value.checked_mul(1_000), - "min" => value.checked_mul(60_000), - "h" => value.checked_mul(3_600_000), - "d" => value.checked_mul(86_400_000), - _ => None, - } -} - -/// Postgres accepts a timeout as an integer (milliseconds) with an optional -/// unit, bounded by `i32::MAX` ms once converted. Anything else is refused in -/// favor of the default rather than failing the config: an unusable value would -/// break every `after_connect`, taking all Postgres access with it, and a relay -/// that keeps its documented default is a better outcome than one that will not -/// start. Magnitude is range-checked, not just shape-checked — `999...9d` is a -/// well-formed spelling that Postgres still rejects. -fn pg_timeout_or_default(raw: Option<&str>, default: &str) -> String { - let candidate = raw.map(str::trim).filter(|value| !value.is_empty()); - let Some(candidate) = candidate else { - return default.to_string(); - }; - - let digits = candidate.chars().take_while(char::is_ascii_digit).count(); - let (magnitude, unit) = candidate.split_at(digits); - let unit = unit.trim().to_ascii_lowercase(); - - match pg_timeout_millis(magnitude, &unit) { - // Return the spelling we validated. PostgreSQL's timeout units are - // case-sensitive, so returning `candidate` here would let `45S` pass - // validation and then fail every pool's `after_connect`. - Some(millis) if millis <= buzz_db::PG_TIMEOUT_MAX_MILLIS => { - format!("{magnitude}{unit}") - } - Some(millis) => { - tracing::warn!( - value = candidate, - millis = %millis, - max_millis = %buzz_db::PG_TIMEOUT_MAX_MILLIS, - default, - "ignoring out-of-range Postgres timeout — Postgres stores it as int milliseconds" - ); - default.to_string() - } - None => { - tracing::warn!( - value = candidate, - default, - "ignoring malformed Postgres timeout — expected an integer with an optional \ - us/ms/s/min/h/d unit" - ); - default.to_string() - } - } -} - fn positive_u64_from_env(name: &str, default: u64) -> Result { match std::env::var(name) { Ok(raw) => raw @@ -603,14 +540,10 @@ impl Config { .and_then(|v| v.parse::().ok()) .filter(|&v| v > 0); - let db_statement_timeout = pg_timeout_or_default( - std::env::var("BUZZ_DB_STATEMENT_TIMEOUT").ok().as_deref(), - buzz_db::RUNTIME_STATEMENT_TIMEOUT, - ); - let db_lock_timeout = pg_timeout_or_default( - std::env::var("BUZZ_DB_LOCK_TIMEOUT").ok().as_deref(), - buzz_db::RUNTIME_LOCK_TIMEOUT, - ); + let db_statement_timeout = + PgTimeout::from_env_or("BUZZ_DB_STATEMENT_TIMEOUT", PgTimeout::statement_default()); + let db_lock_timeout = + PgTimeout::from_env_or("BUZZ_DB_LOCK_TIMEOUT", PgTimeout::lock_default()); let relay_url = std::env::var("RELAY_URL").unwrap_or_else(|_| "ws://localhost:3000".to_string()); @@ -1131,7 +1064,6 @@ impl Config { #[cfg(test)] mod tests { use super::*; - use sqlx::Connection; // Mutex to serialize tests that mutate environment variables. // Parallel env-var mutation causes `defaults_are_valid` to see the invalid @@ -1348,117 +1280,6 @@ mod tests { assert_eq!(junk, 50, "unparsable value must fall back to the default"); } - #[test] - fn pg_timeout_accepts_postgres_spellings_and_refuses_the_rest() { - for (accepted, canonical) in [ - ("30s", "30s"), - ("500ms", "500ms"), - ("0", "0"), - ("45S", "45s"), - ("2MIN", "2min"), - (" 10 H ", "10h"), - ] { - assert_eq!( - pg_timeout_or_default(Some(accepted), "30s"), - canonical, - "{accepted} is a valid Postgres timeout" - ); - } - - // A rejected value must not reach Postgres: `after_connect` would fail - // for every connection, which is worse than the documented default. - for rejected in ["", " ", "soon", "30 seconds", "s30", "-5s", "30s;DROP"] { - assert_eq!( - pg_timeout_or_default(Some(rejected), "30s"), - "30s", - "{rejected:?} must fall back to the default" - ); - } - - assert_eq!(pg_timeout_or_default(None, "5s"), "5s"); - } - - /// Every supported unit spelling emitted by the parser must be usable by - /// PostgreSQL. A parser-only assertion missed that PostgreSQL rejects - /// uppercase units even though validation lowercases them. - #[tokio::test] - #[ignore = "requires Postgres"] - async fn pg_timeout_canonical_spellings_are_accepted_by_postgres() { - let database_url = std::env::var("BUZZ_TEST_DATABASE_URL") - .or_else(|_| std::env::var("DATABASE_URL")) - .unwrap_or_else(|_| { - "postgres://buzz:buzz_dev@localhost:5432/buzz".to_string() // sadscan:disable np.postgres.1 - }); - let mut connection = sqlx::PgConnection::connect(&database_url) - .await - .expect("connect test Postgres"); - - for (raw, canonical) in [ - ("500US", "500us"), - ("500MS", "500ms"), - ("2S", "2s"), - ("2MIN", "2min"), - ("2H", "2h"), - ("2D", "2d"), - ("0", "0"), - (" 10 S ", "10s"), - ] { - let parsed = pg_timeout_or_default(Some(raw), "30s"); - assert_eq!(parsed, canonical); - buzz_db::apply_runtime_connection_timeouts(&mut connection, &parsed, &parsed) - .await - .unwrap_or_else(|error| panic!("{raw:?} normalized to {parsed:?}: {error}")); - } - - connection.close().await.expect("close test Postgres"); - } - - #[test] - fn pg_timeout_refuses_magnitudes_postgres_cannot_store() { - // Well-formed spellings whose millisecond value exceeds the int GUC - // range. Postgres rejects these in `set_config`, which would fail every - // pool's `after_connect`. - for rejected in [ - "2147483648", - "2147483648ms", - "2147484s", - "35792min", - "597h", - "25d", - // Wider than any integer type — must fall back, not overflow. - "999999999999999999999999999999999999999999d", - "99999999999999999999999999999999999999999999999999", - // Parses as u128, so unlike the two above it reaches the unit - // conversion — where rounding must not overflow on the way to the - // range check. - &format!("{}us", u128::MAX), - &format!("{}us", u128::MAX - 499), - ] { - assert_eq!( - pg_timeout_or_default(Some(rejected), "30s"), - "30s", - "{rejected:?} is out of range for Postgres and must fall back" - ); - } - - // The boundary itself, and the same instant in every unit, stay valid. - for accepted in [ - "2147483647", - "2147483647ms", - "2147483647000us", - "2147483s", - "35791min", - "596h", - "24d", - ] { - assert_eq!( - pg_timeout_or_default(Some(accepted), "30s"), - accepted, - "{accepted} is inside the Postgres range" - ); - } - } - #[test] fn db_timeout_env_overrides_and_invalid_fallback() { let _guard = ENV_MUTEX.lock().unwrap(); @@ -1469,27 +1290,30 @@ mod tests { std::env::remove_var("BUZZ_DB_LOCK_TIMEOUT"); let defaults = Config::from_env().expect("config"); assert_eq!( - defaults.db_statement_timeout, + defaults.db_statement_timeout.as_str(), buzz_db::RUNTIME_STATEMENT_TIMEOUT ); - assert_eq!(defaults.db_lock_timeout, buzz_db::RUNTIME_LOCK_TIMEOUT); + assert_eq!( + defaults.db_lock_timeout.as_str(), + buzz_db::RUNTIME_LOCK_TIMEOUT + ); std::env::set_var("BUZZ_DB_STATEMENT_TIMEOUT", "90s"); std::env::set_var("BUZZ_DB_LOCK_TIMEOUT", "250ms"); let overridden = Config::from_env().expect("config"); - assert_eq!(overridden.db_statement_timeout, "90s"); - assert_eq!(overridden.db_lock_timeout, "250ms"); + assert_eq!(overridden.db_statement_timeout.as_str(), "90s"); + assert_eq!(overridden.db_lock_timeout.as_str(), "250ms"); std::env::set_var("BUZZ_DB_STATEMENT_TIMEOUT", "45S"); std::env::set_var("BUZZ_DB_LOCK_TIMEOUT", " 2 MIN "); let canonicalized = Config::from_env().expect("config"); - assert_eq!(canonicalized.db_statement_timeout, "45s"); - assert_eq!(canonicalized.db_lock_timeout, "2min"); + assert_eq!(canonicalized.db_statement_timeout.as_str(), "45s"); + assert_eq!(canonicalized.db_lock_timeout.as_str(), "2min"); std::env::set_var("BUZZ_DB_STATEMENT_TIMEOUT", "half a minute"); let junk = Config::from_env().expect("config"); assert_eq!( - junk.db_statement_timeout, + junk.db_statement_timeout.as_str(), buzz_db::RUNTIME_STATEMENT_TIMEOUT, "a malformed value must not be handed to Postgres" ); @@ -1503,11 +1327,14 @@ mod tests { std::env::set_var("BUZZ_DB_LOCK_TIMEOUT", "25d"); let out_of_range = Config::from_env().expect("config"); assert_eq!( - out_of_range.db_statement_timeout, + out_of_range.db_statement_timeout.as_str(), buzz_db::RUNTIME_STATEMENT_TIMEOUT, "an out-of-range value must not be handed to Postgres" ); - assert_eq!(out_of_range.db_lock_timeout, buzz_db::RUNTIME_LOCK_TIMEOUT); + assert_eq!( + out_of_range.db_lock_timeout.as_str(), + buzz_db::RUNTIME_LOCK_TIMEOUT + ); match previous_statement { Some(value) => std::env::set_var("BUZZ_DB_STATEMENT_TIMEOUT", value),