Skip to content

Commit ee6f087

Browse files
notheotherbenclaude
andcommitted
refactor: reuse the central HTTP client for self-update requests
Instead of the updater constructing its own reqwest client, hand it Git-Tool's central one. `update::manager` now takes `&Core` and passes `core.http_client().reqwest_client()` to update-rs, so self-update requests share Git-Tool's client configuration and connection pool. To make that possible the central `TrueHttpClient` now owns a single reqwest client (built once, with a default `Git-Tool/<version>` User-Agent) rather than creating one per request, and the `HttpClient` trait exposes it via `reqwest_client()`. Call sites that set a per-request User-Agent (the GitHub registry/service) still override the default, so their behaviour is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 15d0de4 commit ee6f087

4 files changed

Lines changed: 44 additions & 27 deletions

File tree

src/commands/open.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -131,8 +131,8 @@ New applications can be configured either by making changes to your configuratio
131131
}
132132

133133
impl OpenCommand {
134-
#[tracing::instrument(err, skip(self, _core))]
135-
async fn check_for_update(&self, _core: &Core) -> Result<Option<Release>, human_errors::Error> {
134+
#[tracing::instrument(err, skip(self, core))]
135+
async fn check_for_update(&self, core: &Core) -> Result<Option<Release>, human_errors::Error> {
136136
let current_version: semver::Version = version!().parse().map_err(|err| human_errors::wrap_system(
137137
err,
138138
"Could not parse the current application version into a SemVer version number.",
@@ -141,7 +141,7 @@ impl OpenCommand {
141141

142142
info!("Current application version is v{}", current_version);
143143

144-
let releases = crate::update::manager().get_releases().await?;
144+
let releases = crate::update::manager(core).get_releases().await?;
145145

146146
let target_release = Release::get_latest(releases.iter().filter(|&r| {
147147
r.get_variant().is_some() && r.version > current_version && !r.prerelease

src/commands/update.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ impl CommandRunnable for UpdateCommand {
3636

3737
#[tracing::instrument(name = "gt update", err, skip(self, core, matches))]
3838
async fn run(&self, core: &Core, matches: &ArgMatches) -> Result<i32, engine::Error> {
39-
let manager = crate::update::manager();
39+
let manager = crate::update::manager(core);
4040

4141
// When the updater relaunches us between phases it invokes
4242
// `gt update --state <json>`; hand that straight back to the updater to
@@ -125,10 +125,10 @@ impl CommandRunnable for UpdateCommand {
125125

126126
#[tracing::instrument(
127127
name = "gt complete -- gt update",
128-
skip(self, _core, completer, _matches)
128+
skip(self, core, completer, _matches)
129129
)]
130-
async fn complete(&self, _core: &Core, completer: &Completer, _matches: &ArgMatches) {
131-
if let Ok(releases) = crate::update::manager().get_releases().await {
130+
async fn complete(&self, core: &Core, completer: &Completer, _matches: &ArgMatches) {
131+
if let Ok(releases) = crate::update::manager(core).get_releases().await {
132132
completer.offer_many(
133133
releases
134134
.iter()

src/engine/http.rs

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,34 @@ pub trait HttpClient: Send + Sync {
2020
}
2121

2222
async fn request(&self, req: Request) -> Result<Response, human_errors::Error>;
23+
24+
/// The underlying [`reqwest::Client`], for code that needs a real client
25+
/// rather than this trait — e.g. the self-updater, which hands a
26+
/// `reqwest::Client` to `update-rs`. It shares this client's configuration
27+
/// and connection pool.
28+
fn reqwest_client(&self) -> Client;
2329
}
2430

2531
pub fn client() -> Arc<dyn HttpClient + Send + Sync> {
26-
Arc::new(TrueHttpClient {})
32+
Arc::new(TrueHttpClient {
33+
client: build_client(),
34+
})
35+
}
36+
37+
/// Build Git-Tool's shared reqwest client, carrying a default `User-Agent` so
38+
/// requests that don't set their own are still attributed to Git-Tool (GitHub
39+
/// requires one). Call sites that set a per-request `User-Agent` still override
40+
/// it.
41+
fn build_client() -> Client {
42+
Client::builder()
43+
.user_agent(version!("Git-Tool/"))
44+
.build()
45+
.unwrap_or_else(|_| Client::new())
2746
}
2847

29-
struct TrueHttpClient {}
48+
struct TrueHttpClient {
49+
client: Client,
50+
}
3051

3152
impl std::fmt::Debug for dyn HttpClient {
3253
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
@@ -58,9 +79,7 @@ impl HttpClient for TrueHttpClient {
5879
)
5980
)]
6081
async fn request(&self, req: Request) -> Result<Response, human_errors::Error> {
61-
let client = Client::new();
62-
63-
let response = client.execute(req).await.to_human_error()?;
82+
let response = self.client.execute(req).await.to_human_error()?;
6483

6584
if !response.status().is_success() {
6685
Span::current().record("otel.status_code", 2_u32).record(
@@ -78,11 +97,17 @@ impl HttpClient for TrueHttpClient {
7897

7998
Ok(response)
8099
}
100+
101+
fn reqwest_client(&self) -> Client {
102+
self.client.clone()
103+
}
81104
}
82105

83106
impl From<Arc<Config>> for TrueHttpClient {
84107
fn from(_: Arc<Config>) -> Self {
85-
Self {}
108+
Self {
109+
client: build_client(),
110+
}
86111
}
87112
}
88113

src/update.rs

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
99
pub use update_rs::Release;
1010

11+
use crate::engine::Core;
1112
use std::ffi::OsString;
1213
use update_rs::{GitHubSource, Launcher, UpdateManager, naming};
1314

@@ -29,27 +30,18 @@ impl Launcher for GitToolLauncher {
2930
}
3031
}
3132

32-
/// Build the HTTP client the updater uses, carrying Git-Tool's `User-Agent` (as
33-
/// GitHub requires) so self-update requests are attributed to Git-Tool rather
34-
/// than to update-rs's generic default client.
35-
fn http_client() -> reqwest::Client {
36-
reqwest::Client::builder()
37-
.user_agent(version!("Git-Tool/"))
38-
.build()
39-
.unwrap_or_else(|_| reqwest::Client::new())
40-
}
41-
4233
/// Build an [`UpdateManager`] configured for Git-Tool's releases.
4334
///
4435
/// It downloads the Go-style `git-tool-<os>-<arch>[.exe]` asset for the current
4536
/// platform (matching the names produced by `.github/workflows/release.yml`)
46-
/// from the project's GitHub releases, whose tags are `vX.Y.Z`, using Git-Tool's
47-
/// own HTTP client, and relaunches through the `update --state` sub-command.
48-
pub fn manager() -> UpdateManager<GitHubSource> {
37+
/// from the project's GitHub releases, whose tags are `vX.Y.Z`, reusing
38+
/// Git-Tool's shared HTTP client (from `core`), and relaunches through the
39+
/// `update --state` sub-command.
40+
pub fn manager(core: &Core) -> UpdateManager<GitHubSource> {
4941
UpdateManager::new(
5042
GitHubSource::new(REPO, naming::go("git-tool"))
5143
.with_release_tag_prefix("v")
52-
.with_client(http_client()),
44+
.with_client(core.http_client().reqwest_client()),
5345
)
5446
.with_launcher(Box::new(GitToolLauncher))
5547
}

0 commit comments

Comments
 (0)