Skip to content

Commit d5207c8

Browse files
Don't show errors in console (#468)
Changes our requests to simply log context, and adds a hidden_in_console field filter so that when we have logs that should go to sentry only, we can hide them from the user. Uses this to hide internal rust errors from our existing logging. ![2025-03-07-085547_1834x598_scrot](https://github.com/user-attachments/assets/1af017f0-75ef-4bea-9f5e-1f6fed8efc8f)
1 parent b47d0cf commit d5207c8

File tree

9 files changed

+66
-48
lines changed

9 files changed

+66
-48
lines changed

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

api/src/client.rs

Lines changed: 25 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
use std::path::Path;
22

3-
use anyhow::Context;
43
use constants::{DEFAULT_ORIGIN, TRUNK_PUBLIC_API_ADDRESS_ENV};
54
use http::{header::HeaderMap, HeaderValue};
65
use reqwest::{header, Client, Response, StatusCode};
6+
use serde::de::DeserializeOwned;
77
use tokio::fs;
88

99
use crate::call_api::CallApi;
@@ -124,10 +124,7 @@ impl ApiClient {
124124
&self.org_url_slug,
125125
)?;
126126

127-
response
128-
.json::<message::CreateBundleUploadResponse>()
129-
.await
130-
.context("Failed to get response body as json.")
127+
self.deserialize_response::<message::CreateBundleUploadResponse>(response).await
131128
},
132129
log_progress_message: |time_elapsed, _| {
133130
format!("Reporting bundle upload initiation to Trunk services is taking longer than expected. It has taken {} seconds so far.", time_elapsed.as_secs())
@@ -168,10 +165,7 @@ impl ApiClient {
168165
&self.org_url_slug,
169166
)?;
170167

171-
response
172-
.json::<message::GetQuarantineConfigResponse>()
173-
.await
174-
.context("Failed to get response body as json.")
168+
self.deserialize_response::<message::GetQuarantineConfigResponse>(response).await
175169
},
176170
log_progress_message: |time_elapsed, _| {
177171
format!("Getting quarantine configuration from Trunk services is taking longer than expected. It has taken {} seconds so far.", time_elapsed.as_secs())
@@ -244,12 +238,14 @@ impl ApiClient {
244238
let error_message = "Failed to send telemetry metrics.";
245239
if !response.status().is_client_error() {
246240
response.error_for_status().map_err(|reqwest_error| {
247-
anyhow::Error::from(reqwest_error).context(error_message)
241+
tracing::warn!("{}", error_message);
242+
anyhow::Error::from(reqwest_error)
248243
}).map(|_| ())
249244
} else {
245+
tracing::warn!("{}", error_message);
250246
match response.error_for_status() {
251247
Ok(..) => Err(anyhow::Error::msg(error_message)),
252-
Err(error) => Err(anyhow::Error::from(error).context(error_message)),
248+
Err(error) => Err(anyhow::Error::from(error)),
253249
}
254250
}
255251
},
@@ -267,6 +263,17 @@ impl ApiClient {
267263
.call_api()
268264
.await
269265
}
266+
267+
async fn deserialize_response<MessageType: DeserializeOwned>(
268+
&self,
269+
response: Response,
270+
) -> Result<MessageType, anyhow::Error> {
271+
let deserialized: reqwest::Result<MessageType> = response.json::<MessageType>().await;
272+
if deserialized.is_err() {
273+
tracing::warn!("Failed to get response body as json.");
274+
}
275+
deserialized.map_err(anyhow::Error::from)
276+
}
270277
}
271278

272279
#[derive(Debug, Clone, Copy)]
@@ -306,7 +313,8 @@ pub(crate) fn status_code_help<T: FnMut(&Response) -> String>(
306313
if !response.status().is_client_error() {
307314
response.error_for_status().map_err(|reqwest_error| {
308315
let error_message = format!("{base_error_message}{HELP_TEXT}");
309-
anyhow::Error::from(reqwest_error).context(error_message)
316+
tracing::warn!("{}", error_message);
317+
anyhow::Error::from(reqwest_error)
310318
})
311319
} else {
312320
let domain: Option<String> = url::Url::parse(api_host)
@@ -333,9 +341,10 @@ pub(crate) fn status_code_help<T: FnMut(&Response) -> String>(
333341

334342
let error_message_with_help = format!("{error_message}{HELP_TEXT}");
335343

344+
tracing::warn!("{}", error_message_with_help);
336345
match response.error_for_status() {
337346
Ok(..) => Err(anyhow::Error::msg(error_message_with_help)),
338-
Err(error) => Err(anyhow::Error::from(error).context(error_message_with_help)),
347+
Err(error) => Err(anyhow::Error::from(error)),
339348
}
340349
}
341350
}
@@ -443,7 +452,7 @@ mod tests {
443452
.await
444453
.unwrap_err()
445454
.to_string()
446-
.contains("Failed to get quarantine bulk test."));
455+
.contains("501 Not Implemented"));
447456
assert_eq!(CALL_COUNT.load(Ordering::Relaxed), 1);
448457
}
449458

@@ -487,7 +496,7 @@ mod tests {
487496
.await
488497
.unwrap_err()
489498
.to_string()
490-
.contains("Failed to get quarantine bulk test."));
499+
.contains("500 Internal Server Error"));
491500
assert_eq!(CALL_COUNT.load(Ordering::Relaxed), 6);
492501
}
493502

@@ -526,6 +535,6 @@ mod tests {
526535
.await
527536
.unwrap_err()
528537
.to_string()
529-
.contains("Quarantining config not found"));
538+
.contains("404 Not Found"));
530539
}
531540
}

cli-tests/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ tempfile = "3.2.0"
2424
test_utils = { path = "../test_utils" }
2525
tokio = { version = "*" }
2626
trunk-analytics-cli = { path = "../cli", features = ["force-sentry-env-dev"] }
27+
pretty_assertions = "0.6"
2728

2829
[features]
2930
default = []

cli-tests/src/upload.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -413,7 +413,7 @@ async fn upload_bundle_invalid_repo_root() {
413413
.assert()
414414
.failure()
415415
.stdout(predicate::str::contains(
416-
"error: Failed to open git repository at \"../\"",
416+
"Could not open the repo_root specified",
417417
));
418418
let requests = state.requests.lock().unwrap().clone();
419419
assert_eq!(requests.len(), 0);
@@ -438,7 +438,7 @@ async fn upload_bundle_invalid_repo_root_explicit() {
438438
.assert()
439439
.failure()
440440
.stdout(predicate::str::contains(
441-
"error: Failed to open git repository at",
441+
"Could not open the repo_root specified",
442442
));
443443
let requests = state.requests.lock().unwrap().clone();
444444
assert_eq!(requests.len(), 0);
@@ -694,7 +694,7 @@ async fn is_not_ok_on_bad_request() {
694694
command
695695
.assert()
696696
.failure()
697-
.stdout(predicate::str::contains("error: "));
697+
.stdout(predicate::str::contains("error"));
698698
}
699699

700700
#[tokio::test(flavor = "multi_thread")]

cli/src/error_report.rs

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,8 @@ pub fn log_error(error: &anyhow::Error, base_message: Option<&str>) -> i32 {
55
if let Some(io_error) = root_cause.downcast_ref::<std::io::Error>() {
66
if io_error.kind() == std::io::ErrorKind::ConnectionRefused {
77
tracing::warn!(
8-
"{}: {:?}",
9-
message(base_message, "could not connect to trunk's server"),
10-
error
8+
"{}",
9+
message(base_message, "could not connect to trunk's server")
1110
);
1211
return exitcode::OK;
1312
}
@@ -19,17 +18,14 @@ pub fn log_error(error: &anyhow::Error, base_message: Option<&str>) -> i32 {
1918
|| status == StatusCode::FORBIDDEN
2019
|| status == StatusCode::NOT_FOUND
2120
{
22-
tracing::warn!(
23-
"{}: {:?}",
24-
message(base_message, "unauthorized to access trunk"),
25-
error,
26-
);
21+
tracing::warn!("{}", message(base_message, "unauthorized to access trunk"),);
2722
return exitcode::SOFTWARE;
2823
}
2924
}
3025
}
3126

32-
tracing::error!("{}: {:?}", message(base_message, "error"), error,);
27+
tracing::error!("{}", message(base_message, "error"));
28+
tracing::error!(hidden_in_console = true, "Caused by error: {:#?}", error);
3329
exitcode::SOFTWARE
3430
}
3531

cli/src/main.rs

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use std::env;
33
use clap::{Parser, Subcommand};
44
use clap_verbosity_flag::{log::LevelFilter, InfoLevel, Verbosity};
55
use third_party::sentry;
6-
use tracing_subscriber::prelude::*;
6+
use tracing_subscriber::{filter::FilterFn, prelude::*};
77
use trunk_analytics_cli::{
88
context::gather_debug_props,
99
error_report::log_error,
@@ -128,13 +128,20 @@ fn setup_logger(log_level_filter: LevelFilter) -> anyhow::Result<()> {
128128
&tracing::Level::DEBUG => sentry_tracing::EventFilter::Breadcrumb,
129129
_ => sentry_tracing::EventFilter::Ignore,
130130
});
131+
132+
let console_layer = tracing_subscriber::fmt::Layer::new()
133+
.without_time()
134+
.with_target(false)
135+
.with_writer(std::io::stdout.with_max_level(to_trace_filter(log_level_filter)))
136+
.with_filter(FilterFn::new(|metadata| {
137+
!metadata
138+
.fields()
139+
.iter()
140+
.any(|field| field.name() == "hidden_in_console")
141+
}));
142+
131143
tracing_subscriber::registry()
132-
.with(
133-
tracing_subscriber::fmt::Layer::new()
134-
.without_time()
135-
.with_target(false)
136-
.with_writer(std::io::stdout.with_max_level(to_trace_filter(log_level_filter))),
137-
)
144+
.with(console_layer)
138145
.with(sentry_layer)
139146
.init();
140147
Ok(())

cli/src/validate_command.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,9 @@ async fn validate(
9191
None,
9292
)?;
9393
if file_set_builder.no_files_found() {
94-
return Err(anyhow::anyhow!("No test output files found to validate."));
94+
let msg = "No test output files found to validate.";
95+
tracing::warn!(msg);
96+
return Err(anyhow::anyhow!(msg));
9597
}
9698
print_matched_files(&file_set_builder);
9799

context/src/repo/mod.rs

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -77,17 +77,19 @@ impl BundleRepo {
7777

7878
#[cfg(feature = "git-access")]
7979
{
80-
let git_repo = repo_root
81-
// if a repo root is provided then use that
82-
// otherwise try to discover the repo root
83-
.map_or(gix::discover(current_dir).ok(), |dir| gix::open(dir).ok())
84-
.context(format!(
85-
"Failed to open git repository at {:?}",
86-
bundle_repo_options
87-
.repo_root
88-
.clone()
89-
.unwrap_or(PathBuf::from(""))
90-
))?;
80+
let git_repo = match repo_root {
81+
Some(manual_root) => gix::open(manual_root.clone()).map_err(|e| {
82+
tracing::warn!("Could not open the repo_root specified: {:?}", manual_root);
83+
anyhow::Error::from(e)
84+
})?,
85+
None => gix::discover(current_dir.clone()).map_err(|e| {
86+
tracing::warn!(
87+
"Current directory {:?} does not have a git repo",
88+
current_dir
89+
);
90+
anyhow::Error::from(e)
91+
})?,
92+
};
9193
tracing::info!("Using repo root at {:?}", git_repo.git_dir());
9294

9395
bundle_repo_options.repo_url = bundle_repo_options.repo_url.or_else(|| {

test_report/src/report.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ impl MutTestReport {
174174
)) {
175175
Ok(_) => true,
176176
Err(e) => {
177-
tracing::error!("Error uploading: {:?}", e);
177+
tracing::error!(hidden_in_console = true, "Error uploading: {:?}", e);
178178
false
179179
}
180180
}

0 commit comments

Comments
 (0)