fix: set timeouts on the upstream HTTP client #83

Merged

benvin merged 2 commits from benvin/upstream-http-timeouts into master 2026-07-02 22:24:50 +10:00

Conversation 1 Commits 2 Files Changed 6

+133 -9

unkinben commented 2026-07-02 00:26:38 +10:00

Owner

Copy Link

Copy Source

Fixes #67

Why

The proxy used http.DefaultClient for all upstream GET/HEAD and bearer-token requests. It has no timeouts, so a slow or hung upstream holds a goroutine and connection indefinitely.

Changes

• Add a shared upstreamClient (internal/proxy/httpclient.go) with dial, TLS-handshake, response-header and idle-connection timeouts, plus connection pooling.
• Deliberately no overall Client.Timeout, so large artifact bodies can still stream; total time is bounded by the request context.
• Route all four upstream calls in the engine through it.

Validation

• make e2e passes.

Fixes #67 ## Why The proxy used `http.DefaultClient` for all upstream GET/HEAD and bearer-token requests. It has no timeouts, so a slow or hung upstream holds a goroutine and connection indefinitely. ## Changes - Add a shared `upstreamClient` (`internal/proxy/httpclient.go`) with dial, TLS-handshake, response-header and idle-connection timeouts, plus connection pooling. - Deliberately no overall `Client.Timeout`, so large artifact bodies can still stream; total time is bounded by the request context. - Route all four upstream calls in the engine through it. ## Validation - `make e2e` passes.

unkinben added 1 commit 2026-07-02 00:26:38 +10:00

fix: use a timeout-configured HTTP client for upstream requests ... 1476120c7b

All upstream GET/HEAD and bearer-token requests used http.DefaultClient,
which has no timeouts, so a slow or wedged upstream could pin a goroutine
and connection indefinitely. Introduce a shared upstreamClient with
dial, TLS-handshake and response-header timeouts (no overall Client
timeout, so large artifact bodies can still stream, bounded by the
request context).

Refs #67

unkinben added 1 commit 2026-07-02 20:16:54 +10:00

feat: make upstream timeouts configurable per-remote ... 1e879126d7

Keep the dial/TLS/response-header timeouts as defaults, but allow each
remote to override them via upstream_dial_timeout / upstream_tls_timeout /
upstream_response_header_timeout (seconds; 0 = default). Clients are cached
by their timeout set so remotes sharing a configuration also share a
connection pool.

Refs #67

unkinben commented 2026-07-02 20:17:06 +10:00

Author

Owner

Copy Link

Copy Source

Extended per review feedback: the dial/TLS/response-header timeouts remain the defaults, and are now overridable per-remote via upstream_dial_timeout / upstream_tls_timeout / upstream_response_header_timeout (seconds; 0 = default). Clients are cached by timeout set so remotes sharing a config share a connection pool. New e2e test TestRemoteUpstreamTimeouts covers the round-trip; make e2e passes.

Extended per review feedback: the dial/TLS/response-header timeouts remain the defaults, and are now overridable per-remote via `upstream_dial_timeout` / `upstream_tls_timeout` / `upstream_response_header_timeout` (seconds; 0 = default). Clients are cached by timeout set so remotes sharing a config share a connection pool. New e2e test `TestRemoteUpstreamTimeouts` covers the round-trip; `make e2e` passes.

benvin merged commit f61ab99ae8 into master 2026-07-02 22:24:50 +10:00

benvin deleted branch benvin/upstream-http-timeouts 2026-07-02 22:24:50 +10:00

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

No items

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

benvin (BenVincent) unkinben (Ben Vincent)

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: unkin/artifactapi#83