From 7115881a9768d9d305d10d644715a0cc56169c48 Mon Sep 17 00:00:00 2001 From: Laukik Hase Date: Mon, 7 Mar 2022 11:05:43 +0530 Subject: [PATCH] esp_http_client/esp_https_ota: Removed errno checks - Returned -ESP_ERR_HTTP_EAGAIN for timeout errors from esp_http_client whenever tcp transport layer returns connection timeout - Removed redundant conditional statements as required Co-authored-by: Shubham Kulkarni --- components/esp_http_client/esp_http_client.c | 35 ++++++++++++------- .../esp_http_client/include/esp_http_client.h | 3 ++ components/esp_https_ota/src/esp_https_ota.c | 28 ++++++--------- 3 files changed, 37 insertions(+), 29 deletions(-) diff --git a/components/esp_http_client/esp_http_client.c b/components/esp_http_client/esp_http_client.c index 7df3e22615..cfb8c49908 100644 --- a/components/esp_http_client/esp_http_client.c +++ b/components/esp_http_client/esp_http_client.c @@ -1063,9 +1063,8 @@ int esp_http_client_read(esp_http_client_handle_t client, char *buffer, int len) if (rlen <= 0) { if (errno != 0) { esp_log_level_t sev = ESP_LOG_WARN; - /* On connection close from server, recv should ideally return 0 but we have error conversion - * in `tcp_transport` SSL layer which translates it `-1` and hence below additional checks */ - if (rlen == -1 && errno == ENOTCONN && client->response->is_chunked) { + /* Check for cleanly closed connection */ + if (rlen == ERR_TCP_TRANSPORT_CONNECTION_CLOSED_BY_FIN && client->response->is_chunked) { /* Explicit call to parser for invoking `message_complete` callback */ http_parser_execute(client->parser, client->parser_settings, res_buffer->data, 0); /* ...and lowering the message severity, as closed connection from server side is expected in chunked transport */ @@ -1073,14 +1072,21 @@ int esp_http_client_read(esp_http_client_handle_t client, char *buffer, int len) } ESP_LOG_LEVEL(sev, TAG, "esp_transport_read returned:%d and errno:%d ", rlen, errno); } -#ifdef CONFIG_ESP_HTTP_CLIENT_ENABLE_HTTPS - if (rlen == ESP_TLS_ERR_SSL_WANT_READ || errno == EAGAIN) { -#else - if (errno == EAGAIN) { -#endif - ESP_LOGD(TAG, "Received EAGAIN! rlen = %d, errno %d", rlen, errno); - return ridx; + + if (rlen == ERR_TCP_TRANSPORT_CONNECTION_TIMEOUT) { + ESP_LOGD(TAG, "Connection timed out before data was ready!"); + /* Returning the number of bytes read upto the point where connection timed out */ + if (ridx) { + return ridx; + } + return -ESP_ERR_HTTP_EAGAIN; } + + if (rlen != ERR_TCP_TRANSPORT_CONNECTION_CLOSED_BY_FIN) { + esp_err_t err = esp_transport_translate_error(rlen); + ESP_LOGE(TAG, "transport_read: error - %d | %s", err, esp_err_to_name(err)); + } + if (rlen < 0 && ridx == 0 && !esp_http_client_is_complete_data_received(client)) { http_dispatch_event(client, HTTP_EVENT_ERROR, esp_transport_get_error_handle(client->transport), 0); return ESP_FAIL; @@ -1142,8 +1148,9 @@ esp_err_t esp_http_client_perform(esp_http_client_handle_t client) /* Disable caching response body, as data should * be handled by application event handler */ client->cache_data_in_fetch_hdr = 0; - if (esp_http_client_fetch_headers(client) < 0) { - if (client->is_async && errno == EAGAIN) { + int64_t ret = esp_http_client_fetch_headers(client); + if (ret < 0) { + if ((client->is_async && errno == EAGAIN) || ret == -ESP_ERR_HTTP_EAGAIN) { return ESP_ERR_HTTP_EAGAIN; } /* Enable caching after error condition because next @@ -1219,6 +1226,10 @@ int64_t esp_http_client_fetch_headers(esp_http_client_handle_t client) while (client->state < HTTP_STATE_RES_COMPLETE_HEADER) { buffer->len = esp_transport_read(client->transport, buffer->data, client->buffer_size_rx, client->timeout_ms); if (buffer->len <= 0) { + if (buffer->len == ERR_TCP_TRANSPORT_CONNECTION_TIMEOUT) { + ESP_LOGW(TAG, "Connection timed out before data was ready!"); + return -ESP_ERR_HTTP_EAGAIN; + } return ESP_FAIL; } http_parser_execute(client->parser, client->parser_settings, buffer->data, buffer->len); diff --git a/components/esp_http_client/include/esp_http_client.h b/components/esp_http_client/include/esp_http_client.h index 3d54d2b13e..6377988782 100644 --- a/components/esp_http_client/include/esp_http_client.h +++ b/components/esp_http_client/include/esp_http_client.h @@ -427,6 +427,7 @@ int esp_http_client_write(esp_http_client_handle_t client, const char *buffer, i * @return * - (0) if stream doesn't contain content-length header, or chunked encoding (checked by `esp_http_client_is_chunked` response) * - (-1: ESP_FAIL) if any errors + * - (-ESP_ERR_HTTP_EAGAIN = -0x7007) if call is timed-out before any data was ready * - Download data length defined by content-length header */ int64_t esp_http_client_fetch_headers(esp_http_client_handle_t client); @@ -451,6 +452,8 @@ bool esp_http_client_is_chunked_response(esp_http_client_handle_t client); * @return * - (-1) if any errors * - Length of data was read + * + * @note (-ESP_ERR_HTTP_EAGAIN = -0x7007) is returned when call is timed-out before any data was ready */ int esp_http_client_read(esp_http_client_handle_t client, char *buffer, int len); diff --git a/components/esp_https_ota/src/esp_https_ota.c b/components/esp_https_ota/src/esp_https_ota.c index 431a8628d3..76dc53cace 100644 --- a/components/esp_https_ota/src/esp_https_ota.c +++ b/components/esp_https_ota/src/esp_https_ota.c @@ -361,11 +361,11 @@ static esp_err_t read_header(esp_https_ota_t *handle) data_read = esp_http_client_read(handle->http_client, (handle->ota_upgrade_buf + bytes_read), data_read_size); - /* - * As esp_http_client_read doesn't return negative error code if select fails, we rely on - * `errno` to check for underlying transport connectivity closure if any - */ - if (errno == ENOTCONN || errno == ECONNRESET || errno == ECONNABORTED || data_read < 0) { + if (data_read < 0) { + if (data_read == -ESP_ERR_HTTP_EAGAIN) { + ESP_LOGD(TAG, "ESP_ERR_HTTP_EAGAIN invoked: Call timed out before data was ready"); + continue; + } ESP_LOGE(TAG, "Connection closed, errno = %d", errno); break; } @@ -491,19 +491,9 @@ esp_err_t esp_https_ota_perform(esp_https_ota_handle_t https_ota_handle) * esp_http_client_is_complete_data_received is added to check whether * complete image is received. */ - bool is_recv_complete = esp_http_client_is_complete_data_received(handle->http_client); - /* - * As esp_http_client_read doesn't return negative error code if select fails, we rely on - * `errno` to check for underlying transport connectivity closure if any. - * Incase the complete data has not been received but the server has sent - * an ENOTCONN or ECONNRESET, failure is returned. We close with success - * if complete data has been received. - */ - if ((errno == ENOTCONN || errno == ECONNRESET || errno == ECONNABORTED) && !is_recv_complete) { - ESP_LOGE(TAG, "Connection closed, errno = %d", errno); + if (!esp_http_client_is_complete_data_received(handle->http_client)) { + ESP_LOGE(TAG, "Connection closed before complete data was received!"); return ESP_FAIL; - } else if (!is_recv_complete) { - return ESP_ERR_HTTPS_OTA_IN_PROGRESS; } ESP_LOGD(TAG, "Connection closed"); } else if (data_read > 0) { @@ -523,6 +513,10 @@ esp_err_t esp_https_ota_perform(esp_https_ota_handle_t https_ota_handle) #endif // CONFIG_ESP_HTTPS_OTA_DECRYPT_CB return _ota_write(handle, data_buf, data_len); } else { + if (data_read == -ESP_ERR_HTTP_EAGAIN) { + ESP_LOGD(TAG, "ESP_ERR_HTTP_EAGAIN invoked: Call timed out before data was ready"); + return ESP_ERR_HTTPS_OTA_IN_PROGRESS; + } ESP_LOGE(TAG, "data read %d, errno %d", data_read, errno); return ESP_FAIL; }