diff --git a/CHANGELOG.md b/CHANGELOG.md index 84ad3d81a..3d5701063 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,10 +2,20 @@ ## Unreleased +### New Features + +- Added [`TransportFactory::create_transport_with_options`](https://docs.rs/sentry-core/latest/sentry_core/trait.TransportFactory.html#method.create_transport_with_options), which constructs transports from [`TransportOptions`](https://docs.rs/sentry-core/latest/sentry_core/struct.TransportOptions.html) instead of full [`ClientOptions`](https://docs.rs/sentry-core/latest/sentry_core/struct.ClientOptions.html) ([#1142](https://github.com/getsentry/sentry-rust/pull/1142)). +- Added transport-specific options types and `with_options` constructors for built-in HTTP transports, including `ReqwestHttpTransportOptions`, `CurlHttpTransportOptions`, `UreqHttpTransportOptions`, and `EmbeddedSVCHttpTransportOptions` ([#1142](https://github.com/getsentry/sentry-rust/pull/1142)). +- Added transport worker options types and deprecated the legacy constructors for built-in background transport workers, including `StdTransportThreadOptions` and `TokioTransportThreadOptions` ([#1142](https://github.com/getsentry/sentry-rust/pull/1142)). + ### Fixes - Fixed `ureq` transport handling for HTTP error statuses so `429` rate limits and `413` payload-too-large responses are processed correctly ([#1177](https://github.com/getsentry/sentry-rust/pull/1177)). +### Behavior Changes + +- Custom transport factories that implement [`TransportFactory::create_transport`](https://docs.rs/sentry-core/latest/sentry_core/trait.TransportFactory.html#method.create_transport) now receive [`ClientOptions`](https://docs.rs/sentry-core/latest/sentry_core/struct.ClientOptions.html) reconstructed from [`TransportOptions`](https://docs.rs/sentry-core/latest/sentry_core/struct.TransportOptions.html). The reconstructed options include only transport-relevant fields, such as DSN, user agent, proxy settings, and TLS certificate validation settings. This may affect code that reads non-transport fields in `create_transport`, but the API remains source-compatible and this change is included in a minor/patch release ([#1142](https://github.com/getsentry/sentry-rust/pull/1142)). + ## 0.48.2 ### New Features diff --git a/sentry-core/src/client/mod.rs b/sentry-core/src/client/mod.rs index 00db0d9b3..51bf5f281 100644 --- a/sentry-core/src/client/mod.rs +++ b/sentry-core/src/client/mod.rs @@ -13,6 +13,7 @@ use std::time::Duration; use crate::metrics::IntoProtocolMetric; #[cfg(feature = "release-health")] use crate::protocol::SessionUpdate; +use crate::transport::TransportOptions; use rand::random; use sentry_types::random_uuid; @@ -594,13 +595,25 @@ fn build_envelope_sender(client_options: &ClientOptions) -> EnvelopeSender { let ClientOptions { dsn, transport: transport_factory, + user_agent, + http_proxy, + https_proxy, + accept_invalid_certs, .. } = client_options; match (dsn.as_ref(), transport_factory.as_ref()) { - (Some(_), Some(transport_factory)) => { - EnvelopeSender::new(|| transport_factory.create_transport(client_options)) - } + (Some(dsn), Some(transport_factory)) => EnvelopeSender::new(|| { + let options = TransportOptions { + dsn: dsn.clone(), + user_agent: user_agent.clone(), + http_proxy: http_proxy.clone(), + https_proxy: https_proxy.clone(), + accept_invalid_certs: *accept_invalid_certs, + }; + + transport_factory.create_transport_with_options(options) + }), _ => Default::default(), } } diff --git a/sentry-core/src/lib.rs b/sentry-core/src/lib.rs index c6e0cd28b..5f2ee1717 100644 --- a/sentry-core/src/lib.rs +++ b/sentry-core/src/lib.rs @@ -131,7 +131,7 @@ pub use crate::integration::Integration; pub use crate::intodsn::IntoDsn; pub use crate::performance::*; pub use crate::scope::{Scope, ScopeGuard}; -pub use crate::transport::{Transport, TransportFactory}; +pub use crate::transport::{Transport, TransportFactory, TransportOptions}; #[cfg(feature = "logs")] mod logger; // structured logging macros exported with `#[macro_export]` diff --git a/sentry-core/src/transport/mod.rs b/sentry-core/src/transport/mod.rs index 6dd716029..6af97647e 100644 --- a/sentry-core/src/transport/mod.rs +++ b/sentry-core/src/transport/mod.rs @@ -3,6 +3,10 @@ use std::time::Duration; use crate::{ClientOptions, Envelope}; +mod options; + +pub use self::options::TransportOptions; + /// The trait for transports. /// /// A transport is responsible for sending events to Sentry. Custom implementations @@ -32,21 +36,62 @@ pub trait Transport: Send + Sync + 'static { /// A factory creating transport instances. /// /// Because options are potentially reused between different clients the -/// options do not actually contain a transport but a factory object that +/// [`ClientOptions`] do not actually contain a transport but a factory object that /// can create transports instead. /// -/// The factory has a single method that creates a new arced transport. -/// Because transports can be wrapped in `Arc`s and those are clonable -/// any `Arc` is also a valid transport factory. This for -/// instance lets you put a `Arc` directly into the options. +/// This factory has two methods. Although both methods have default implementations, the default +/// implementations call each other, so to avoid an infinitely recursive loop, types implementing +/// this trait **must implement at least one of these methods**. We recommend that implementors +/// implement only [`TransportFactory::create_transport_with_options`] because the other method, +/// [`TransportFactory::create_transport`] only exists for backwards compatibility. /// -/// This is automatically implemented for all closures optionally taking -/// options and returning a boxed factory. +/// Both factory methods create a new transport wrapped in an [`Arc`]. Because transports can be +/// wrapped in `Arc`s and those are clonable, `Arc` is also a valid transport factory. +/// This for instance lets you put a `Arc` directly into the options. pub trait TransportFactory: Send + Sync { - /// Given some options creates a transport. - fn create_transport(&self, options: &ClientOptions) -> Arc; + /// Create a transport with the given `options`. + /// + /// Although a default implementation is provided for this trait method, we recommend that all + /// custom transport factories implement this method, as it is the way the SDK constructs the + /// transport. The default implementaton calls [`TransportFactory::create_transport`] as a + /// fallback. + fn create_transport_with_options(&self, options: TransportOptions) -> Arc { + #[expect(deprecated, reason = "need to call deprecated method for back-compat")] + self.create_transport(&options.into_client_options()) + } + + /// The legacy method for creating a transport. + /// + /// This method exists for backwards compatiblity with custom transport factories, which were + /// created before [`TransportFactory::create_transport_with_options`] was added, and thus only + /// implement this method. + /// + /// New custom transport factories **should not** implement this method, as it is not called + /// from the SDK. A sensible default implementation, which forwards to + /// [`TransportFactory::create_transport_with_options`] is provided. + #[deprecated = "use and implement `create_transport_with_options` instead"] + fn create_transport(&self, options: &ClientOptions) -> Arc { + TransportOptions::try_from_client_options(options).map_or_else( + || { + let no_op: Arc = Arc::new(NoOpTransport); + no_op + }, + |options| self.create_transport_with_options(options), + ) + } } +/// A no-op transport. +/// +/// This is returned by [`TransportFactory::create_transport`] when called without a `dsn` in the +/// [`ClientOptions`], rendering the transport disabled. +struct NoOpTransport; + +/// This implementor is **deprecated**, as the closure is used as the deprecated +/// [`TransportFactory::create_transport`] method. +/// +/// Use or create a [`TransportFactory`] which provides +/// [`TransportFactory::create_transport_with_options`], instead. impl TransportFactory for F where F: Fn(&ClientOptions) -> Arc + Clone + Send + Sync + 'static, @@ -67,8 +112,13 @@ impl Transport for Arc { } impl TransportFactory for Arc { - fn create_transport(&self, options: &ClientOptions) -> Arc { - let _options = options; + fn create_transport_with_options(&self, _: TransportOptions) -> Arc { self.clone() } } + +impl Transport for NoOpTransport { + fn send_envelope(&self, envelope: Envelope) { + let _ = envelope; + } +} diff --git a/sentry-core/src/transport/options.rs b/sentry-core/src/transport/options.rs new file mode 100644 index 000000000..4129a31aa --- /dev/null +++ b/sentry-core/src/transport/options.rs @@ -0,0 +1,80 @@ +//! Includes the [`TransportOptions`] struct. + +use std::borrow::Cow; + +use sentry_types::Dsn; + +use crate::ClientOptions; + +/// Options for a transport. +#[derive(Debug)] +#[must_use] +#[non_exhaustive] +pub struct TransportOptions { + /// The transport's Sentry DSN. + pub dsn: Dsn, + /// The user agent sent with transport requests. + pub user_agent: Cow<'static, str>, + /// An optional HTTP proxy to use. + pub http_proxy: Option>, + /// An optional HTTPS proxy to use. + pub https_proxy: Option>, + /// Whether TLS certificate validation should be disabled. + pub accept_invalid_certs: bool, +} + +impl TransportOptions { + /// Try to convert a [`&ClientOptions`](ClientOptions) to a [`TransportOptions`] by extracting + /// the relevant fields from the `ClientOptions`. + /// + /// This method is provided so that code which expects [`TransportOptions`] can be + /// backwards-compatible with older code, which provides `ClientOptions`. + /// + /// Returns [`None`] if `options.dsn` is `None`, `Some(_)` otherwise. + pub fn try_from_client_options(options: &ClientOptions) -> Option { + let ClientOptions { + dsn, + http_proxy, + https_proxy, + accept_invalid_certs, + user_agent, + .. + } = options; + + dsn.as_ref().cloned().map(|dsn| Self { + dsn, + user_agent: user_agent.clone(), + http_proxy: http_proxy.clone(), + https_proxy: https_proxy.clone(), + accept_invalid_certs: *accept_invalid_certs, + }) + } + + /// Converts these [`TransportOptions`] into [`ClientOptions`]. + /// + /// This method is provided for backwards-compatibility with custom transports which cannot + /// be contructed from [`TransportOptions`] because they expect [`ClientOptions`]. + /// + /// Any fields on [`ClientOptions`] which are not present in [`TransportOptions`] will be + /// set to their default values. + pub(crate) fn into_client_options(self) -> ClientOptions { + let Self { + dsn, + user_agent, + http_proxy, + https_proxy, + accept_invalid_certs, + } = self; + + let dsn = Some(dsn); + + ClientOptions { + dsn, + user_agent, + http_proxy, + https_proxy, + accept_invalid_certs, + ..Default::default() + } + } +} diff --git a/sentry/src/transports/curl.rs b/sentry/src/transports/curl.rs index f3ae2a864..48bd37028 100644 --- a/sentry/src/transports/curl.rs +++ b/sentry/src/transports/curl.rs @@ -2,8 +2,12 @@ use std::io::{Cursor, Read}; use std::time::Duration; use curl::easy::Easy as CurlClient; +use sentry_core::TransportOptions; -use super::{thread::TransportThread, HTTP_PAYLOAD_TOO_LARGE, HTTP_PAYLOAD_TOO_LARGE_MESSAGE}; +use super::{ + thread::{TransportThread, TransportThreadOptions}, + RateLimiter, HTTP_PAYLOAD_TOO_LARGE, HTTP_PAYLOAD_TOO_LARGE_MESSAGE, +}; use crate::{sentry_debug, types::Scheme, ClientOptions, Envelope, Transport}; @@ -15,30 +19,78 @@ pub struct CurlHttpTransport { thread: TransportThread, } +/// Options for constructing a [`CurlHttpTransport`]. +/// +/// Currently, this is primarily a wrapper around a [`TransportOptions`], and must be created with +/// the `From` implementation. Optionally, a [`curl::easy::Easy`] client for the +/// transport may be provided with [`Self::with_client`]. +#[derive(Debug)] +#[must_use] +pub struct CurlHttpTransportOptions { + general_options: TransportOptions, + client: Option, +} + impl CurlHttpTransport { - /// Creates a new Transport. + /// Backwards-compatible method for creating a [`CurlHttpTransport`]. + /// + /// Please use [`CurlHttpTransportOptions::build`] instead. + /// + /// ### Panics + /// + /// Panics if called with `options` that lack a DSN. + #[inline] + #[deprecated = "use `CurlHttpTransportOptions::build` instead"] pub fn new(options: &ClientOptions) -> Self { - Self::new_internal(options, None) + let general_options = TransportOptions::try_from_client_options(options) + .expect("this method should only be called when options has a DSN"); + + CurlHttpTransportOptions::from(general_options).build() } - /// Creates a new Transport that uses the specified [`CurlClient`]. + /// Backwards-compatible method for creating a [`CurlHttpTransport`] that uses the specified + /// [`CurlClient`]. + /// + /// Please use [`CurlHttpTransportOptions::build`] instead. + /// + /// ### Panics + /// + /// Panics if called with `options` that lack a DSN. + #[inline] + #[deprecated = "use `CurlHttpTransportOptions::build` instead"] pub fn with_client(options: &ClientOptions, client: CurlClient) -> Self { - Self::new_internal(options, Some(client)) + let general_options = TransportOptions::try_from_client_options(options) + .expect("this method should only be called when options has a DSN"); + + CurlHttpTransportOptions::from(general_options) + .with_client(client) + .build() } - fn new_internal(options: &ClientOptions, client: Option) -> Self { + /// Creates a new [`CurlHttpTransport`] with the given `options`. + #[inline] + pub(super) fn with_options(options: CurlHttpTransportOptions) -> Self { + let CurlHttpTransportOptions { + general_options: + TransportOptions { + dsn, + user_agent, + http_proxy, + https_proxy, + accept_invalid_certs, + .. + }, + client, + } = options; + let client = client.unwrap_or_else(CurlClient::new); - let http_proxy = options.http_proxy.as_ref().map(ToString::to_string); - let https_proxy = options.https_proxy.as_ref().map(ToString::to_string); - let dsn = options.dsn.as_ref().unwrap(); - let user_agent = options.user_agent.clone(); let auth = dsn.to_auth(Some(&user_agent)).to_string(); let url = dsn.envelope_api_url().to_string(); let scheme = dsn.scheme(); - let accept_invalid_certs = options.accept_invalid_certs; let mut handle = client; - let thread = TransportThread::new(move |envelope, rl| { + + let send_fn = move |envelope: Envelope, rl: &mut RateLimiter| { handle.reset(); handle.url(&url).unwrap(); handle.custom_request("POST").unwrap(); @@ -130,7 +182,9 @@ impl CurlHttpTransport { sentry_debug!("Failed to send envelope: {}", err); } } - }); + }; + + let thread = TransportThreadOptions::new(send_fn).spawn_thread(); Self { thread } } } @@ -147,3 +201,28 @@ impl Transport for CurlHttpTransport { self.flush(timeout) } } + +impl From for CurlHttpTransportOptions { + #[inline] + fn from(value: TransportOptions) -> Self { + Self { + general_options: value, + client: None, + } + } +} + +impl CurlHttpTransportOptions { + /// Specify the [`CurlClient`] for the [`CurlHttpTransport`]. + #[inline] + pub fn with_client(self, client: CurlClient) -> Self { + let client = Some(client); + Self { client, ..self } + } + + /// Create a [`CurlHttpTransport`] using these options. + #[inline] + pub fn build(self) -> CurlHttpTransport { + CurlHttpTransport::with_options(self) + } +} diff --git a/sentry/src/transports/embedded_svc_http.rs b/sentry/src/transports/embedded_svc_http.rs index 5643ada11..72af2e825 100644 --- a/sentry/src/transports/embedded_svc_http.rs +++ b/sentry/src/transports/embedded_svc_http.rs @@ -1,3 +1,5 @@ +use sentry_core::TransportOptions; + use super::{HTTP_PAYLOAD_TOO_LARGE, HTTP_PAYLOAD_TOO_LARGE_MESSAGE}; use crate::{sentry_debug, ClientOptions, Transport}; use embedded_svc::http::client::Client as HttpClient; @@ -5,14 +7,38 @@ use esp_idf_svc::{http::client::EspHttpConnection, io::Write}; /// Transport using the embedded-svc http client pub struct EmbeddedSVCHttpTransport { - options: ClientOptions, + /// The transport options. + /// + /// For backwards-compatibility, this is an [`Option`]. A value of [`None`] only occurs when + /// the transport is constructed with [`Self::new`] without a `dsn` in the [`ClientOptions`]. + options: Option, +} + +/// Options for constructing an [`EmbeddedSVCHttpTransport`]. +/// +/// Currently, this is a wrapper around a [`TransportOptions`], and must be created with the +/// `From` implementation. +#[derive(Debug)] +#[must_use] +pub struct EmbeddedSVCHttpTransportOptions { + general_options: TransportOptions, } impl EmbeddedSVCHttpTransport { - /// Creates a new transport + /// Backwards-compatible method for creating an [`EmbeddedSVCHttpTransport`]. + /// + /// Please use [`EmbeddedSVCHttpTransportOptions::build`] instead. + #[deprecated = "use `EmbeddedSVCHttpTransportOptions::build` instead"] pub fn new(options: &ClientOptions) -> Self { Self { - options: options.clone(), + options: TransportOptions::try_from_client_options(options), + } + } + + /// Creates a new [`EmbeddedSVCHttpTransport`] with the given `options`. + pub(super) fn with_options(options: EmbeddedSVCHttpTransportOptions) -> Self { + Self { + options: Some(options.general_options), } } } @@ -22,12 +48,9 @@ impl EmbeddedSVCHttpTransport { &self, envelope: sentry_core::Envelope, ) -> Result<(), Box> { - let dsn = self - .options - .dsn - .as_ref() - .ok_or_else(|| "No DSN specified")?; - let user_agent = &self.options.user_agent; + let TransportOptions { + dsn, user_agent, .. + } = self.options.as_ref().ok_or_else(|| "No DSN specified")?; let auth = dsn.to_auth(Some(user_agent)).to_string(); let headers = [("X-Sentry-Auth", auth.as_str())]; let url = dsn.envelope_api_url(); @@ -67,3 +90,20 @@ impl Transport for EmbeddedSVCHttpTransport { } } } + +impl From for EmbeddedSVCHttpTransportOptions { + #[inline] + fn from(value: TransportOptions) -> Self { + Self { + general_options: value, + } + } +} + +impl EmbeddedSVCHttpTransportOptions { + /// Create an [`EmbeddedSVCHttpTransport`] using these options. + #[inline] + pub fn build(self) -> EmbeddedSVCHttpTransport { + EmbeddedSVCHttpTransport::with_options(self) + } +} diff --git a/sentry/src/transports/mod.rs b/sentry/src/transports/mod.rs index 7e959612b..f2bf72e25 100644 --- a/sentry/src/transports/mod.rs +++ b/sentry/src/transports/mod.rs @@ -3,7 +3,9 @@ //! This module exposes all transports that are compiled into the sentry //! library. The `reqwest`, `curl`, and `ureq` features turn on these transports. -use crate::{ClientOptions, Transport, TransportFactory}; +use sentry_core::TransportOptions; + +use crate::{Transport, TransportFactory}; use std::sync::Arc; #[cfg(feature = "httpdate")] @@ -14,32 +16,36 @@ pub use self::ratelimit::{RateLimiter, RateLimitingCategory}; #[cfg(any(feature = "curl", feature = "ureq"))] mod thread; #[cfg(any(feature = "curl", feature = "ureq"))] -pub use self::thread::TransportThread as StdTransportThread; +pub use self::thread::{ + TransportThread as StdTransportThread, TransportThreadOptions as StdTransportThreadOptions, +}; #[cfg(feature = "reqwest")] mod tokio_thread; #[cfg(feature = "reqwest")] -pub use self::tokio_thread::TransportThread as TokioTransportThread; +pub use self::tokio_thread::{ + TransportThread as TokioTransportThread, TransportThreadOptions as TokioTransportThreadOptions, +}; #[cfg(feature = "reqwest")] mod reqwest; #[cfg(feature = "reqwest")] -pub use self::reqwest::ReqwestHttpTransport; +pub use self::reqwest::{ReqwestHttpTransport, ReqwestHttpTransportOptions}; #[cfg(sentry_embedded_svc_http)] mod embedded_svc_http; #[cfg(sentry_embedded_svc_http)] -pub use self::embedded_svc_http::EmbeddedSVCHttpTransport; +pub use self::embedded_svc_http::{EmbeddedSVCHttpTransport, EmbeddedSVCHttpTransportOptions}; #[cfg(feature = "curl")] mod curl; #[cfg(feature = "curl")] -pub use self::curl::CurlHttpTransport; +pub use self::curl::{CurlHttpTransport, CurlHttpTransportOptions}; #[cfg(feature = "ureq")] mod ureq; #[cfg(feature = "ureq")] -pub use self::ureq::UreqHttpTransport; +pub use self::ureq::{UreqHttpTransport, UreqHttpTransportOptions}; #[cfg(sentry_any_http_transport)] pub(crate) const HTTP_PAYLOAD_TOO_LARGE: u16 = 413; @@ -88,10 +94,10 @@ pub type HttpTransport = DefaultTransport; pub struct DefaultTransportFactory; impl TransportFactory for DefaultTransportFactory { - fn create_transport(&self, options: &ClientOptions) -> Arc { + fn create_transport_with_options(&self, options: TransportOptions) -> Arc { #[cfg(sentry_any_http_transport)] { - Arc::new(HttpTransport::new(options)) + Arc::new(HttpTransport::with_options(options.into())) } #[cfg(not(sentry_any_http_transport))] { diff --git a/sentry/src/transports/reqwest.rs b/sentry/src/transports/reqwest.rs index a125b18ff..0884ebdb4 100644 --- a/sentry/src/transports/reqwest.rs +++ b/sentry/src/transports/reqwest.rs @@ -1,9 +1,11 @@ use std::time::Duration; use reqwest::{header as ReqwestHeaders, Client as ReqwestClient, Proxy, StatusCode}; +use sentry_core::TransportOptions; use super::{ - tokio_thread::TransportThread, HTTP_PAYLOAD_TOO_LARGE, HTTP_PAYLOAD_TOO_LARGE_MESSAGE, + tokio_thread::{TransportThread, TransportThreadOptions}, + RateLimiter, HTTP_PAYLOAD_TOO_LARGE, HTTP_PAYLOAD_TOO_LARGE_MESSAGE, }; use crate::{sentry_debug, ClientOptions, Envelope, Transport}; @@ -18,24 +20,76 @@ pub struct ReqwestHttpTransport { thread: TransportThread, } +/// Options for constructing a [`ReqwestHttpTransport`]. +/// +/// Currently, this is primarily a wrapper around a [`TransportOptions`], and must be created with +/// the `From` implementation. Optionally, a [`reqwest::Client`] for the +/// transport may be provided with [`Self::with_client`]. +#[derive(Debug)] +#[must_use] +pub struct ReqwestHttpTransportOptions { + general_options: TransportOptions, + client: Option, +} + impl ReqwestHttpTransport { - /// Creates a new Transport. + /// Backwards-compatible method for creating a [`ReqwestHttpTransport`]. + /// + /// Please use [`ReqwestHttpTransportOptions::build`] instead. + /// + /// ### Panics + /// + /// Panics if called with `options` that lack a DSN. + #[inline] + #[deprecated = "use `ReqwestHttpTransportOptions::build` instead"] pub fn new(options: &ClientOptions) -> Self { - Self::new_internal(options, None) + let general_options = TransportOptions::try_from_client_options(options) + .expect("this method should only be called when options has a DSN"); + + ReqwestHttpTransportOptions::from(general_options).build() } - /// Creates a new Transport that uses the specified [`ReqwestClient`]. + /// Backwards-compatible method for creating a [`ReqwestHttpTransport`] that uses the specified + /// [`ReqwestClient`]. + /// + /// Please use [`ReqwestHttpTransportOptions::build`] instead. + /// + /// ### Panics + /// + /// Panics if called with `options` that lack a DSN. + #[inline] + #[deprecated = "use `ReqwestHttpTransportOptions::build` instead"] pub fn with_client(options: &ClientOptions, client: ReqwestClient) -> Self { - Self::new_internal(options, Some(client)) + let general_options = TransportOptions::try_from_client_options(options) + .expect("this method should only be called when options has a DSN"); + + ReqwestHttpTransportOptions::from(general_options) + .with_client(client) + .build() } - fn new_internal(options: &ClientOptions, client: Option) -> Self { + /// Creates a new [`ReqwestHttpTransport`] with the given `options`. + #[inline] + pub(super) fn with_options(options: ReqwestHttpTransportOptions) -> Self { + let ReqwestHttpTransportOptions { + general_options: + TransportOptions { + dsn, + user_agent, + http_proxy, + https_proxy, + accept_invalid_certs, + .. + }, + client, + } = options; + let client = client.unwrap_or_else(|| { let mut builder = reqwest::Client::builder(); - if options.accept_invalid_certs { + if accept_invalid_certs { builder = builder.danger_accept_invalid_certs(true); } - if let Some(url) = options.http_proxy.as_ref() { + if let Some(url) = http_proxy.as_ref() { match Proxy::http(url.as_ref()) { Ok(proxy) => { builder = builder.proxy(proxy); @@ -45,7 +99,7 @@ impl ReqwestHttpTransport { } } }; - if let Some(url) = options.https_proxy.as_ref() { + if let Some(url) = https_proxy.as_ref() { match Proxy::https(url.as_ref()) { Ok(proxy) => { builder = builder.proxy(proxy); @@ -59,12 +113,11 @@ impl ReqwestHttpTransport { .build() .expect("Failed to build `reqwest` client as a TLS backend is not available. Enable either the `native-tls` or the `rustls` feature of the `sentry` crate.") }); - let dsn = options.dsn.as_ref().unwrap(); - let user_agent = options.user_agent.clone(); + let auth = dsn.to_auth(Some(&user_agent)).to_string(); let url = dsn.envelope_api_url().to_string(); - let thread = TransportThread::new(move |envelope, mut rl| { + let send_fn = move |envelope: Envelope, mut rl: RateLimiter| { let mut body = Vec::new(); envelope.to_writer(&mut body).unwrap(); let request = client.post(&url).header("X-Sentry-Auth", &auth).body(body); @@ -110,7 +163,9 @@ impl ReqwestHttpTransport { } rl } - }); + }; + + let thread = TransportThreadOptions::new(send_fn).spawn_thread(); Self { thread } } } @@ -127,3 +182,28 @@ impl Transport for ReqwestHttpTransport { self.flush(timeout) } } + +impl From for ReqwestHttpTransportOptions { + #[inline] + fn from(value: TransportOptions) -> Self { + Self { + general_options: value, + client: None, + } + } +} + +impl ReqwestHttpTransportOptions { + /// Specify the [`reqwest::Client`] for the [`ReqwestHttpTransport`]. + #[inline] + pub fn with_client(self, client: ReqwestClient) -> Self { + let client = Some(client); + Self { client, ..self } + } + + /// Create a [`ReqwestHttpTransport`] using these options. + #[inline] + pub fn build(self) -> ReqwestHttpTransport { + ReqwestHttpTransport::with_options(self) + } +} diff --git a/sentry/src/transports/thread.rs b/sentry/src/transports/thread.rs index 4ab402600..5fa6cec58 100644 --- a/sentry/src/transports/thread.rs +++ b/sentry/src/transports/thread.rs @@ -5,6 +5,8 @@ use std::thread::{self, JoinHandle}; use std::time::Duration; use super::ratelimit::{RateLimiter, RateLimitingCategory}; +#[cfg(doc)] +use super::{StdTransportThread, StdTransportThreadOptions}; // so we can use pub re-exports in docs use crate::{sentry_debug, Envelope}; #[expect( @@ -25,12 +27,47 @@ pub struct TransportThread { handle: Option>, } +/// Options for constructing a [`StdTransportThread`]. +#[must_use] +pub struct TransportThreadOptions { + send_fn: F, +} + +impl TransportThreadOptions { + /// Creates options with the function used to send envelopes. + pub fn new(send_fn: F) -> Self { + Self { send_fn } + } +} + +impl TransportThreadOptions +where + F: FnMut(Envelope, &mut RateLimiter) + Send + 'static, +{ + /// Spawn a [`StdTransportThread`], configured per these options. + pub fn spawn_thread(self) -> TransportThread { + TransportThread::with_options(self) + } +} + impl TransportThread { - /// Spawn a new background thread. - pub fn new(mut send: SendFn) -> Self + /// Backwards-compatible method to spawn a new background thread. + /// + /// Please construct this type via [`StdTransportThreadOptions`] instead. + #[deprecated(note = "construct via `TransportThreadOptions` instead")] + pub fn new(send: SendFn) -> Self + where + SendFn: FnMut(Envelope, &mut RateLimiter) + Send + 'static, + { + Self::with_options(TransportThreadOptions::new(send)) + } + + /// Spawn a new background thread with options. + fn with_options(options: TransportThreadOptions) -> Self where SendFn: FnMut(Envelope, &mut RateLimiter) + Send + 'static, { + let TransportThreadOptions { send_fn: mut send } = options; let (sender, receiver) = sync_channel(30); let shutdown = Arc::new(AtomicBool::new(false)); let shutdown_worker = shutdown.clone(); diff --git a/sentry/src/transports/tokio_thread.rs b/sentry/src/transports/tokio_thread.rs index 398963882..800aeb6a6 100644 --- a/sentry/src/transports/tokio_thread.rs +++ b/sentry/src/transports/tokio_thread.rs @@ -5,6 +5,8 @@ use std::thread::{self, JoinHandle}; use std::time::Duration; use super::ratelimit::{RateLimiter, RateLimitingCategory}; +#[cfg(doc)] +use super::{TokioTransportThread, TokioTransportThreadOptions}; // so we can use pub re-exports in docs use crate::{sentry_debug, Envelope}; #[expect( @@ -25,14 +27,53 @@ pub struct TransportThread { handle: Option>, } +/// Options for constructing a [`TokioTransportThread`]. +#[must_use] +pub struct TransportThreadOptions { + send_fn: F, +} + +impl TransportThreadOptions { + /// Creates options with the function used to send envelopes. + pub fn new(send_fn: F) -> Self { + Self { send_fn } + } +} + +impl TransportThreadOptions +where + F: FnMut(Envelope, RateLimiter) -> SendFuture + Send + 'static, + // NOTE: return RateLimiter to avoid lifetime issues with mutable borrowing across await. + SendFuture: std::future::Future, +{ + /// Spawn a [`TokioTransportThread`], configured per these options. + pub fn spawn_thread(self) -> TransportThread { + TransportThread::with_options(self) + } +} + impl TransportThread { - /// Spawn a new background thread. - pub fn new(mut send: SendFn) -> Self + /// Backwards-compatible method to spawn a new background thread. + /// + /// Please construct this type via [`TokioTransportThreadOptions`] instead. + #[deprecated(note = "construct via `TransportThreadOptions` instead")] + pub fn new(send: SendFn) -> Self + where + SendFn: FnMut(Envelope, RateLimiter) -> SendFuture + Send + 'static, + // NOTE: return RateLimiter to avoid lifetime issues with mutable borrowing across await. + SendFuture: std::future::Future, + { + Self::with_options(TransportThreadOptions::new(send)) + } + + /// Spawn a new background thread with options. + fn with_options(options: TransportThreadOptions) -> Self where SendFn: FnMut(Envelope, RateLimiter) -> SendFuture + Send + 'static, - // NOTE: returning RateLimiter here, otherwise we are in borrow hell + // NOTE: return RateLimiter to avoid lifetime issues with mutable borrowing across await. SendFuture: std::future::Future, { + let TransportThreadOptions { send_fn: mut send } = options; let (sender, receiver) = sync_channel(30); let shutdown = Arc::new(AtomicBool::new(false)); let shutdown_worker = shutdown.clone(); diff --git a/sentry/src/transports/ureq.rs b/sentry/src/transports/ureq.rs index a259eaaae..04d4d4fab 100644 --- a/sentry/src/transports/ureq.rs +++ b/sentry/src/transports/ureq.rs @@ -1,5 +1,6 @@ use std::time::Duration; +use sentry_core::TransportOptions; use ureq::http::Response; #[cfg(any( feature = "rustls", @@ -9,7 +10,10 @@ use ureq::http::Response; use ureq::tls::{TlsConfig, TlsProvider}; use ureq::{Agent, Proxy}; -use super::{thread::TransportThread, HTTP_PAYLOAD_TOO_LARGE, HTTP_PAYLOAD_TOO_LARGE_MESSAGE}; +use super::{ + thread::{TransportThread, TransportThreadOptions}, + RateLimiter, HTTP_PAYLOAD_TOO_LARGE, HTTP_PAYLOAD_TOO_LARGE_MESSAGE, +}; use crate::{sentry_debug, types::Scheme, ClientOptions, Envelope, Transport}; @@ -21,19 +25,74 @@ pub struct UreqHttpTransport { thread: TransportThread, } +/// Options for constructing a [`UreqHttpTransport`]. +/// +/// Currently, this is primarily a wrapper around a [`TransportOptions`], and must be created with +/// the `From` implementation. Optionally, a [`ureq::Agent`] for the transport may +/// be provided with [`Self::with_agent`]. +#[derive(Debug)] +#[must_use] +pub struct UreqHttpTransportOptions { + general_options: TransportOptions, + agent: Option, +} + impl UreqHttpTransport { - /// Creates a new Transport. + /// Backwards-compatible method for creating a [`UreqHttpTransport`]. + /// + /// Please use [`UreqHttpTransportOptions::build`] instead. + /// + /// ### Panics + /// + /// Panics if called with `options` that lack a DSN. + #[inline] + #[deprecated = "use `UreqHttpTransportOptions::build` instead"] pub fn new(options: &ClientOptions) -> Self { - Self::new_internal(options, None) + let general_options = TransportOptions::try_from_client_options(options) + .expect("this method should only be called when options has a DSN"); + + UreqHttpTransportOptions::from(general_options).build() } - /// Creates a new Transport that uses the specified [`ureq::Agent`]. + /// Backwards-compatible method for creating a [`UreqHttpTransport`] that uses the specified + /// [`ureq::Agent`]. + /// + /// Please use [`UreqHttpTransportOptions::build`] instead. + /// + /// ### Panics + /// + /// Panics if called with `options` that lack a DSN. + #[inline] + #[deprecated = "use `UreqHttpTransportOptions::build` instead"] pub fn with_agent(options: &ClientOptions, agent: Agent) -> Self { - Self::new_internal(options, Some(agent)) + let general_options = TransportOptions::try_from_client_options(options) + .expect("this method should only be called when options has a DSN"); + + UreqHttpTransportOptions::from(general_options) + .with_agent(agent) + .build() } - fn new_internal(options: &ClientOptions, agent: Option) -> Self { - let dsn = options.dsn.as_ref().unwrap(); + /// Creates a new [`UreqHttpTransport`] with the given `options`. + #[inline] + pub(super) fn with_options(options: UreqHttpTransportOptions) -> Self { + let UreqHttpTransportOptions { + general_options: + TransportOptions { + dsn, + user_agent, + http_proxy, + https_proxy, + #[cfg(any( + feature = "native-tls", + feature = "rustls", + feature = "rustls-no-provider" + ))] + accept_invalid_certs, + .. + }, + agent, + } = options; let scheme = dsn.scheme(); let agent = agent.unwrap_or_else(|| { let mut builder = Agent::config_builder(); @@ -43,7 +102,7 @@ impl UreqHttpTransport { builder = builder.tls_config( TlsConfig::builder() .provider(TlsProvider::NativeTls) - .disable_verification(options.accept_invalid_certs) + .disable_verification(accept_invalid_certs) .build(), ); } @@ -52,15 +111,15 @@ impl UreqHttpTransport { builder = builder.tls_config( TlsConfig::builder() .provider(TlsProvider::Rustls) - .disable_verification(options.accept_invalid_certs) + .disable_verification(accept_invalid_certs) .build(), ); } let mut maybe_proxy = None; - match (scheme, &options.http_proxy, &options.https_proxy) { - (Scheme::Https, _, Some(proxy)) => match Proxy::new(proxy) { + match (scheme, &http_proxy, &https_proxy) { + (Scheme::Https, _, Some(proxy)) => match Proxy::new(proxy.as_ref()) { Ok(proxy) => { maybe_proxy = Some(proxy); } @@ -68,7 +127,7 @@ impl UreqHttpTransport { sentry_debug!("invalid proxy: {:?}", err); } }, - (_, Some(proxy), _) => match Proxy::new(proxy) { + (_, Some(proxy), _) => match Proxy::new(proxy.as_ref()) { Ok(proxy) => { maybe_proxy = Some(proxy); } @@ -83,11 +142,10 @@ impl UreqHttpTransport { builder.build().new_agent() }); - let user_agent = options.user_agent.clone(); let auth = dsn.to_auth(Some(&user_agent)).to_string(); let url = dsn.envelope_api_url().to_string(); - let thread = TransportThread::new(move |envelope, rl| { + let send_fn = move |envelope: Envelope, rl: &mut RateLimiter| { let mut body = Vec::new(); envelope.to_writer(&mut body).unwrap(); let request = agent @@ -128,7 +186,9 @@ impl UreqHttpTransport { sentry_debug!("Failed to send envelope: {}", err); } } - }); + }; + + let thread = TransportThreadOptions::new(send_fn).spawn_thread(); Self { thread } } } @@ -145,3 +205,28 @@ impl Transport for UreqHttpTransport { self.flush(timeout) } } + +impl From for UreqHttpTransportOptions { + #[inline] + fn from(value: TransportOptions) -> Self { + Self { + general_options: value, + agent: None, + } + } +} + +impl UreqHttpTransportOptions { + /// Specify the [`ureq::Agent`] for the [`UreqHttpTransport`]. + #[inline] + pub fn with_agent(self, agent: Agent) -> Self { + let agent = Some(agent); + Self { agent, ..self } + } + + /// Create a [`UreqHttpTransport`] using these options. + #[inline] + pub fn build(self) -> UreqHttpTransport { + UreqHttpTransport::with_options(self) + } +}