From 485aeabff3dda9db93b262140f1696f7cc0f2c54 Mon Sep 17 00:00:00 2001
From: Anthony Eid <56899983+Anthony-Eid@users.noreply.github.com>
Date: Mon, 29 Jun 2026 10:31:45 -0400
Subject: [PATCH] reqwest_client: Drop stale connections with keepalives
 (#59929)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

## Summary

Tune the shared `ReqwestClient` builder so HTTP connections that have
silently gone bad on a flaky network path are detected and dropped
rather than reused.

A stale, reused HTTP/2 connection (after a NAT/conntrack timeout, a
silent reset, or a degraded path) is a common source of intermittent
TLS `BadRecordMac` errors against long-lived endpoints such as
`cloud.zed.dev`. The client already retries these (`HttpSend` is
retryable with exponential backoff), but users still see periodic
multi-second stalls and "connection error" warnings.

These settings make the client probe and recycle connections instead of
sending a request's first records into a connection that is already
dead:

- `tcp_keepalive(30s)` — surface dead TCP connections instead of reusing
them.
- `pool_idle_timeout(30s)` — bound how long an idle connection lingers
in the pool.
- `http2_keep_alive_interval(15s)` / `http2_keep_alive_timeout(10s)` /
`http2_keep_alive_while_idle(true)` — ping idle HTTP/2 connections so
broken ones are torn down.

All three constructors (`new`, `user_agent`, `proxy_and_user_agent`) go
through `builder()`, so every Zed HTTP client picks this up.

## Notes

- Values are conservative; they can be tightened if stale-connection
  errors persist.
- `tcp_keepalive_interval` is not yet available in the pinned
`zed-reqwest`
  fork rev, so only the initial keepalive idle time is set here.

## Verification

Behavior is network-dependent, so there is no deterministic test.
Verified
that the crate builds (`cargo check -p reqwest_client`). The change is
being validated empirically against a setup that reproduces the
`BadRecordMac` errors.

Release Notes:

- Improved resilience to intermittent network errors by detecting and
dropping stale HTTP connections instead of reusing them.
---
 crates/reqwest_client/src/reqwest_client.rs | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/crates/reqwest_client/src/reqwest_client.rs b/crates/reqwest_client/src/reqwest_client.rs
index 3239a48904a..60908d8a8d4 100644
--- a/crates/reqwest_client/src/reqwest_client.rs
+++ b/crates/reqwest_client/src/reqwest_client.rs
@@ -30,6 +30,15 @@ impl ReqwestClient {
         reqwest::Client::builder()
             .use_rustls_tls()
             .connect_timeout(Duration::from_secs(10))
+            // Detect and drop connections that have silently gone bad on a
+            // flaky path (NAT timeouts, resets) instead of reusing them. A
+            // stale reused HTTP/2 connection is a common source of
+            // `BadRecordMac` TLS errors against long-lived endpoints.
+            .tcp_keepalive(Duration::from_secs(30))
+            .pool_idle_timeout(Duration::from_secs(30))
+            .http2_keep_alive_interval(Duration::from_secs(15))
+            .http2_keep_alive_timeout(Duration::from_secs(10))
+            .http2_keep_alive_while_idle(true)
     }
 
     pub fn new() -> Self {