fix: remove initial global HTTP client usage (#20128)

This PR makes the initial steps at removing usage of the global Go HTTP
client, which was seen to have impacts on test flakiness in
https://github.com/coder/internal/issues/1020. The first commit removes
uses from tests, with the exception of one test that is tightly coupled
to the default client. The second commit makes easy/low-risk removals
from application code. This should have some impact to reduce test flakiness.
This commit is contained in:
Zach
2025-10-02 11:43:13 -06:00
committed by GitHub
parent 0d2ccacacd
commit 4d1003eace
18 changed files with 59 additions and 33 deletions
+2 -1
View File
@@ -2027,7 +2027,8 @@ func runSubAgentMain() int {
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
defer cancel()
req = req.WithContext(ctx)
resp, err := http.DefaultClient.Do(req)
client := &http.Client{}
resp, err := client.Do(req)
if err != nil {
_, _ = fmt.Fprintf(os.Stderr, "agent connection failed: %v\n", err)
return 11
+2 -1
View File
@@ -63,6 +63,7 @@ func NewAppHealthReporterWithClock(
// run a ticker for each app health check.
var mu sync.RWMutex
failures := make(map[uuid.UUID]int, 0)
client := &http.Client{}
for _, nextApp := range apps {
if !shouldStartTicker(nextApp) {
continue
@@ -91,7 +92,7 @@ func NewAppHealthReporterWithClock(
if err != nil {
return err
}
res, err := http.DefaultClient.Do(req)
res, err := client.Do(req)
if err != nil {
return err
}
+4 -2
View File
@@ -1254,8 +1254,9 @@ func TestServer(t *testing.T) {
t.Logf("error creating request: %s", err.Error())
return false
}
client := &http.Client{}
// nolint:bodyclose
res, err := http.DefaultClient.Do(req)
res, err := client.Do(req)
if err != nil {
t.Logf("error hitting prometheus endpoint: %s", err.Error())
return false
@@ -1316,8 +1317,9 @@ func TestServer(t *testing.T) {
t.Logf("error creating request: %s", err.Error())
return false
}
client := &http.Client{}
// nolint:bodyclose
res, err := http.DefaultClient.Do(req)
res, err := client.Do(req)
if err != nil {
t.Logf("error hitting prometheus endpoint: %s", err.Error())
return false
+2 -1
View File
@@ -1242,7 +1242,8 @@ func TestSSH(t *testing.T) {
// true exits the loop.
return true
}
resp, err := http.DefaultClient.Do(req)
client := &http.Client{}
resp, err := client.Do(req)
if err != nil {
t.Logf("HTTP GET http://localhost:8222/ %s", err)
return false
+1 -1
View File
@@ -55,7 +55,7 @@ func TestAccessURL(t *testing.T) {
defer cancel()
report.Run(ctx, &healthcheck.AccessURLReportOptions{
Client: nil, // defaults to http.DefaultClient
Client: &http.Client{},
AccessURL: nil,
})
+2 -1
View File
@@ -511,7 +511,8 @@ func tsDERPMap(ctx context.Context, t testing.TB) *tailcfg.DERPMap {
req, err := http.NewRequestWithContext(ctx, "GET", ipn.DefaultControlURL+"/derpmap/default", nil)
require.NoError(t, err)
res, err := http.DefaultClient.Do(req)
client := &http.Client{}
res, err := client.Do(req)
require.NoError(t, err)
defer res.Body.Close()
require.Equal(t, http.StatusOK, res.StatusCode)
+10 -8
View File
@@ -141,7 +141,8 @@ func TestMCPHTTP_E2E_UnauthenticatedAccess(t *testing.T) {
require.NoError(t, err, "Should be able to create HTTP request")
req.Header.Set("Content-Type", "application/json")
resp, err := http.DefaultClient.Do(req)
client := &http.Client{}
resp, err := client.Do(req)
require.NoError(t, err, "Should be able to make HTTP request")
defer resp.Body.Close()
@@ -613,7 +614,7 @@ func TestMCPHTTP_E2E_OAuth2_EndToEnd(t *testing.T) {
require.NoError(t, err)
tokenReq.Header.Set("Content-Type", "application/x-www-form-urlencoded")
tokenResp, err := http.DefaultClient.Do(tokenReq)
tokenResp, err := client.Do(tokenReq)
require.NoError(t, err)
defer tokenResp.Body.Close()
@@ -711,7 +712,7 @@ func TestMCPHTTP_E2E_OAuth2_EndToEnd(t *testing.T) {
require.NoError(t, err)
refreshReq.Header.Set("Content-Type", "application/x-www-form-urlencoded")
refreshResp, err := http.DefaultClient.Do(refreshReq)
refreshResp, err := client.Do(refreshReq)
require.NoError(t, err)
defer refreshResp.Body.Close()
@@ -846,7 +847,7 @@ func TestMCPHTTP_E2E_OAuth2_EndToEnd(t *testing.T) {
regReq.Header.Set("Content-Type", "application/json")
// Dynamic client registration should not require authentication (public endpoint)
regResp, err := http.DefaultClient.Do(regReq)
regResp, err := client.Do(regReq)
require.NoError(t, err)
defer regResp.Body.Close()
@@ -936,7 +937,7 @@ func TestMCPHTTP_E2E_OAuth2_EndToEnd(t *testing.T) {
require.NoError(t, err)
tokenReq.Header.Set("Content-Type", "application/x-www-form-urlencoded")
tokenResp, err := http.DefaultClient.Do(tokenReq)
tokenResp, err := client.Do(tokenReq)
require.NoError(t, err)
defer tokenResp.Body.Close()
@@ -1037,7 +1038,7 @@ func TestMCPHTTP_E2E_OAuth2_EndToEnd(t *testing.T) {
require.NoError(t, err)
refreshReq.Header.Set("Content-Type", "application/x-www-form-urlencoded")
refreshResp, err := http.DefaultClient.Do(refreshReq)
refreshResp, err := client.Do(refreshReq)
require.NoError(t, err)
defer refreshResp.Body.Close()
@@ -1151,7 +1152,8 @@ func TestMCPHTTP_E2E_OAuth2_EndToEnd(t *testing.T) {
require.NoError(t, err)
regReq1.Header.Set("Content-Type", "application/json")
regResp1, err := http.DefaultClient.Do(regReq1)
client := &http.Client{}
regResp1, err := client.Do(regReq1)
require.NoError(t, err)
defer regResp1.Body.Close()
@@ -1181,7 +1183,7 @@ func TestMCPHTTP_E2E_OAuth2_EndToEnd(t *testing.T) {
require.NoError(t, err)
regReq2.Header.Set("Content-Type", "application/json")
regResp2, err := http.DefaultClient.Do(regReq2)
regResp2, err := client.Do(regReq2)
require.NoError(t, err)
defer regResp2.Body.Close()
+4 -2
View File
@@ -29,7 +29,8 @@ func TestOAuth2AuthorizationServerMetadata(t *testing.T) {
req, err := http.NewRequestWithContext(ctx, http.MethodGet, endpoint, nil)
require.NoError(t, err)
resp, err := http.DefaultClient.Do(req)
httpClient := &http.Client{}
resp, err := httpClient.Do(req)
require.NoError(t, err)
defer resp.Body.Close()
@@ -65,7 +66,8 @@ func TestOAuth2ProtectedResourceMetadata(t *testing.T) {
req, err := http.NewRequestWithContext(ctx, http.MethodGet, endpoint, nil)
require.NoError(t, err)
resp, err := http.DefaultClient.Do(req)
httpClient := &http.Client{}
resp, err := httpClient.Do(req)
require.NoError(t, err)
defer resp.Body.Close()
+2 -1
View File
@@ -94,7 +94,8 @@ func TestInstrument(t *testing.T) {
must[*url.URL](t)(idp.IssuerURL().Parse("/.well-known/openid-configuration")).String(), nil)
require.NoError(t, err)
resp, err := http.DefaultClient.Do(req)
client := &http.Client{}
resp, err := client.Do(req)
require.NoError(t, err)
_ = resp.Body.Close()
+3 -1
View File
@@ -87,6 +87,7 @@ func New(options Options) (Reporter, error) {
deploymentURL: deploymentURL,
snapshotURL: snapshotURL,
startedAt: dbtime.Now(),
client: &http.Client{},
}
go reporter.runSnapshotter()
return reporter, nil
@@ -119,6 +120,7 @@ type remoteReporter struct {
snapshotURL *url.URL
startedAt time.Time
shutdownAt *time.Time
client *http.Client
}
func (r *remoteReporter) Enabled() bool {
@@ -142,7 +144,7 @@ func (r *remoteReporter) reportSync(snapshot *Snapshot) {
return
}
req.Header.Set(VersionHeader, buildinfo.Version())
resp, err := http.DefaultClient.Do(req)
resp, err := r.client.Do(req)
if err != nil {
// If the request fails it's not necessarily an error.
// In an airgapped environment, it's fine if this fails!
+6 -3
View File
@@ -42,7 +42,8 @@ func TestStreamAgentReinitEvents(t *testing.T) {
requestCtx := testutil.Context(t, testutil.WaitShort)
req, err := http.NewRequestWithContext(requestCtx, "GET", srv.URL, nil)
require.NoError(t, err)
resp, err := http.DefaultClient.Do(req)
client := &http.Client{}
resp, err := client.Do(req)
require.NoError(t, err)
defer resp.Body.Close()
@@ -77,7 +78,8 @@ func TestStreamAgentReinitEvents(t *testing.T) {
requestCtx := testutil.Context(t, testutil.WaitShort)
req, err := http.NewRequestWithContext(requestCtx, "GET", srv.URL, nil)
require.NoError(t, err)
resp, err := http.DefaultClient.Do(req)
client := &http.Client{}
resp, err := client.Do(req)
require.NoError(t, err)
defer resp.Body.Close()
@@ -110,7 +112,8 @@ func TestStreamAgentReinitEvents(t *testing.T) {
requestCtx := testutil.Context(t, testutil.WaitShort)
req, err := http.NewRequestWithContext(requestCtx, "GET", srv.URL, nil)
require.NoError(t, err)
resp, err := http.DefaultClient.Do(req)
client := &http.Client{}
resp, err := client.Do(req)
require.NoError(t, err)
defer resp.Body.Close()
@@ -495,6 +495,7 @@ func TestProvisionerDaemon_PrometheusEnabled(t *testing.T) {
// Fetch metrics from Prometheus endpoint
var req *http.Request
var res *http.Response
httpClient := &http.Client{}
require.Eventually(t, func() bool {
req, err = http.NewRequestWithContext(ctx, "GET", fmt.Sprintf("http://127.0.0.1:%d", prometheusPort), nil)
if err != nil {
@@ -503,7 +504,7 @@ func TestProvisionerDaemon_PrometheusEnabled(t *testing.T) {
}
// nolint:bodyclose
res, err = http.DefaultClient.Do(req)
res, err = httpClient.Do(req)
if err != nil {
t.Logf("unable to call Prometheus endpoint: %s", err.Error())
return false
+2 -1
View File
@@ -114,11 +114,12 @@ func TestWorkspaceProxy_Server_PrometheusEnabled(t *testing.T) {
// Fetch metrics from Prometheus endpoint
var res *http.Response
client := &http.Client{}
require.Eventually(t, func() bool {
req, err := http.NewRequestWithContext(ctx, "GET", fmt.Sprintf("http://127.0.0.1:%d", prometheusPort), nil)
assert.NoError(t, err)
// nolint:bodyclose
res, err = http.DefaultClient.Do(req)
res, err = client.Do(req)
return err == nil
}, testutil.WaitShort, testutil.IntervalFast)
defer res.Body.Close()
+2 -1
View File
@@ -43,13 +43,14 @@ func TestServer_Single(t *testing.T) {
)
clitest.Start(t, inv.WithContext(ctx))
accessURL := waitAccessURL(t, cfg)
client := &http.Client{}
require.Eventually(t, func() bool {
reqCtx := testutil.Context(t, testutil.IntervalMedium)
req, err := http.NewRequestWithContext(reqCtx, http.MethodGet, accessURL.String()+"/healthz", nil)
if err != nil {
panic(err)
}
resp, err := http.DefaultClient.Do(req)
resp, err := client.Do(req)
if err != nil {
t.Log("/healthz not ready yet")
return false
+2 -1
View File
@@ -595,6 +595,7 @@ func TestSCIMDisabled(t *testing.T) {
"/scim/v2/random/path/that/is/long.txt",
}
client := &http.Client{}
for _, p := range checkPaths {
t.Run(p, func(t *testing.T) {
t.Parallel()
@@ -605,7 +606,7 @@ func TestSCIMDisabled(t *testing.T) {
req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, u.String(), nil)
require.NoError(t, err)
resp, err := http.DefaultClient.Do(req)
resp, err := client.Do(req)
require.NoError(t, err)
defer resp.Body.Close()
require.Equal(t, http.StatusNotFound, resp.StatusCode)
+7 -4
View File
@@ -689,12 +689,13 @@ func TestWorkspaceProxyDERPMeshProbe(t *testing.T) {
t.Log("all replicas have pinged")
// Check they're all healthy according to /healthz-report.
httpClient := &http.Client{}
for _, proxy := range proxies {
// GET /healthz-report
u := proxy.ServerURL.ResolveReference(&url.URL{Path: "/healthz-report"})
req, err := http.NewRequestWithContext(ctx, http.MethodGet, u.String(), nil)
require.NoError(t, err)
resp, err := http.DefaultClient.Do(req)
resp, err := httpClient.Do(req)
require.NoError(t, err)
var respJSON codersdk.ProxyHealthReport
@@ -781,7 +782,8 @@ func TestWorkspaceProxyDERPMeshProbe(t *testing.T) {
u := proxy.ServerURL.ResolveReference(&url.URL{Path: "/healthz-report"})
req, err := http.NewRequestWithContext(ctx, http.MethodGet, u.String(), nil)
require.NoError(t, err)
resp, err := http.DefaultClient.Do(req)
httpClient := &http.Client{}
resp, err := httpClient.Do(req)
require.NoError(t, err)
var respJSON codersdk.ProxyHealthReport
@@ -869,7 +871,8 @@ func TestWorkspaceProxyDERPMeshProbe(t *testing.T) {
u := proxy.ServerURL.ResolveReference(&url.URL{Path: "/healthz-report"})
req, err := http.NewRequestWithContext(ctx, http.MethodGet, u.String(), nil)
require.NoError(t, err)
resp, err := http.DefaultClient.Do(req)
httpClient := &http.Client{}
resp, err := httpClient.Do(req)
require.NoError(t, err)
var respJSON codersdk.ProxyHealthReport
err = json.NewDecoder(resp.Body).Decode(&respJSON)
@@ -903,7 +906,7 @@ func TestWorkspaceProxyDERPMeshProbe(t *testing.T) {
// GET /healthz-report
req, err = http.NewRequestWithContext(ctx, http.MethodGet, u.String(), nil)
require.NoError(t, err)
resp, err = http.DefaultClient.Do(req)
resp, err = httpClient.Do(req)
require.NoError(t, err)
err = json.NewDecoder(resp.Body).Decode(&respJSON)
resp.Body.Close()
+2 -1
View File
@@ -70,6 +70,7 @@ func TestServeHTTP_FailureModes(t *testing.T) {
t.Parallel()
defaultHeaders := map[string]string{"Authorization": "Bearer key"}
httpClient := &http.Client{}
cases := []struct {
name string
@@ -155,7 +156,7 @@ func TestServeHTTP_FailureModes(t *testing.T) {
req.Header.Set(k, v)
}
resp, err := http.DefaultClient.Do(req)
resp, err := httpClient.Do(req)
t.Cleanup(func() {
if resp == nil || resp.Body == nil {
return
+4 -2
View File
@@ -232,6 +232,7 @@ func TestServingFiles(t *testing.T) {
Database: db,
}))
defer srv.Close()
client := &http.Client{}
// Create a context
ctx, cancelFunc := context.WithTimeout(context.Background(), testutil.WaitShort)
@@ -275,7 +276,7 @@ func TestServingFiles(t *testing.T) {
req, err := http.NewRequestWithContext(ctx, "GET", path, nil)
require.NoError(t, err)
resp, err := http.DefaultClient.Do(req)
resp, err := client.Do(req)
require.NoError(t, err, "get file")
data, _ := io.ReadAll(resp.Body)
require.Equal(t, string(data), testCase.expected, "Verify file: "+testCase.path)
@@ -521,6 +522,7 @@ func TestServingBin(t *testing.T) {
compressor := middleware.NewCompressor(1, "text/*", "application/*")
srv := httptest.NewServer(compressor.Handler(site))
defer srv.Close()
client := &http.Client{}
// Create a context
ctx, cancelFunc := context.WithTimeout(context.Background(), testutil.WaitShort)
@@ -538,7 +540,7 @@ func TestServingBin(t *testing.T) {
req.Header.Set("Accept-Encoding", "gzip")
}
resp, err := http.DefaultClient.Do(req)
resp, err := client.Do(req)
require.NoError(t, err, "http do failed")
defer resp.Body.Close()