mirror of
https://github.com/esphome/esphome.git
synced 2026-02-08 08:41:59 +00:00
[http_request] Fix requests taking full timeout when response is already complete
This commit is contained in:
@@ -196,11 +196,12 @@ bool Esp32HostedUpdate::fetch_manifest_() {
|
||||
int read_or_error = container->read(buf, sizeof(buf));
|
||||
App.feed_wdt();
|
||||
yield();
|
||||
auto result = http_request::http_read_loop_result(read_or_error, last_data_time, read_timeout);
|
||||
auto result =
|
||||
http_request::http_read_loop_result(read_or_error, last_data_time, read_timeout, container->is_read_complete());
|
||||
if (result == http_request::HttpReadLoopResult::RETRY)
|
||||
continue;
|
||||
if (result != http_request::HttpReadLoopResult::DATA)
|
||||
break; // ERROR or TIMEOUT
|
||||
break; // COMPLETE, ERROR, or TIMEOUT
|
||||
json_str.append(reinterpret_cast<char *>(buf), read_or_error);
|
||||
}
|
||||
container->end();
|
||||
@@ -321,9 +322,12 @@ bool Esp32HostedUpdate::stream_firmware_to_coprocessor_() {
|
||||
App.feed_wdt();
|
||||
yield();
|
||||
|
||||
auto result = http_request::http_read_loop_result(read_or_error, last_data_time, read_timeout);
|
||||
auto result =
|
||||
http_request::http_read_loop_result(read_or_error, last_data_time, read_timeout, container->is_read_complete());
|
||||
if (result == http_request::HttpReadLoopResult::RETRY)
|
||||
continue;
|
||||
if (result == http_request::HttpReadLoopResult::COMPLETE)
|
||||
break; // All data received
|
||||
if (result != http_request::HttpReadLoopResult::DATA) {
|
||||
if (result == http_request::HttpReadLoopResult::TIMEOUT) {
|
||||
ESP_LOGE(TAG, "Timeout reading firmware data");
|
||||
|
||||
@@ -126,19 +126,21 @@ struct HttpReadResult {
|
||||
|
||||
/// Result of processing a non-blocking read with timeout (for manual loops)
|
||||
enum class HttpReadLoopResult : uint8_t {
|
||||
DATA, ///< Data was read, process it
|
||||
RETRY, ///< No data yet, already delayed, caller should continue loop
|
||||
ERROR, ///< Read error, caller should exit loop
|
||||
TIMEOUT, ///< Timeout waiting for data, caller should exit loop
|
||||
DATA, ///< Data was read, process it
|
||||
COMPLETE, ///< All content has been read, caller should exit loop
|
||||
RETRY, ///< No data yet, already delayed, caller should continue loop
|
||||
ERROR, ///< Read error, caller should exit loop
|
||||
TIMEOUT, ///< Timeout waiting for data, caller should exit loop
|
||||
};
|
||||
|
||||
/// Process a read result with timeout tracking and delay handling
|
||||
/// @param bytes_read_or_error Return value from read() - positive for bytes read, negative for error
|
||||
/// @param last_data_time Time of last successful read, updated when data received
|
||||
/// @param timeout_ms Maximum time to wait for data
|
||||
/// @return DATA if data received, RETRY if should continue loop, ERROR/TIMEOUT if should exit
|
||||
inline HttpReadLoopResult http_read_loop_result(int bytes_read_or_error, uint32_t &last_data_time,
|
||||
uint32_t timeout_ms) {
|
||||
/// @param is_read_complete Whether all expected content has been read (from HttpContainer::is_read_complete())
|
||||
/// @return How the caller should proceed - see HttpReadLoopResult enum
|
||||
inline HttpReadLoopResult http_read_loop_result(int bytes_read_or_error, uint32_t &last_data_time, uint32_t timeout_ms,
|
||||
bool is_read_complete) {
|
||||
if (bytes_read_or_error > 0) {
|
||||
last_data_time = millis();
|
||||
return HttpReadLoopResult::DATA;
|
||||
@@ -146,7 +148,10 @@ inline HttpReadLoopResult http_read_loop_result(int bytes_read_or_error, uint32_
|
||||
if (bytes_read_or_error < 0) {
|
||||
return HttpReadLoopResult::ERROR;
|
||||
}
|
||||
// bytes_read_or_error == 0: no data available yet
|
||||
// bytes_read_or_error == 0: either "no data yet" or "all content read"
|
||||
if (is_read_complete) {
|
||||
return HttpReadLoopResult::COMPLETE;
|
||||
}
|
||||
if (millis() - last_data_time >= timeout_ms) {
|
||||
return HttpReadLoopResult::TIMEOUT;
|
||||
}
|
||||
@@ -197,6 +202,12 @@ class HttpContainer : public Parented<HttpRequestComponent> {
|
||||
|
||||
size_t get_bytes_read() const { return this->bytes_read_; }
|
||||
|
||||
/// Check if all expected content has been read (only valid for non-chunked responses)
|
||||
/// For chunked responses (content_length == 0 on ESP-IDF, SIZE_MAX on Arduino), returns false
|
||||
bool is_read_complete() const {
|
||||
return this->content_length > 0 && this->content_length < SIZE_MAX && this->bytes_read_ >= this->content_length;
|
||||
}
|
||||
|
||||
/**
|
||||
* @brief Get response headers.
|
||||
*
|
||||
@@ -231,9 +242,11 @@ inline HttpReadResult http_read_fully(HttpContainer *container, uint8_t *buffer,
|
||||
App.feed_wdt();
|
||||
yield();
|
||||
|
||||
auto result = http_read_loop_result(read_bytes_or_error, last_data_time, timeout_ms);
|
||||
auto result = http_read_loop_result(read_bytes_or_error, last_data_time, timeout_ms, container->is_read_complete());
|
||||
if (result == HttpReadLoopResult::RETRY)
|
||||
continue;
|
||||
if (result == HttpReadLoopResult::COMPLETE)
|
||||
break; // Server sent less data than requested, but transfer is complete
|
||||
if (result == HttpReadLoopResult::ERROR)
|
||||
return {HttpReadStatus::ERROR, read_bytes_or_error};
|
||||
if (result == HttpReadLoopResult::TIMEOUT)
|
||||
@@ -393,11 +406,12 @@ template<typename... Ts> class HttpRequestSendAction : public Action<Ts...> {
|
||||
int read_or_error = container->read(buf + read_index, std::min<size_t>(max_length - read_index, 512));
|
||||
App.feed_wdt();
|
||||
yield();
|
||||
auto result = http_read_loop_result(read_or_error, last_data_time, read_timeout);
|
||||
auto result =
|
||||
http_read_loop_result(read_or_error, last_data_time, read_timeout, container->is_read_complete());
|
||||
if (result == HttpReadLoopResult::RETRY)
|
||||
continue;
|
||||
if (result != HttpReadLoopResult::DATA)
|
||||
break; // ERROR or TIMEOUT
|
||||
break; // COMPLETE, ERROR, or TIMEOUT
|
||||
read_index += read_or_error;
|
||||
}
|
||||
response_body.reserve(read_index);
|
||||
|
||||
@@ -135,6 +135,18 @@ std::shared_ptr<HttpContainer> HttpRequestArduino::perform(const std::string &ur
|
||||
// When cast to size_t, -1 becomes SIZE_MAX (4294967295 on 32-bit).
|
||||
// The read() method handles this: bytes_read_ can never reach SIZE_MAX, so the
|
||||
// early return check (bytes_read_ >= content_length) will never trigger.
|
||||
//
|
||||
// TODO: Chunked transfer encoding is NOT properly supported on Arduino.
|
||||
// The implementation in #7884 was incomplete - it only works correctly on ESP-IDF where
|
||||
// esp_http_client_read() decodes chunks internally. On Arduino, using getStreamPtr()
|
||||
// returns raw TCP data with chunk framing (e.g., "12a\r\n{json}\r\n0\r\n\r\n") instead
|
||||
// of decoded content. This wasn't noticed because requests would complete and payloads
|
||||
// were only examined on IDF. The long transfer times were also masked by the misleading
|
||||
// "HTTP on Arduino version >= 3.1 is **very** slow" warning above. This causes two issues:
|
||||
// 1. Response body is corrupted - contains chunk size headers mixed with data
|
||||
// 2. Cannot detect end of transfer - connection stays open (keep-alive), causing timeout
|
||||
// The proper fix would be to use getString() for chunked responses, which decodes chunks
|
||||
// internally, but this buffers the entire response in memory.
|
||||
int content_length = container->client_.getSize();
|
||||
ESP_LOGD(TAG, "Content-Length: %d", content_length);
|
||||
container->content_length = (size_t) content_length;
|
||||
@@ -178,9 +190,9 @@ int HttpContainerArduino::read(uint8_t *buf, size_t max_len) {
|
||||
|
||||
if (bufsize == 0) {
|
||||
this->duration_ms += (millis() - start);
|
||||
// Check if we've read all expected content (only valid when content_length is known and not SIZE_MAX)
|
||||
// For chunked encoding (content_length == SIZE_MAX), we can't use this check
|
||||
if (this->content_length > 0 && this->bytes_read_ >= this->content_length) {
|
||||
// Check if we've read all expected content (non-chunked only)
|
||||
// For chunked encoding (content_length == SIZE_MAX), is_read_complete() returns false
|
||||
if (this->is_read_complete()) {
|
||||
return 0; // All content read successfully
|
||||
}
|
||||
// No data available - check if connection is still open
|
||||
|
||||
@@ -239,10 +239,9 @@ int HttpContainerIDF::read(uint8_t *buf, size_t max_len) {
|
||||
const uint32_t start = millis();
|
||||
watchdog::WatchdogManager wdm(this->parent_->get_watchdog_timeout());
|
||||
|
||||
// Check if we've already read all expected content
|
||||
// Skip this check when content_length is 0 (chunked transfer encoding or unknown length)
|
||||
// For chunked responses, esp_http_client_read() will return 0 when all data is received
|
||||
if (this->content_length > 0 && this->bytes_read_ >= this->content_length) {
|
||||
// Check if we've already read all expected content (non-chunked only)
|
||||
// For chunked responses (content_length == 0), esp_http_client_read() handles EOF
|
||||
if (this->is_read_complete()) {
|
||||
return 0; // All content read successfully
|
||||
}
|
||||
|
||||
|
||||
@@ -130,9 +130,11 @@ uint8_t OtaHttpRequestComponent::do_ota_() {
|
||||
App.feed_wdt();
|
||||
yield();
|
||||
|
||||
auto result = http_read_loop_result(bufsize_or_error, last_data_time, read_timeout);
|
||||
auto result = http_read_loop_result(bufsize_or_error, last_data_time, read_timeout, container->is_read_complete());
|
||||
if (result == HttpReadLoopResult::RETRY)
|
||||
continue;
|
||||
if (result == HttpReadLoopResult::COMPLETE)
|
||||
break; // All data received
|
||||
if (result != HttpReadLoopResult::DATA) {
|
||||
if (result == HttpReadLoopResult::TIMEOUT) {
|
||||
ESP_LOGE(TAG, "Timeout reading data");
|
||||
|
||||
Reference in New Issue
Block a user