[jay23606]

Problem

cloudflared update used http.Client{Timeout: 60s}, which caps the whole request, including reading the body. On slow links a download that is still making progress is cut off after 60 seconds with context deadline exceeded (Client.Timeout or context cancellation while reading body).

Change

• The download no longer has a total time limit. A stall timer is armed for clientTimeout (60s) and re-armed on every body read, so a download is aborted only when no data arrives for 60 seconds.
• If the stall timer fires before any response headers arrive, or while the body is read, the error is download stalled for 1m0s without data: ....
• Regression tests in cmd/cloudflared/updater/workers_update_test.go:
  • TestDownloadSlowBodyExceedsClientTimeout: a body that takes about 90 seconds overall, with gaps of 15 seconds, downloads completely.
  • TestDownloadStalledBodyFails: a body that stops after a partial write fails with the stall error.
  • TestDownloadNoResponseFails: a server that sends no headers fails with the stall error.

The slow-body test fails on the base commit with Client.Timeout or context cancellation while reading body, which is the reported error.

Not addressed: the issue also asks for several download retries. This change does not add retry logic.

Testing

• go test -count=1 -run TestDownloadSlowBodyExceedsClientTimeout ./cmd/cloudflared/updater/: passes with the fix. It fails on the base commit with Client.Timeout or context cancellation while reading body.
• go test -count=1 -run 'TestDownload' -v ./cmd/cloudflared/updater/: all three download tests pass.
• go vet ./cmd/cloudflared/updater/: no output.

The download tests take about 90 seconds in total, because they run in real time. make lint was not run in this session.

Regression test: fails on base three times (/Client.Timeout/), passes on fix three times (go test -count=1 -run TestDownloadSlowBodyExceedsClientTimeout ./cmd/cloudflared/updater/)

Fixes #1493

[agammann]

These regression tests use the production 60-second timeout. Even with t.Parallel, TestDownloadSlowBodyExceedsClientTimeout sleeps six times for 15 seconds, so every non-short updater test run now takes at least 90 seconds (the comment says about 75). Could the internal download helper accept a timeout parameter, with the production caller passing clientTimeout, so the progress and stall cases can use millisecond deadlines in tests? I reviewed the diff; I did not run tests locally.

[jay23606]

Thanks @agammann, addressed in 1091e80.

• download now takes a timeout time.Duration parameter. Apply passes clientTimeout, so production behaviour is unchanged.
• TestDownloadSlowBodyExceedsClientTimeout, TestDownloadStalledBodyFails and TestDownloadNoResponseFails use a 500ms deadline. The slow-body test still sends data at gaps of a quarter of the deadline, so the body outlasts the deadline while data keeps arriving.
• The testing.Short skips on those tests are removed, since they no longer wait for 60s.

go test -count=1 ./cmd/cloudflared/updater/ went from 90.08s to 0.83s here, and -short gives the same result. gofmt, go vet and the full package pass.

[agammann]

Thanks for the update. I rechecked head 1091e80: the production caller retains the 60-second timeout, while the progress and stall tests use a shorter injected deadline. That addresses the test-runtime concern I raised. I reviewed the diff but did not run the tests locally. AI-assisted review.