From 3a1defae0b66b68eed248540b1076a769e26cbfe Mon Sep 17 00:00:00 2001 From: Daniel Szoke Date: Fri, 8 May 2026 13:09:33 +0200 Subject: [PATCH] chore(hub): Deprecate `Hub::with` Following the safety improvements in #1123, `Hub::with` no longer offers any performance benefit over using `Hub::current`. `Hub::current` provides a simpler API, and also returns an owned `Arc` rather than providing access to a `&Arc` via a callback (necessitating a second clone for anyone needing an owned `Arc` in that callback). As `Hub::current` is the arguably better API, let's deprecate `Hub::with` and encourage folks to migrate. --- sentry-core/src/api.rs | 15 +++++++-------- sentry-core/src/client.rs | 2 +- sentry-core/src/hub.rs | 14 +++++++------- sentry-core/src/hub_impl.rs | 32 +++++++++++++++++++++----------- sentry-core/src/macros.rs | 15 +++++++-------- sentry-tower/src/lib.rs | 7 +------ sentry/src/init.rs | 4 +++- sentry/tests/test_tower.rs | 3 +-- 8 files changed, 48 insertions(+), 44 deletions(-) diff --git a/sentry-core/src/api.rs b/sentry-core/src/api.rs index ea0b8532f..a4279fe35 100644 --- a/sentry-core/src/api.rs +++ b/sentry-core/src/api.rs @@ -188,13 +188,12 @@ where { #[cfg(feature = "client")] { - Hub::with(|hub| { - if hub.is_active_and_usage_safe() { - hub.with_scope(scope_config, callback) - } else { - callback() - } - }) + let hub = Hub::current(); + if hub.is_active_and_usage_safe() { + hub.with_scope(scope_config, callback) + } else { + callback() + } } #[cfg(not(feature = "client"))] { @@ -261,7 +260,7 @@ where /// [`Hub`]: struct.Hub.html pub fn last_event_id() -> Option { with_client_impl! {{ - Hub::with(|hub| hub.last_event_id()) + Hub::with_current(|hub| hub.last_event_id()) }} } diff --git a/sentry-core/src/client.rs b/sentry-core/src/client.rs index 63c73e8cf..69f419051 100644 --- a/sentry-core/src/client.rs +++ b/sentry-core/src/client.rs @@ -159,7 +159,7 @@ impl Client { pub fn with_options(mut options: ClientOptions) -> Client { // Create the main hub eagerly to avoid problems with the background thread // See https://github.com/getsentry/sentry-rust/issues/237 - Hub::with(|_| {}); + Hub::with_current(|_| {}); let create_transport = || { options.dsn.as_ref()?; diff --git a/sentry-core/src/hub.rs b/sentry-core/src/hub.rs index 511273a5f..b8411173a 100644 --- a/sentry-core/src/hub.rs +++ b/sentry-core/src/hub.rs @@ -65,13 +65,13 @@ impl Hub { { use_without_client!(f); with_client_impl! {{ - Hub::with(|hub| { - if hub.is_active_and_usage_safe() { - f(hub) - } else { - Default::default() - } - }) + let hub = Hub::current(); + if hub.is_active_and_usage_safe() { + f(&hub) + } else { + Default::default() + } + }} } diff --git a/sentry-core/src/hub_impl.rs b/sentry-core/src/hub_impl.rs index ca72725c8..aa1dce975 100644 --- a/sentry-core/src/hub_impl.rs +++ b/sentry-core/src/hub_impl.rs @@ -146,7 +146,7 @@ impl Hub { /// This method is unavailable if the client implementation is disabled. /// When using the minimal API set use [`Hub::with_active`] instead. pub fn current() -> Arc { - Hub::with(Arc::clone) + THREAD_HUB.with_borrow(Arc::clone) } /// Returns the main thread's hub. @@ -158,20 +158,29 @@ impl Hub { } /// Invokes the callback with the default hub. - /// - /// This is a slightly more efficient version than [`Hub::current`] and - /// also unavailable in minimal mode. + #[deprecated = "Use `Hub::current` instead; this function offers no performance benefit."] pub fn with(f: F) -> R where F: FnOnce(&Arc) -> R, { - THREAD_HUB.with(|hub| { - // Bind `hub` as `hub.borrow().clone()`. - // It is essential we drop `hub.borrow()` before the callback, otherwise we will - // panic if the callback calls `Hub::run`, so we need the binding. - let hub = hub.borrow().clone(); - f(&hub) - }) + f(&Hub::current()) + } + + /// Invokes the callback with a reference to the thread hub. + /// + /// This is potentially more performant than [`Hub::current`] as we avoid an [`Arc::clone`], + /// but it holds a borrow to the [`THREAD_HUB`]'s `RefCell` for the duration of the callback. + /// It is therefore essential to avoid calling [`Hub::run`], [`SwitchGuard::new`], or anything + /// else that mutably borrows the [`THREAD_HUB`] during the callback, e.g. any user-supplied + /// callbacks. + /// + /// # Panics + /// Panics if the [`THREAD_HUB`] is mutably borrowed at any point during the callback. + pub(crate) fn with_current(f: F) -> R + where + F: FnOnce(&Hub) -> R, + { + THREAD_HUB.with_borrow(|hub| f(hub)) } /// Binds a hub to the current thread for the duration of the call. @@ -234,6 +243,7 @@ mod tests { let inner_hub = Arc::new(Hub::new(None, Default::default())); Hub::run(outer_hub, || { + #[expect(deprecated)] // We are intentionally testing deprecated functionality Hub::with(|_| Hub::run(inner_hub, || {})); }); } diff --git a/sentry-core/src/macros.rs b/sentry-core/src/macros.rs index 647ce252e..202eb96bf 100644 --- a/sentry-core/src/macros.rs +++ b/sentry-core/src/macros.rs @@ -54,14 +54,13 @@ macro_rules! with_client_impl { #[macro_export] #[doc(hidden)] macro_rules! sentry_debug { - ($($arg:tt)*) => { - $crate::Hub::with(|hub| { - if hub.client().map_or(false, |c| c.options().debug) { - eprint!("[sentry] "); - eprintln!($($arg)*); - } - }); - } + ($($arg:tt)*) => {{ + let hub = $crate::Hub::current(); + if hub.client().map_or(false, |c| c.options().debug) { + eprint!("[sentry] "); + eprintln!($($arg)*); + } + }} } /// Panics in debug builds and logs through `sentry_debug!` in non-debug builds. diff --git a/sentry-tower/src/lib.rs b/sentry-tower/src/lib.rs index 8c40e5b0c..724abdc52 100644 --- a/sentry-tower/src/lib.rs +++ b/sentry-tower/src/lib.rs @@ -193,12 +193,7 @@ pub struct NewFromTopProvider; impl HubProvider, Request> for NewFromTopProvider { fn hub(&self, _request: &Request) -> Arc { - // The Clippy lint here is a false positive, the suggestion to write - // `Hub::with(Hub::new_from_top)` does not compiles: - // 143 | Hub::with(Hub::new_from_top).into() - // | ^^^^^^^^^ implementation of `std::ops::FnOnce` is not general enough - #[allow(clippy::redundant_closure)] - Hub::with(|hub| Hub::new_from_top(hub)).into() + Hub::new_from_top(Hub::current()).into() } } diff --git a/sentry/src/init.rs b/sentry/src/init.rs index 12527f0d3..59355b3f6 100644 --- a/sentry/src/init.rs +++ b/sentry/src/init.rs @@ -104,7 +104,9 @@ where let client = Arc::new(Client::from(opts)); - Hub::with(|hub| hub.bind_client(Some(client.clone()))); + let hub = Hub::current(); + hub.bind_client(Some(client.clone())); + if let Some(dsn) = client.dsn() { sentry_debug!("enabled sentry client for DSN {}", dsn); } else { diff --git a/sentry/tests/test_tower.rs b/sentry/tests/test_tower.rs index 37f610c0d..264a654a9 100644 --- a/sentry/tests/test_tower.rs +++ b/sentry/tests/test_tower.rs @@ -29,8 +29,7 @@ fn test_tower_hub() { }); sentry::capture_message("Started service", Level::Info); - #[allow(clippy::redundant_closure)] - let hub = Arc::new(Hub::with(|hub| Hub::new_from_top(hub))); + let hub = Arc::new(Hub::new_from_top(Hub::current())); hub.bind_client(Some(Arc::new(opts.into()))); let service = ServiceBuilder::new()