Skip to content

Commit 9e8f3de

Browse files
2bndy52bndy5
committed
improve log output for silent errors
1 parent fcb343f commit 9e8f3de

File tree

4 files changed

+45
-32
lines changed

4 files changed

+45
-32
lines changed

‎cpp-linter/src/rest_api/github/mod.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ impl RestApiClient for GithubApiClient {
126126
.join(if is_pr { pr.as_str() } else { sha.as_str() })?;
127127
let mut diff_header = HeaderMap::new();
128128
diff_header.insert("Accept", "application/vnd.github.diff".parse()?);
129+
log::debug!("Getting file changes from {}", url.as_str());
129130
let request = Self::make_api_request(
130131
&self.client,
131132
url.as_str(),
@@ -150,6 +151,8 @@ impl RestApiClient for GithubApiClient {
150151
} else {
151152
url
152153
};
154+
Self::log_response(response, "Failed to get full diff for event").await;
155+
log::debug!("Trying paginated request to {}", endpoint.as_str());
153156
self.get_changed_files_paginated(endpoint, file_filter)
154157
.await
155158
}

‎cpp-linter/src/rest_api/github/specific_api.rs

Lines changed: 28 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -186,12 +186,7 @@ impl GithubApiClient {
186186
.await
187187
{
188188
Ok(response) => {
189-
if let Err(e) = response.error_for_status_ref() {
190-
log::error!("Failed to post thread comment: {e:?}");
191-
if let Ok(text) = response.text().await {
192-
log::error!("{text}");
193-
}
194-
}
189+
Self::log_response(response, "Failed to post thread comment").await;
195190
}
196191
Err(e) => {
197192
log::error!("Failed to post thread comment: {e:?}");
@@ -227,11 +222,12 @@ impl GithubApiClient {
227222
.await
228223
{
229224
Ok(response) => {
230-
if let Err(e) = response.error_for_status_ref() {
231-
log::error!("Failed to get list of existing thread comments: {e:?}");
232-
if let Ok(text) = response.text().await {
233-
log::error!("{text}");
234-
}
225+
if !response.status().is_success() {
226+
Self::log_response(
227+
response,
228+
"Failed to get list of existing thread comments",
229+
)
230+
.await;
235231
return Ok(comment_url);
236232
}
237233
comments_url = Self::try_next_page(response.headers());
@@ -264,7 +260,7 @@ impl GithubApiClient {
264260
};
265261
let req = Self::make_api_request(
266262
&self.client,
267-
del_url.clone(),
263+
del_url.as_str(),
268264
Method::DELETE,
269265
None,
270266
None,
@@ -278,13 +274,12 @@ impl GithubApiClient {
278274
.await
279275
{
280276
Ok(result) => {
281-
if let Err(e) = result.error_for_status_ref() {
282-
log::error!(
283-
"Failed to delete old thread comment {e:?}"
284-
);
285-
if let Ok(text) = result.text().await {
286-
log::error!("{text}");
287-
}
277+
if !result.status().is_success() {
278+
Self::log_response(
279+
result,
280+
"Failed to delete old thread comment",
281+
)
282+
.await;
288283
}
289284
}
290285
Err(e) => {
@@ -336,7 +331,7 @@ impl GithubApiClient {
336331
0,
337332
)
338333
.await
339-
.with_context(|| "Failed to get PR info")?;
334+
.with_context(|| format!("Failed to get PR info from {}", url.as_str()))?;
340335
let pr_info: PullRequestInfo = serde_json::from_str(&response.text().await?)
341336
.with_context(|| "Failed to deserialize PR info")?;
342337

@@ -390,7 +385,7 @@ impl GithubApiClient {
390385
dismissal.await?; // free up the `url` variable
391386
let request = Self::make_api_request(
392387
&self.client,
393-
url,
388+
url.clone(),
394389
Method::POST,
395390
Some(
396391
serde_json::to_string(&payload)
@@ -407,11 +402,8 @@ impl GithubApiClient {
407402
.await
408403
{
409404
Ok(response) => {
410-
if let Err(e) = response.error_for_status_ref() {
411-
log::error!("Failed to post a new PR review: {e:?}");
412-
if let Ok(text) = response.text().await {
413-
log::error!("{text}");
414-
}
405+
if !response.status().is_success() {
406+
Self::log_response(response, "Failed to post a new PR review").await;
415407
}
416408
}
417409
Err(e) => {
@@ -442,7 +434,10 @@ impl GithubApiClient {
442434
)
443435
.await;
444436
if response.is_err() || response.as_ref().is_ok_and(|r| !r.status().is_success()) {
445-
log::error!("Failed to get a list of existing PR reviews");
437+
log::error!(
438+
"Failed to get a list of existing PR reviews: {}",
439+
endpoint.as_str()
440+
);
446441
return Ok(());
447442
}
448443
let response = response.unwrap();
@@ -481,11 +476,12 @@ impl GithubApiClient {
481476
.await
482477
{
483478
Ok(result) => {
484-
if let Err(e) = result.error_for_status_ref() {
485-
log::error!("Failed to dismiss outdated review: {e:?}");
486-
if let Ok(text) = result.text().await {
487-
log::error!("{text}");
488-
}
479+
if !result.status().is_success() {
480+
Self::log_response(
481+
result,
482+
"Failed to dismiss outdated review",
483+
)
484+
.await;
489485
}
490486
}
491487
Err(e) => {

‎cpp-linter/src/rest_api/mod.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,17 @@ pub trait RestApiClient {
298298
}
299299
None
300300
}
301+
302+
fn log_response(response: Response, context: &str) -> impl Future<Output = ()> + Send {
303+
async move {
304+
if let Err(e) = response.error_for_status_ref() {
305+
log::error!("{}: {e:?}", context.to_owned());
306+
if let Ok(text) = response.text().await {
307+
log::error!("{text}");
308+
}
309+
}
310+
}
311+
}
301312
}
302313

303314
fn make_format_comment(

‎cpp-linter/tests/paginated_changed_files.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use tempfile::{NamedTempFile, TempDir};
66

77
use cpp_linter::{
88
common_fs::FileFilter,
9+
logger,
910
rest_api::{github::GithubApiClient, RestApiClient},
1011
};
1112
use std::{env, io::Write, path::Path};
@@ -56,6 +57,8 @@ async fn get_paginated_changes(lib_root: &Path, event_type: EventType) {
5657
env::set_var("GITHUB_API_URL", server.url());
5758
env::set_current_dir(tmp.path()).unwrap();
5859
let gh_client = GithubApiClient::new().unwrap();
60+
logger::init().unwrap();
61+
log::set_max_level(log::LevelFilter::Debug);
5962

6063
let mut mocks = vec![];
6164
let diff_end_point = format!(

0 commit comments

Comments
 (0)