esp_http_server: Handling stale websocket connections (IDFGH-17036)

[nebkat]

Answers checklist.

• I have read the documentation ESP-IDF Programming Guide and the issue is not addressed there.
• I have updated my IDF branch (master or release) to the latest version and checked that the issue is present there.
• I have searched the issue tracker for a similar issue and not found a similar issue.

General issue report

The HTTP server does not currently do a good job handling "stale" WebSocket connections where the peer has gone offline but not closed the TCP socket. We have had problems with this in our application where we send messages from the ESP32 to a browser via web sockets.

We queue up 10 messages every second to the HTTP task to be sent using httpd_ws_send_frame_async, however once the peer disappears the send operations start timing out (by default after 5 seconds).

if (httpd_ws_send_frame_async(http_handle, fd, frame) != ESP_OK) {
    httpd_sess_trigger_close(this->handle.get(), fd);
}

The httpd_sess_trigger_close internally does a httpd_queue_work(handle, httpd_sess_close, session), so that operation is placed at the end of the queue, after our now 50(!) queued up send operations, so we just get log spam forever with:

W (1764610736790) httpd_ws: httpd_ws_send_frame_async: Failed to send WS header
W (1764610741789) httpd_txrx: httpd_sock_err: error in send : 11
W (1764610741790) httpd_ws: httpd_ws_send_frame_async: Failed to send WS header
W (1764610746789) httpd_txrx: httpd_sock_err: error in send : 11
....

---

The simple solution we are using for now has been to add a httpd_sess_trigger_close_async function which calls httpd_sess_close directly.

But this has me wondering whether there is any reason why the server shouldn't automatically end the sessions when it gets socket send (and possibly receive) errors?

---

Side-note: the terminology used in the httpd functions is rather confusing:

• httpd_ws_send_frame() is just a combination of httpd_req_to_sockfd() and httpd_ws_send_frame_async()
• httpd_ws_send_frame_async() performs a synchronous send and must be performed on the worker task (it is just "async" because it doesn't require a HTTP request to obtain the FD)
• httpd_ws_send_data() can be performed outside of the worker task, and blocks until the send is complete
• httpd_ws_send_data_async() can be performed outside of the worker task but does not block
• httpd_sess_trigger_close() can be performed outside of the worker task

IMO these should become:

• httpd_ws_resp_frame() (httpd_ws_send_frame())
• httpd_ws_send_frame() (httpd_ws_send_frame_async())
• httpd_queue_ws_send_frame_blocking() (httpd_ws_send_data())
• httpd_queue_ws_send_frame_cb() (httpd_ws_send_data_async())
• httpd_queue_sess_close() (httpd_sess_trigger_close())
• httpd_sess_close() (NEW httpd_sess_trigger_close_async())

[Ashish285]

Hi @nebkat , thanks for creating this issue.

why the server shouldn't automatically end the sessions when it gets socket send (and possibly receive) errors?

I believe this is not added to the server but left on the application to decide what to do on failures. Maybe the application wants to retry send after a small delay or keep the failed data to be sent on next connection. To keep these possibilities open, the application can decide on how to handle the failure.

The simple solution we are using for now has been to add a httpd_sess_trigger_close_async function which calls httpd_sess_close directly.

I agree with this, we should include in an API to directly close the connection with the downside that any existing queued data will be lost. Feel free to raise a PR and we will accept it or we will add it in the next release.

Side-note: the terminology used in the httpd functions is rather confusing

I understand that the terminologies might be slightly outdated and confusing but it will be a big change to rename the APIs as it will break existing applications. Maybe we can introduce some alias for these functions.

Feel free to ask if you have any more questions.

[nebkat]

I believe this is not added to the server but left on the application to decide what to do on failures. Maybe the application wants to retry send after a small delay or keep the failed data to be sent on next connection. To keep these possibilities open, the application can decide on how to handle the failure.

My thinking here was that since most of our socket sends are broken up into multiple parts (e.g. we do a separate send_fn(...) for WS header and payload) it is almost certainly going to result in corruption at the protocol level. Or at least we would need a way to know whether the very first send failed.

I agree with this, we should include in an API to directly close the connection with the downside that any existing queued data will be lost. Feel free to raise a PR and we will accept it or we will add it in the next release.

Will do.

I understand that the terminologies might be slightly outdated and confusing but it will be a big change to rename the APIs as it will break existing applications. Maybe we can introduce some alias for these functions.

Maybe too late but could be a chance to do it now before 6.0? Old ones could be aliased + deprecated. To avoid conflict it would need to be:

• httpd_ws_resp() (httpd_ws_send_frame())
• httpd_ws_send() (httpd_ws_send_frame_async())
• httpd_queue_ws_send_blocking() (httpd_ws_send_data())
• httpd_queue_ws_send_cb() (httpd_ws_send_data_async())
• httpd_queue_sess_close() (httpd_sess_trigger_close())
• httpd_sess_close() (NEW httpd_sess_trigger_close_async())

[Ashish285]

Hi @nebkat ,

Maybe too late but could be a chance to do it now before 6.0?

I believe it will not be possible to do this in v6.0 release as it is quite late at this point. We will keep this suggestion and try to include it in the next release where we can include these breaking changes.

Feel free to raise the PR, we will be happy to accept it and include in your contributions. Thanks!