Skip to content

Commit 9c8f904

Browse files
committed
fix: add Debug impl, clamp channel capacity, run cargo fmt
- Add transport_channel_capacity to manual Debug implementation - Clamp channel capacity to minimum of 1 to prevent rendezvous channel from silently dropping envelopes via try_send - Run cargo fmt to fix lint CI failure
1 parent d87061c commit 9c8f904

6 files changed

Lines changed: 172 additions & 153 deletions

File tree

sentry-core/src/clientoptions.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,13 @@ impl fmt::Debug for ClientOptions {
284284
.field("enable_logs", &self.enable_logs)
285285
.field("before_send_log", &before_send_log);
286286

287-
debug_struct.field("user_agent", &self.user_agent).finish()
287+
debug_struct
288+
.field("user_agent", &self.user_agent)
289+
.field(
290+
"transport_channel_capacity",
291+
&self.transport_channel_capacity,
292+
)
293+
.finish()
288294
}
289295
}
290296

sentry/src/transports/curl.rs

Lines changed: 87 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -39,99 +39,103 @@ impl CurlHttpTransport {
3939
let channel_capacity = options.transport_channel_capacity;
4040

4141
let mut handle = client;
42-
let thread = TransportThread::new(move |envelope, rl| {
43-
handle.reset();
44-
handle.url(&url).unwrap();
45-
handle.custom_request("POST").unwrap();
46-
47-
if accept_invalid_certs {
48-
handle.ssl_verify_host(false).unwrap();
49-
handle.ssl_verify_peer(false).unwrap();
50-
}
51-
52-
match (scheme, &http_proxy, &https_proxy) {
53-
(Scheme::Https, _, Some(proxy)) => {
54-
if let Err(err) = handle.proxy(proxy) {
55-
sentry_debug!("invalid proxy: {:?}", err);
56-
}
42+
let thread = TransportThread::new(
43+
move |envelope, rl| {
44+
handle.reset();
45+
handle.url(&url).unwrap();
46+
handle.custom_request("POST").unwrap();
47+
48+
if accept_invalid_certs {
49+
handle.ssl_verify_host(false).unwrap();
50+
handle.ssl_verify_peer(false).unwrap();
5751
}
58-
(_, Some(proxy), _) => {
59-
if let Err(err) = handle.proxy(proxy) {
60-
sentry_debug!("invalid proxy: {:?}", err);
52+
53+
match (scheme, &http_proxy, &https_proxy) {
54+
(Scheme::Https, _, Some(proxy)) => {
55+
if let Err(err) = handle.proxy(proxy) {
56+
sentry_debug!("invalid proxy: {:?}", err);
57+
}
58+
}
59+
(_, Some(proxy), _) => {
60+
if let Err(err) = handle.proxy(proxy) {
61+
sentry_debug!("invalid proxy: {:?}", err);
62+
}
6163
}
64+
_ => {}
6265
}
63-
_ => {}
64-
}
65-
66-
let mut body = Vec::new();
67-
envelope.to_writer(&mut body).unwrap();
68-
let mut body = Cursor::new(body);
69-
70-
let mut retry_after = None;
71-
let mut sentry_header = None;
72-
let mut headers = curl::easy::List::new();
73-
headers.append(&format!("X-Sentry-Auth: {auth}")).unwrap();
74-
headers.append("Expect:").unwrap();
75-
handle.http_headers(headers).unwrap();
76-
handle.upload(true).unwrap();
77-
handle.in_filesize(body.get_ref().len() as u64).unwrap();
78-
handle
79-
.read_function(move |buf| Ok(body.read(buf).unwrap_or(0)))
80-
.unwrap();
81-
handle.verbose(true).unwrap();
82-
handle
83-
.debug_function(move |info, data| {
84-
let prefix = match info {
85-
curl::easy::InfoType::HeaderIn => "< ",
86-
curl::easy::InfoType::HeaderOut => "> ",
87-
curl::easy::InfoType::DataOut => "",
88-
_ => return,
89-
};
90-
sentry_debug!("curl: {}{}", prefix, String::from_utf8_lossy(data).trim());
91-
})
92-
.unwrap();
93-
94-
{
95-
let mut handle = handle.transfer();
96-
let retry_after_setter = &mut retry_after;
97-
let sentry_header_setter = &mut sentry_header;
66+
67+
let mut body = Vec::new();
68+
envelope.to_writer(&mut body).unwrap();
69+
let mut body = Cursor::new(body);
70+
71+
let mut retry_after = None;
72+
let mut sentry_header = None;
73+
let mut headers = curl::easy::List::new();
74+
headers.append(&format!("X-Sentry-Auth: {auth}")).unwrap();
75+
headers.append("Expect:").unwrap();
76+
handle.http_headers(headers).unwrap();
77+
handle.upload(true).unwrap();
78+
handle.in_filesize(body.get_ref().len() as u64).unwrap();
79+
handle
80+
.read_function(move |buf| Ok(body.read(buf).unwrap_or(0)))
81+
.unwrap();
82+
handle.verbose(true).unwrap();
9883
handle
99-
.header_function(move |data| {
100-
if let Ok(data) = std::str::from_utf8(data) {
101-
let mut iter = data.split(':');
102-
if let Some(key) = iter.next().map(str::to_lowercase) {
103-
if key == "retry-after" {
104-
*retry_after_setter = iter.next().map(|x| x.trim().to_string());
105-
} else if key == "x-sentry-rate-limits" {
106-
*sentry_header_setter =
107-
iter.next().map(|x| x.trim().to_string());
84+
.debug_function(move |info, data| {
85+
let prefix = match info {
86+
curl::easy::InfoType::HeaderIn => "< ",
87+
curl::easy::InfoType::HeaderOut => "> ",
88+
curl::easy::InfoType::DataOut => "",
89+
_ => return,
90+
};
91+
sentry_debug!("curl: {}{}", prefix, String::from_utf8_lossy(data).trim());
92+
})
93+
.unwrap();
94+
95+
{
96+
let mut handle = handle.transfer();
97+
let retry_after_setter = &mut retry_after;
98+
let sentry_header_setter = &mut sentry_header;
99+
handle
100+
.header_function(move |data| {
101+
if let Ok(data) = std::str::from_utf8(data) {
102+
let mut iter = data.split(':');
103+
if let Some(key) = iter.next().map(str::to_lowercase) {
104+
if key == "retry-after" {
105+
*retry_after_setter =
106+
iter.next().map(|x| x.trim().to_string());
107+
} else if key == "x-sentry-rate-limits" {
108+
*sentry_header_setter =
109+
iter.next().map(|x| x.trim().to_string());
110+
}
108111
}
109112
}
113+
true
114+
})
115+
.unwrap();
116+
handle.perform().ok();
117+
}
118+
119+
match handle.response_code() {
120+
Ok(response_code) => {
121+
if let Some(sentry_header) = sentry_header {
122+
rl.update_from_sentry_header(&sentry_header);
123+
} else if let Some(retry_after) = retry_after {
124+
rl.update_from_retry_after(&retry_after);
125+
} else if response_code == 429 {
126+
rl.update_from_429();
127+
}
128+
if response_code == HTTP_PAYLOAD_TOO_LARGE as u32 {
129+
sentry_debug!("{HTTP_PAYLOAD_TOO_LARGE_MESSAGE}");
110130
}
111-
true
112-
})
113-
.unwrap();
114-
handle.perform().ok();
115-
}
116-
117-
match handle.response_code() {
118-
Ok(response_code) => {
119-
if let Some(sentry_header) = sentry_header {
120-
rl.update_from_sentry_header(&sentry_header);
121-
} else if let Some(retry_after) = retry_after {
122-
rl.update_from_retry_after(&retry_after);
123-
} else if response_code == 429 {
124-
rl.update_from_429();
125131
}
126-
if response_code == HTTP_PAYLOAD_TOO_LARGE as u32 {
127-
sentry_debug!("{HTTP_PAYLOAD_TOO_LARGE_MESSAGE}");
132+
Err(err) => {
133+
sentry_debug!("Failed to send envelope: {}", err);
128134
}
129135
}
130-
Err(err) => {
131-
sentry_debug!("Failed to send envelope: {}", err);
132-
}
133-
}
134-
}, channel_capacity);
136+
},
137+
channel_capacity,
138+
);
135139
Self { thread }
136140
}
137141
}

sentry/src/transports/reqwest.rs

Lines changed: 41 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -65,53 +65,56 @@ impl ReqwestHttpTransport {
6565
let url = dsn.envelope_api_url().to_string();
6666
let channel_capacity = options.transport_channel_capacity;
6767

68-
let thread = TransportThread::new(move |envelope, mut rl| {
69-
let mut body = Vec::new();
70-
envelope.to_writer(&mut body).unwrap();
71-
let request = client.post(&url).header("X-Sentry-Auth", &auth).body(body);
68+
let thread = TransportThread::new(
69+
move |envelope, mut rl| {
70+
let mut body = Vec::new();
71+
envelope.to_writer(&mut body).unwrap();
72+
let request = client.post(&url).header("X-Sentry-Auth", &auth).body(body);
7273

73-
// NOTE: because of lifetime issues, building the request using the
74-
// `client` has to happen outside of this async block.
75-
async move {
76-
match request.send().await {
77-
Ok(response) => {
78-
let headers = response.headers();
74+
// NOTE: because of lifetime issues, building the request using the
75+
// `client` has to happen outside of this async block.
76+
async move {
77+
match request.send().await {
78+
Ok(response) => {
79+
let headers = response.headers();
7980

80-
if let Some(sentry_header) = headers
81-
.get("x-sentry-rate-limits")
82-
.and_then(|x| x.to_str().ok())
83-
{
84-
rl.update_from_sentry_header(sentry_header);
85-
} else if let Some(retry_after) = headers
86-
.get(ReqwestHeaders::RETRY_AFTER)
87-
.and_then(|x| x.to_str().ok())
88-
{
89-
rl.update_from_retry_after(retry_after);
90-
} else if response.status() == StatusCode::TOO_MANY_REQUESTS {
91-
rl.update_from_429();
92-
}
81+
if let Some(sentry_header) = headers
82+
.get("x-sentry-rate-limits")
83+
.and_then(|x| x.to_str().ok())
84+
{
85+
rl.update_from_sentry_header(sentry_header);
86+
} else if let Some(retry_after) = headers
87+
.get(ReqwestHeaders::RETRY_AFTER)
88+
.and_then(|x| x.to_str().ok())
89+
{
90+
rl.update_from_retry_after(retry_after);
91+
} else if response.status() == StatusCode::TOO_MANY_REQUESTS {
92+
rl.update_from_429();
93+
}
9394

94-
let is_payload_too_large =
95-
response.status().as_u16() == HTTP_PAYLOAD_TOO_LARGE;
96-
match response.text().await {
97-
Err(err) => {
98-
sentry_debug!("Failed to read sentry response: {}", err);
95+
let is_payload_too_large =
96+
response.status().as_u16() == HTTP_PAYLOAD_TOO_LARGE;
97+
match response.text().await {
98+
Err(err) => {
99+
sentry_debug!("Failed to read sentry response: {}", err);
100+
}
101+
Ok(text) => {
102+
sentry_debug!("Get response: `{}`", text);
103+
}
99104
}
100-
Ok(text) => {
101-
sentry_debug!("Get response: `{}`", text);
105+
if is_payload_too_large {
106+
sentry_debug!("{HTTP_PAYLOAD_TOO_LARGE_MESSAGE}");
102107
}
103108
}
104-
if is_payload_too_large {
105-
sentry_debug!("{HTTP_PAYLOAD_TOO_LARGE_MESSAGE}");
109+
Err(err) => {
110+
sentry_debug!("Failed to send envelope: {}", err);
106111
}
107112
}
108-
Err(err) => {
109-
sentry_debug!("Failed to send envelope: {}", err);
110-
}
113+
rl
111114
}
112-
rl
113-
}
114-
}, channel_capacity);
115+
},
116+
channel_capacity,
117+
);
115118
Self { thread }
116119
}
117120
}

sentry/src/transports/thread.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ impl TransportThread {
3131
where
3232
SendFn: FnMut(Envelope, &mut RateLimiter) + Send + 'static,
3333
{
34-
let (sender, receiver) = sync_channel(channel_capacity);
34+
let (sender, receiver) = sync_channel(channel_capacity.max(1));
3535
let shutdown = Arc::new(AtomicBool::new(false));
3636
let shutdown_worker = shutdown.clone();
3737
let handle = thread::Builder::new()

sentry/src/transports/tokio_thread.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ impl TransportThread {
3333
// NOTE: returning RateLimiter here, otherwise we are in borrow hell
3434
SendFuture: std::future::Future<Output = RateLimiter>,
3535
{
36-
let (sender, receiver) = sync_channel(channel_capacity);
36+
let (sender, receiver) = sync_channel(channel_capacity.max(1));
3737
let shutdown = Arc::new(AtomicBool::new(false));
3838
let shutdown_worker = shutdown.clone();
3939
let handle = thread::Builder::new()

sentry/src/transports/ureq.rs

Lines changed: 35 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -84,42 +84,48 @@ impl UreqHttpTransport {
8484
let url = dsn.envelope_api_url().to_string();
8585
let channel_capacity = options.transport_channel_capacity;
8686

87-
let thread = TransportThread::new(move |envelope, rl| {
88-
let mut body = Vec::new();
89-
envelope.to_writer(&mut body).unwrap();
90-
let request = agent.post(&url).header("X-Sentry-Auth", &auth).send(&body);
91-
92-
match request {
93-
Ok(mut response) => {
94-
fn header_str<'a, B>(response: &'a Response<B>, key: &str) -> Option<&'a str> {
95-
response.headers().get(key)?.to_str().ok()
96-
}
87+
let thread = TransportThread::new(
88+
move |envelope, rl| {
89+
let mut body = Vec::new();
90+
envelope.to_writer(&mut body).unwrap();
91+
let request = agent.post(&url).header("X-Sentry-Auth", &auth).send(&body);
92+
93+
match request {
94+
Ok(mut response) => {
95+
fn header_str<'a, B>(
96+
response: &'a Response<B>,
97+
key: &str,
98+
) -> Option<&'a str> {
99+
response.headers().get(key)?.to_str().ok()
100+
}
97101

98-
if let Some(sentry_header) = header_str(&response, "x-sentry-rate-limits") {
99-
rl.update_from_sentry_header(sentry_header);
100-
} else if let Some(retry_after) = header_str(&response, "retry-after") {
101-
rl.update_from_retry_after(retry_after);
102-
} else if response.status() == 429 {
103-
rl.update_from_429();
104-
}
102+
if let Some(sentry_header) = header_str(&response, "x-sentry-rate-limits") {
103+
rl.update_from_sentry_header(sentry_header);
104+
} else if let Some(retry_after) = header_str(&response, "retry-after") {
105+
rl.update_from_retry_after(retry_after);
106+
} else if response.status() == 429 {
107+
rl.update_from_429();
108+
}
105109

106-
match response.body_mut().read_to_string() {
107-
Err(err) => {
108-
sentry_debug!("Failed to read sentry response: {}", err);
110+
match response.body_mut().read_to_string() {
111+
Err(err) => {
112+
sentry_debug!("Failed to read sentry response: {}", err);
113+
}
114+
Ok(text) => {
115+
sentry_debug!("Get response: `{}`", text);
116+
}
109117
}
110-
Ok(text) => {
111-
sentry_debug!("Get response: `{}`", text);
118+
if response.status() == HTTP_PAYLOAD_TOO_LARGE {
119+
sentry_debug!("{HTTP_PAYLOAD_TOO_LARGE_MESSAGE}");
112120
}
113121
}
114-
if response.status() == HTTP_PAYLOAD_TOO_LARGE {
115-
sentry_debug!("{HTTP_PAYLOAD_TOO_LARGE_MESSAGE}");
122+
Err(err) => {
123+
sentry_debug!("Failed to send envelope: {}", err);
116124
}
117125
}
118-
Err(err) => {
119-
sentry_debug!("Failed to send envelope: {}", err);
120-
}
121-
}
122-
}, channel_capacity);
126+
},
127+
channel_capacity,
128+
);
123129
Self { thread }
124130
}
125131
}

0 commit comments

Comments
 (0)