Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: Preserve original error message in warn! logs #36

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Feat: Preserve original error message in warn! logs #36

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
982ae5f
Feat: Preserve original error message in warn! logs
May 31, 2022
4ec212a
Refactor: Lift update into fn. Unnest everything
Jun 1, 2022
47183dc
Feat: Convert features response in 2 steps, log if error
Jun 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 35 additions & 22 deletions src/client.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -695,35 +695,48 @@ where
loop {
debug!("poll: retrieving features");
let res = self.http.get(&endpoint).recv_json().await;
if let Ok(res) = res {
let features: Features = res;
match self.memoize(features.features) {
Ok(None) => {}
Ok(Some(metrics)) => {
if !self.disable_metric_submission {
let mut metrics_uploaded = false;
let req = self.http.post(&metrics_endpoint);
if let Ok(body) = http_types::Body::from_json(&metrics) {
let res = req.body(body).await;
if let Ok(res) = res {
if res.status().is_success() {
metrics_uploaded = true;
debug!("poll: uploaded feature metrics")
match res {
Err(e) => warn!("poll: failed to retrieve features - {:?}", e),
Ok(res) => {
let features: Features = res;
match self.memoize(features.features) {
Ok(None) => {}
Ok(Some(metrics)) => {
if !self.disable_metric_submission {
let req = self.http.post(&metrics_endpoint);
match http_types::Body::from_json(&metrics) {
Err(e) => {
warn!(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like this pattern - it discards key ergonomics around error handling and will grow the code substantially - it has nearly doubled in size.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand what the previous code did that this iteration of the code does not do, that discards key ergonomics around error handling. If anything, the previous code discarding errors seems like an antipattern. Can you clarify what you mean?

"poll: error serializing metrics for upload - {:?}",
e
);
}
Ok(body) => {
let res = req.body(body).await;
match res {
Ok(res) => {
if res.status().is_success() {
debug!("poll: uploaded feature metrics")
}
}
Err(e) => {
warn!(
"poll: error uploading feature metrics - {:?}",
e
);
}
}
}
}
}
if !metrics_uploaded {
warn!("poll: error uploading feature metrics");
}
}
}
Err(_) => {
warn!("poll: failed to memoize features");
Err(e) => {
warn!("poll: failed to memoize features - {:?}", e);
}
}
}
} else {
warn!("poll: failed to retrieve features");
}

let duration = Duration::from_millis(self.interval);
debug!("poll: waiting {:?}", duration);
Delay::new(duration).await;
Expand Down