fix(http): Gmail/Photos/Slides/Zoom hang when a peer never answers - #995
fix(http): Gmail/Photos/Slides/Zoom hang when a peer never answers#995SebTardif wants to merge 1 commit into
Conversation
Unauthenticated Gmail tracking, Photos downloads, Slides thumbnails, and Zoom API calls used http.DefaultClient and could hang forever on a silent peer. Reuse the existing 30s ResponseHeaderTimeout transport for those fetches, and give Zoom a 30s client timeout. Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
|
🦞👀 Pull request received. I will update this pull request when review starts. |
|
Codex review: needs maintainer review before merge. Reviewed August 15, 2026, 6:43 AM ET / 10:43 UTC. ClawSweeper reviewWhat this changesThe PR replaces unbounded Gmail tracking, Photos, Slides, and Zoom HTTP clients with 30-second bounded clients and adds focused coverage. Merge readinessKeep open: current v0.37.0 still uses unbounded default HTTP clients on the affected paths, while this PR applies the repository’s established 30-second response-header policy and supplies a live silent-peer trace. No blocking patch defect was found; a maintainer should explicitly accept the bounded-availability tradeoff before merge. Priority: P2 Review scores
Verification
How this fits togetherSeveral gog CLI commands make direct HTTP requests after obtaining API data or credentials. This change determines whether a nonresponsive remote peer yields a bounded error or leaves the command waiting indefinitely. flowchart LR
A[CLI command] --> B[HTTP request]
B --> C{Service path}
C --> D[Google fetch transport]
C --> E[Zoom JSON client]
D --> F[Remote service]
E --> F
F --> G[Response or timeout error]
Decision needed
Why: The implementation matches an established authenticated-Google policy, but it intentionally changes the failure mode for existing slow peers. Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Adopt the existing 30-second Google response-header policy for the remaining unauthenticated fetches and retain Zoom’s narrow 30-second JSON-client bound. Do we have a high-confidence way to reproduce the issue? Yes: a silent TCP listener that accepts a request without returning headers is a high-confidence path, and current main’s affected callers use http.DefaultClient while the PR routes them to a 30-second bounded client. Is this the best way to solve the issue? Yes: reusing the established Google transport avoids truncating media after headers arrive, while Zoom only handles token and small JSON operations with its full timeout. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 45b5d766e137. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
History |
What Problem This Solves
Fixes an issue where users running Gmail tracking queries, Photos media downloads, Slides thumbnail downloads, or Zoom API calls would hang forever when the remote peer accepted TCP but never sent response headers.
Why This Change Was Made
Those paths used
http.DefaultClient(no timeout). Authenticated Google clients already boundResponseHeaderTimeoutat 30s without a fullClient.Timeout, so large downloads are not cut short after headers arrive. This change reuses that transport for the leftover unauthenticated fetches, and gives Zoom a 30s client timeout for its small JSON API.User Impact
Those commands now fail with a timeout error after about 30 seconds instead of hanging until the process is killed.
Evidence
terminal output from a live silent peer on
127.0.0.1:62317.curlshows the peer never answers.NewBoundedHTTPClient(the same client the patched call sites now use) returns after 30s withtimeout awaiting response headers:Real behavior proof
go.mod, patched worktree at/tmp/gogcli-http-timeout, silent TCP listener on127.0.0.1:62317.curl --max-time 2to confirm the peer is silent. Rango runagainstgoogleapi.NewBoundedHTTPClient().Dofor that URL.headerTimeout=30sand returnednet/http: timeout awaiting response headersat 30.002s.NewClientuses a 30sTimeoutinstead ofDefaultClient.Related
responseHeaderTimeouttransport ininternal/googleapi/client.goandgog update status --timeout.