Skip to content

[HttpClientLight] Time out reading a body when the server goes silent - #5649

Open
adamsthws wants to merge 1 commit into
letscontrolit:megafrom
adamsthws:fix/httpclientlight-read-timeout
Open

adamsthws wants to merge 1 commit into
letscontrolit:megafrom
adamsthws:fix/httpclientlight-read-timeout

Conversation

@adamsthws

Copy link
Copy Markdown

HTTPClientLight::writeToStreamDataBlock() waits for the rest of a response body for as long as connected() holds, with no time limit. If the server stops sending part way through a body without closing the connection (its computer sleeps, a cable is pulled, the network drops), connected() stays true and getString() never returns. SendToHTTP with "wait for ack", and anything else reading a reply through HttpClientLight, then leaves the unit unresponsive until it is power-cycled.

This gives up after _tcpTimeout without data and returns HTTPC_ERROR_READ_TIMEOUT, as handleHeaderResponse() already does for the headers. Each piece of data resets the clock, so a slow body still completes, and setTimeout() still lets a caller wait longer.

HttpClientLight comes from Tasmota, where the same change was merged in arendst/Tasmota#25139. The body loop here was identical to Tasmota's before that change, so the patch is the same.

Testing: a sketch using ESPEasy's HttpClientLight (before and after this change) fetches each case from a test server from loop() while serving a web page, on an emulated ESP32-C3 (QEMU, arduino-esp32 3.3.12):

Server behaviour Before After
Body cut off part way, connection left open (Content-Length, no length, chunked) hangs, web page silent returns after 5.0 s; writeToStream() -11
Full body (Content-Length, server closes, chunked) completes completes
8 pieces 2 s apart completes completes
8 pieces 7 s apart completes times out after 5 s; completes with setTimeout(10000)

normal_ESP32c3_4M316k builds without warnings in this file.

writeToStreamDataBlock() waited for the rest of a body for as long as
connected() held. A server that stops sending part way through a body
without closing the connection leaves connected() true, so getString()
never returned and the unit stopped responding. Give up after
_tcpTimeout without data and return HTTPC_ERROR_READ_TIMEOUT, as
handleHeaderResponse() already does. Same fix as arendst/Tasmota#25139.
@adamsthws

Copy link
Copy Markdown
Author

For context, this came out of Sonoff-OTA, an over-the-air Sonoff S60 to Tasmota converter. The same stalled-download problem turned up in several other projects; the related pull requests, and how each fix was tested, are collected in docs/upstream-contributions.md.

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.

1 participant