[http_request] Fix OTA failures on ESP8266/Arduino by making read semantics consistent

esphome/components/http_request/http_request.h

@@ -79,6 +79,49 @@ inline bool is_redirect(int const status) {
  */
 inline bool is_success(int const status) { return status >= HTTP_STATUS_OK && status < HTTP_STATUS_MULTIPLE_CHOICES; }
 
+/// Status of a read operation
+enum class HttpReadStatus : uint8_t {
+  OK,       ///< Read completed successfully
+  ERROR,    ///< Read error occurred
+  TIMEOUT,  ///< Timeout waiting for data
+};
+
+/// Result of an HTTP read operation
+struct HttpReadResult {
+  HttpReadStatus status;  ///< Status of the read operation
+  int error_code;         ///< Error code from read() on failure, 0 on success
+};
+
+/// 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
+};
+
+/// 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) {
+  if (bytes_read_or_error > 0) {
+    last_data_time = millis();
+    return HttpReadLoopResult::DATA;
+  }
+  if (bytes_read_or_error < 0) {
+    return HttpReadLoopResult::ERROR;
+  }
+  // bytes_read_or_error == 0: no data available yet
+  if (millis() - last_data_time >= timeout_ms) {
+    return HttpReadLoopResult::TIMEOUT;
+  }
+  delay(1);  // Small delay to prevent tight spinning
+  return HttpReadLoopResult::RETRY;
+}
+
 class HttpRequestComponent;
 
 class HttpContainer : public Parented<HttpRequestComponent> {
@@ -110,6 +153,38 @@ class HttpContainer : public Parented<HttpRequestComponent> {
   std::map<std::string, std::list<std::string>> response_headers_{};
 };
 
+/// Read data from HTTP container into buffer with timeout handling
+/// Handles feed_wdt, yield, and timeout checking internally
+/// @param container The HTTP container to read from
+/// @param buffer Buffer to read into
+/// @param total_size Total bytes to read
+/// @param chunk_size Maximum bytes per read call
+/// @param timeout_ms Read timeout in milliseconds
+/// @return HttpReadResult with status and error_code on failure
+inline HttpReadResult http_read_fully(HttpContainer *container, uint8_t *buffer, size_t total_size, size_t chunk_size,
+                                      uint32_t timeout_ms) {
+  size_t read_index = 0;
+  uint32_t last_data_time = millis();
+
+  while (read_index < total_size) {
+    int read_bytes_or_error = container->read(buffer + read_index, std::min(chunk_size, total_size - read_index));
+
+    App.feed_wdt();
+    yield();
+
+    auto result = http_read_loop_result(read_bytes_or_error, last_data_time, timeout_ms);
+    if (result == HttpReadLoopResult::RETRY)
+      continue;
+    if (result == HttpReadLoopResult::ERROR)
+      return {HttpReadStatus::ERROR, read_bytes_or_error};
+    if (result == HttpReadLoopResult::TIMEOUT)
+      return {HttpReadStatus::TIMEOUT, 0};
+
+    read_index += read_bytes_or_error;
+  }
+  return {HttpReadStatus::OK, 0};
+}
+
 class HttpRequestResponseTrigger : public Trigger<std::shared_ptr<HttpContainer>, std::string &> {
  public:
   void process(const std::shared_ptr<HttpContainer> &container, std::string &response_body) {
@@ -124,6 +199,7 @@ class HttpRequestComponent : public Component {
 
   void set_useragent(const char *useragent) { this->useragent_ = useragent; }
   void set_timeout(uint32_t timeout) { this->timeout_ = timeout; }
+  uint32_t get_timeout() const { return this->timeout_; }
   void set_watchdog_timeout(uint32_t watchdog_timeout) { this->watchdog_timeout_ = watchdog_timeout; }
   uint32_t get_watchdog_timeout() const { return this->watchdog_timeout_; }
   void set_follow_redirects(bool follow_redirects) { this->follow_redirects_ = follow_redirects; }

esphome/components/http_request/http_request_idf.cpp

@@ -100,6 +100,7 @@ std::shared_ptr<HttpContainer> HttpRequestIDF::perform(const std::string &url, c
 
   config.buffer_size = this->buffer_size_rx_;
   config.buffer_size_tx = this->buffer_size_tx_;
+  config.is_async = true;  // Enable non-blocking mode
 
   const uint32_t start = millis();
   watchdog::WatchdogManager wdm(this->get_watchdog_timeout());
@@ -213,15 +214,36 @@ int HttpContainerIDF::read(uint8_t *buf, size_t max_len) {
   const uint32_t start = millis();
   watchdog::WatchdogManager wdm(this->parent_->get_watchdog_timeout());
 
-  this->feed_wdt();
-  int read_len = esp_http_client_read(this->client_, (char *) buf, max_len);
-  this->feed_wdt();
-  if (read_len > 0) {
-    this->bytes_read_ += read_len;
+  // Check if we've already read all expected content
+  if (this->bytes_read_ >= this->content_length) {
+    this->duration_ms += (millis() - start);
+    return 0;  // All content read
   }
+
+  this->feed_wdt();
+  int read_len_or_error = esp_http_client_read(this->client_, (char *) buf, max_len);
+  this->feed_wdt();
+
   this->duration_ms += (millis() - start);
 
-  return read_len;
+  if (read_len_or_error > 0) {
+    this->bytes_read_ += read_len_or_error;
+    return read_len_or_error;
+  }
+
+  if (read_len_or_error == 0) {
+    // Connection closed gracefully
+    return 0;
+  }
+
+  // read_len_or_error < 0: check for EAGAIN (no data available in non-blocking mode)
+  // ESP_ERR_HTTP_EAGAIN = 0x7007, returned as negative
+  if (read_len_or_error == -ESP_ERR_HTTP_EAGAIN) {
+    return 0;  // No data available yet, consistent with Arduino behavior
+  }
+
+  // Real error - return the actual error code for debugging
+  return read_len_or_error;
 }
 
 void HttpContainerIDF::end() {

esphome/components/http_request/ota/ota_http_request.cpp

@@ -115,39 +115,45 @@ uint8_t OtaHttpRequestComponent::do_ota_() {
     return error_code;
   }
 
+  uint32_t last_data_time = millis();
+  const uint32_t read_timeout = this->parent_->get_timeout();
+
   while (container->get_bytes_read() < container->content_length) {
-    // read a maximum of chunk_size bytes into buf. (real read size returned)
-    int bufsize = container->read(buf, OtaHttpRequestComponent::HTTP_RECV_BUFFER);
-    ESP_LOGVV(TAG, "bytes_read_ = %u, body_length_ = %u, bufsize = %i", container->get_bytes_read(),
-              container->content_length, bufsize);
+    // read a maximum of chunk_size bytes into buf. (real read size returned, or negative error code)
+    int bufsize_or_error = container->read(buf, OtaHttpRequestComponent::HTTP_RECV_BUFFER);
+    ESP_LOGVV(TAG, "bytes_read_ = %u, body_length_ = %u, bufsize_or_error = %i", container->get_bytes_read(),
+              container->content_length, bufsize_or_error);
 
     // feed watchdog and give other tasks a chance to run
     App.feed_wdt();
     yield();
 
-    // Exit loop if no data available (stream closed or end of data)
-    if (bufsize <= 0) {
-      if (bufsize < 0) {
-        ESP_LOGE(TAG, "Stream closed with error");
-        this->cleanup_(std::move(backend), container);
-        return OTA_CONNECTION_ERROR;
+    auto result = http_read_loop_result(bufsize_or_error, last_data_time, read_timeout);
+    if (result == HttpReadLoopResult::RETRY)
+      continue;
+    if (result != HttpReadLoopResult::DATA) {
+      if (result == HttpReadLoopResult::TIMEOUT) {
+        ESP_LOGE(TAG, "Timeout reading data");
+      } else {
+        ESP_LOGE(TAG, "Error reading data: %d", bufsize_or_error);
       }
-      // bufsize == 0: no more data available, exit loop
-      break;
+      this->cleanup_(std::move(backend), container);
+      return OTA_CONNECTION_ERROR;
     }
 
-    if (bufsize <= OtaHttpRequestComponent::HTTP_RECV_BUFFER) {
+    // At this point bufsize_or_error > 0, so it's a valid size
+    if (bufsize_or_error <= OtaHttpRequestComponent::HTTP_RECV_BUFFER) {
       // add read bytes to MD5
-      md5_receive.add(buf, bufsize);
+      md5_receive.add(buf, bufsize_or_error);
 
       // write bytes to OTA backend
       this->update_started_ = true;
-      error_code = backend->write(buf, bufsize);
+      error_code = backend->write(buf, bufsize_or_error);
       if (error_code != ota::OTA_RESPONSE_OK) {
         // error code explanation available at
         // https://github.com/esphome/esphome/blob/dev/esphome/components/ota/ota_backend.h
         ESP_LOGE(TAG, "Error code (%02X) writing binary data to flash at offset %d and size %d", error_code,
-                 container->get_bytes_read() - bufsize, container->content_length);
+                 container->get_bytes_read() - bufsize_or_error, container->content_length);
         this->cleanup_(std::move(backend), container);
         return error_code;
       }
@@ -244,19 +250,19 @@ bool OtaHttpRequestComponent::http_get_md5_() {
   }
 
   this->md5_expected_.resize(MD5_SIZE);
-  int read_len = 0;
-  while (container->get_bytes_read() < MD5_SIZE) {
-    read_len = container->read((uint8_t *) this->md5_expected_.data(), MD5_SIZE);
-    if (read_len <= 0) {
-      break;
-    }
-    App.feed_wdt();
-    yield();
-  }
+  auto result = http_read_fully(container.get(), (uint8_t *) this->md5_expected_.data(), MD5_SIZE, MD5_SIZE,
+                                this->parent_->get_timeout());
   container->end();
 
-  ESP_LOGV(TAG, "Read len: %u, MD5 expected: %u", read_len, MD5_SIZE);
-  return read_len == MD5_SIZE;
+  if (result.status != HttpReadStatus::OK) {
+    if (result.status == HttpReadStatus::TIMEOUT) {
+      ESP_LOGE(TAG, "Timeout reading MD5");
+    } else {
+      ESP_LOGE(TAG, "Error reading MD5: %d", result.error_code);
+    }
+    return false;
+  }
+  return true;
 }
 
 bool OtaHttpRequestComponent::validate_url_(const std::string &url) {

esphome/components/http_request/update/http_request_update.cpp

@@ -70,19 +70,16 @@ void HttpRequestUpdate::update_task(void *params) {
     UPDATE_RETURN;
   }
 
-  size_t read_index = 0;
-  while (container->get_bytes_read() < container->content_length) {
-    int read_bytes = container->read(data + read_index, MAX_READ_SIZE);
-
-    yield();
-
-    if (read_bytes <= 0) {
-      // Network error or connection closed - break to avoid infinite loop
-      break;
+  auto read_result = http_read_fully(container.get(), data, container->content_length, MAX_READ_SIZE,
+                                     this_update->request_parent_->get_timeout());
+  if (read_result.status != HttpReadStatus::OK) {
+    if (read_result.status == HttpReadStatus::TIMEOUT) {
+      ESP_LOGE(TAG, "Timeout reading manifest");
+    } else {
+      ESP_LOGE(TAG, "Error reading manifest: %d", read_result.error_code);
     }
-
-    read_index += read_bytes;
   }
+  size_t read_index = container->get_bytes_read();
 
   bool valid = false;
   {  // Ensures the response string falls out of scope and deallocates before the task ends