Skip to content

Commit

Permalink
improve reqwest error handling (#923)
Browse files Browse the repository at this point in the history
Previously it was possible for reqwest to fail and cause a panic.
Now the actual error will be returned to the original caller.
  • Loading branch information
Byron committed Jul 26, 2023
1 parent 37ade02 commit d2d9df9
Showing 1 changed file with 21 additions and 11 deletions.
32 changes: 21 additions & 11 deletions gix-transport/src/client/blocking_io/http/reqwest/remote.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,20 @@ impl Default for Remote {

/// utilities
impl Remote {
fn restore_thread_after_failure(&mut self) -> http::Error {
let err_that_brought_thread_down = self
.handle
.take()
.expect("thread handle present")
.join()
.expect("handler thread should never panic")
.expect_err("something should have gone wrong with curl (we join on error only)");
*self = Remote::default();
http::Error::InitHttpClient {
source: Box::new(err_that_brought_thread_down),
}
}

fn make_request(
&mut self,
url: &str,
Expand All @@ -179,14 +193,18 @@ impl Remote {
None => continue,
};
}
self.request
if self
.request
.send(Request {
url: url.to_owned(),
headers: header_map,
upload_body_kind,
config: self.config.clone(),
})
.expect("the remote cannot be down at this point");
.is_err()
{
return Err(self.restore_thread_after_failure());
}

let Response {
headers,
Expand All @@ -195,15 +213,7 @@ impl Remote {
} = match self.response.recv() {
Ok(res) => res,
Err(_) => {
let err = self
.handle
.take()
.expect("always present")
.join()
.expect("no panic")
.expect_err("no receiver means thread is down with init error");
*self = Self::default();
return Err(http::Error::InitHttpClient { source: Box::new(err) });
return Err(self.restore_thread_after_failure());
}
};

Expand Down

0 comments on commit d2d9df9

Please sign in to comment.