esphome / issues

Issue Tracker for ESPHome
https://esphome.io/
290 stars 36 forks source link

http_request now produce an unspecified error and exit container instead of reporting the client code error #6010

Closed vargr-cg closed 1 week ago

vargr-cg commented 4 months ago

The problem

Older versions of http request were reporting client error codes when connections was failing such as " -1" from the on_response trigger

This could be used for determining if a device was up or not and publish state for a sensor or a switch for example. It now fails with an unspecified error wich is cleared later.

The container now exits and no return code or other data are provided.

Which version of ESPHome has the issue?

2024.6.6

What type of installation are you using?

pip

Which version of Home Assistant has the issue?

2024.7.0

What platform are you using?

ESP8266

Board

esp01_1m

Component causing the issue

http_request

Example YAML snippet

esphome:
  name: ir_0
esp8266:
  board: esp01_1m
  framework:
    version: 3.0.2 
  restore_from_flash: true

http_request:
  id: http_request_data
  esp8266_disable_ssl_support: true
  #timeout: 0.5s

### ... sensor,  switches, etc 

script:
####### get fbr state ############
    - id: get_fbr_state
      mode: single
      then:
        - http_request.get:
             url: "http://192.168.0.253:54243/device.xml"
             capture_response: true
             on_response:
               then:
                 - lambda: |-
                    if (response->status_code == 200) {
                      id(fbx_oo).publish_state(true);
                    } else {
                      id(fbx_oo).publish_state(false);
                    };

# or 
    - id: get_lgtv_state
      mode: single
      then:
        - http_request.get:
            url: "http://192.168.0.251/binary_sensor/tv_state"
            #capture_response: true
            on_response:
               then:
                - logger.log:
                    format: "LGTV request Status code %d"
                    args:
                      - response->status_code
                - lambda: |-
                    if (response->status_code == 200) {
                      id(lg_oo).publish_state(true);
                    } else if (response->status_code == -1 ) {
                      id(lg_oo).publish_state(false);
                    };

Anything in the logs that might be useful for us?

[17:14:20][W][http_request.arduino:104]: HTTP Request failed; URL: http://192.168.0.253:54243/device.xml; Error: connection failed
[17:14:20][E][component:164]: Component http_request set Error flag: unspecified

[17:14:21][E][component:176]: Component http_request cleared Error flag

Additional information

No response

rwdnnz commented 4 months ago

I'm facing a similar issue with the later releases of ESPHome 2024.6.x where a 404 from my API is being treated as a failed request.

- http_request.get:
          capture_response: true
          headers:
            content-Type: application/json
          url: !lambda |-
            return ((std::string) "${music_control_api}/lookup?key=" + x);
          on_response:
            - if:
                condition:
                  lambda: "return response->status_code == 200;"
                then:
                  - lambda: |-
                      id(tag_found_in_db).publish_state(true);
                      json::parse_json(body, [](JsonObject root) -> bool {
                      id(artist).publish_state(root["artist"]);
                      id(album).publish_state(root["album"]);
                      return true;
                      });
                  - output.turn_on: play_button_led
                else:
                    - output.turn_on: play_button_led
                    - delay: 0.5s
                    - output.turn_off: play_button_led
                    - delay: 0.5s

Which results in a failed request when a 404 is returned and we never get into the else statement now.

[06:44:17][E][http_request.arduino:112]: HTTP Request failed; URL: http://music-control-api.example.com/lookup?key=53-53-FF-99-BA-00-01; Code: 404
[06:44:17][E][component:164]: Component http_request set Error flag: unspecified
...
[06:44:18][E][component:176]: Component http_request cleared Error flag
vargr-cg commented 4 months ago

It probably come of this I think

https://github.com/esphome/esphome/blame/4c6a17e304002b741388335db30c06725a6730c6/esphome/components/http_request/http_request_arduino.cpp#L102-L116

  container->status_code = container->client_.sendRequest(method.c_str(), body.c_str());
  if (container->status_code < 0) {
    ESP_LOGW(TAG, "HTTP Request failed; URL: %s; Error: %s", url.c_str(),
             HTTPClient::errorToString(container->status_code).c_str());
    this->status_momentary_error("failed", 1000);
    container->end();
    return nullptr;
  }

  if (container->status_code < 200 || container->status_code >= 300) {
    ESP_LOGE(TAG, "HTTP Request failed; URL: %s; Code: %d", url.c_str(), container->status_code);
    this->status_momentary_error("failed", 1000);
    container->end();
    return nullptr;
  }

It seems to end container if status code is different of 200

or this could have a similar effect but i'm not sure my code is using it.: https://github.com/esphome/esphome/blob/4c6a17e304002b741388335db30c06725a6730c6/esphome/components/http_request/http_request_idf.cpp#L104-L119


  if (err != ESP_OK) {
    this->status_momentary_error("failed", 1000);
    ESP_LOGE(TAG, "HTTP Request failed: %s", esp_err_to_name(err));
    esp_http_client_cleanup(client);
    return nullptr;
  }

  container->content_length = esp_http_client_fetch_headers(client);
  const auto status_code = esp_http_client_get_status_code(client);
  container->status_code = status_code;

  if (status_code < 200 || status_code >= 300) {
    ESP_LOGE(TAG, "HTTP Request failed; URL: %s; Code: %d", url.c_str(), status_code);
    this->status_momentary_error("failed", 1000);
    esp_http_client_cleanup(client);
    return nullptr;
djiesr commented 3 months ago

Do you found any solution for that? I have the same issue.

rwdnnz commented 3 months ago

Right now, I think I'll just update my API to return a 200 and a status in the json response that I can take action on. I haven't found a solution other than that or waiting for this to get fixed at some stage.

djiesr commented 3 months ago

Ok, Thanks

vargr-cg commented 3 months ago

On my side i'm Moving back my sensors to home assistant. I hope to try some fix layer.Le 26 juil. 2024 10:10, rwdnnz @.***> a écrit : Right now, I think I'll just update my API to return a 200 and a status in the json response that I can take action on. I haven't found a solution other than that or waiting for this to get fixed at some stage.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you authored the thread.Message ID: @.***>

vargr-cg commented 3 months ago

Later ... Grmbl f...ckin french othographic corrector Le 26 juil. 2024 19:43, @. a écrit :On my side i'm Moving back my sensors to home assistant. I hope to try some fix layer.Le 26 juil. 2024 10:10, rwdnnz @.> a écrit : Right now, I think I'll just update my API to return a 200 and a status in the json response that I can take action on. I haven't found a solution other than that or waiting for this to get fixed at some stage.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you authored the thread.Message ID: @.***>

semaz commented 2 months ago

Any progress here? I encountered the same problem.

flyzet-prog commented 1 week ago

the same problem

clydebarrow commented 1 week ago

Add this to your yaml and test:

external_components:
  - source: github://pr#7689
    components: [http_request]

Report back here on the result.

flyzet-prog commented 1 week ago

Add this to your yaml and test:

external_components:
  - source: github://pr#7689
    components: [http_request]

Report back here on the result.

I tried your suggestion, unfortunately the same results. Basically i am trying to post request SOC of my battery to external database Google Firebase. I sent some random data manually to my Firebase database with Postman, no problem. But as soon as i tried with esphome http_request, i got an error. In attachments my esphome yaml and log. Maybe i am doing something wrongly.

Screenshot 2024-10-28 at 22 05 46 Screenshot 2024-10-28 at 21 57 03
clydebarrow commented 1 week ago

I tried your suggestion, unfortunately the same results.

That's a completely different problem - you have a DNS lookup failure.

pmalecka commented 1 week ago

I've got a different case, the device I'm sending http requests to is a microcontroller that sleeps a lot (Shelly TRV). It sometimes fails refuses connections. Here's what that looks like (with this PR applied):


[V][esp-idf:000]: E (28007) TRANSPORT_BASE: Failed to open a new connection: 32774

[V][esp-idf:000]: E (28009) HTTP_CLIENT: Connection failed, sock < 0

[E][component:164]: Component http_request set Error flag: unspecified
[E][http_request.idf:094]: HTTP Request failed: ESP_ERR_HTTP_CONNECT
[E][component:176]: Component http_request cleared Error flag

I'd like to be able to identify such cases, so that I can re-issue the request sooner.

It could make sense to have an on_error case or something similar in the yaml config for http_request.

vargr-cg commented 1 week ago

For some device aconnection error is a status. May be just reporting the negative status code instead of creating an on_error trigger could be sufficient. My initial demand was about this code in which a connection error is a status.

              - lambda: |-
                    if (response->status_code == 200) {
                      id(lg_oo).publish_state(true);
                    } else if (response->status_code == -1 ) {
                      id(lg_oo).publish_state(false);
                    };

Modifiying this for example as done for other cases and returning status code with a null body could be simpler I think.

  container->status_code = container->client_.sendRequest(method.c_str(), body.c_str());
  if (container->status_code < 0) {
    ESP_LOGW(TAG, "HTTP Request failed; URL: %s; Error: %s", url.c_str(),
             HTTPClient::errorToString(container->status_code).c_str());
    this->status_momentary_error("failed", 1000);
    container->end();
    return nullptr;

  }
clydebarrow commented 1 week ago

The status code is what the server returns, if the connection never succeeds there is no response and no status code. Separating the two events makes it clear that one is providing what the server sent, the other is that the server was uncontactable.