Skip to content

Read upload responses to completion - #59

Merged
kavehtehrani merged 1 commit into
kavehtehrani:mainfrom
agoodkind:fix/read-upload-responses
Oct 26, 2026
Merged

kavehtehrani merged 1 commit into
kavehtehrani:mainfrom
agoodkind:fix/read-upload-responses

Conversation

@agoodkind

Copy link
Copy Markdown
Contributor

Problem

Upload tests can underuse available bandwidth because the worker discards successful HTTP responses without consuming their bodies. For HTTP/1.1, Hyper closes the connection if its cleanup cannot immediately finish reading the reply.

In a dual-WAN test, this repeatedly closed connections on one WAN and concentrated subsequent uploads on the other.

Changes

Read each successful upload response to completion before starting the next request. Treat response-body read errors as upload failures.

Testing

A regression test runs the production upload worker against a local HTTP server that delays the final response chunk and checks connection reuse.

Manual macOS tests on v1.0.8 alternated original and corrected builds using IPv4, six workers, and 5 MB uploads:

Response handling WAN B share of completed uploads Upload
Original, two runs 11% to 14% 992 to 994 Mbps
Read complete responses, two runs 46% to 67% 1,208 to 1,497 Mbps

Packet captures and temporary HTTP-library instrumentation confirmed that the original cleanup closed connections while reply completion remained pending. Reading complete responses prevented those early closures in the comparison runs.

Clippy with Rust 1.95.0 reports the same 13 errors in existing networking and TUI code on this branch and unchanged upstream main.

Co-authored-by: Codex <noreply@openai.com>
@agoodkind

Copy link
Copy Markdown
Contributor Author

Hi @kavehtehrani, please review #59. It fixes premature upload connection closures and includes a regression test plus measurements from a dual-WAN setup.

@kavehtehrani
kavehtehrani merged commit 0a6ba83 into kavehtehrani:main Oct 26, 2026
@kavehtehrani

Copy link
Copy Markdown
Owner

@agoodkind merged and released in v1.0.11. tx for the pr.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants