Skip to content

Fix client rx timeout #226

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Fix client rx timeout #226

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
1dbce42
Fix client rx timeout
TienHuyIoT Jul 12, 2025
a874dfb
ci(pre-commit): Apply automatic fixes
pre-commit-ci-lite[bot] Jul 12, 2025
db7c354
Use CONFIG_TCP_MSS as the default stream buffer size for ESP32
TienHuyIoT Jul 13, 2025
8e83931
Merge branch 'main' of https://github.com/TienHuyIoT/ESP32Async-ESPAs…
TienHuyIoT Jul 13, 2025
43e62c0
ci(pre-commit): Apply automatic fixes
pre-commit-ci-lite[bot] Jul 13, 2025
a2b8307
Include sdkconfig.h only for ESP32
TienHuyIoT Jul 14, 2025
b392f0c
Merge branch 'main' into main
mathieucarbou Jul 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion src/ESPAsyncWebServer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include <vector>

#if defined(ESP32) || defined(LIBRETINY)
#include "sdkconfig.h"
#include <AsyncTCP.h>
#elif defined(ESP8266)
#include <ESPAsyncTCP.h>
Expand Down Expand Up @@ -44,6 +45,10 @@
#define ASYNCWEBSERVER_USE_CHUNK_INFLIGHT 1
#endif

#ifndef ASYNCWEBSERVER_RX_TIMEOUT
#define ASYNCWEBSERVER_RX_TIMEOUT 3 // Seconds for timeout
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be set to 0 (the default in AsyncTCP) for backward compatiblity right ?

Ideally, AsyncTCP should even expose a constant macro that gets reused here

Copy link
Author

@TienHuyIoT TienHuyIoT Jul 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t think so.
The Async WebServer is typically used to continuously listen for new client connections. In the case where a client is connected but no data is exchanged, the server should close the connection after a timeout occurs.

Except connection ws and sse.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, but there are some use cases such as long-lived connections or long polling where a normal request would be more like a sse / ws request.

my second point is that it introduces a change compared to the current defaults. If some users rely on the current default, then this change can break their app.

Also, another place where this timeout should be set to zero I think is when we decide to pause a request (there is a pause method in the code).

So more generally, I think it could be better to just leave it to its current defaults and then issue a 3.8.0 release with this newly added feature , and we document in the wiki and release notes that there is a new extra timeout methods people can set.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@me-no-dev : any opinion on that ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, but there are some use cases such as long-lived connections or long polling where a normal request would be more like a sse / ws request.

In some cases, during file downloads or uploads, long connections may be lost or broken due to network issues. If timeout handling is disabled, the server may keep these connections indefinitely, leading to a memory leak.

my second point is that it introduces a change compared to the current defaults. If some users rely on the current default, then this change can break their app.

This update does not change the design of the implemented Async WebServer; it only fixes incorrect RX timeout handling.

Also, another place where this timeout should be set to zero I think is when we decide to pause a request (there is a pause method in the code).

I understand that point, so the new update has implemented a mechanism to back up and restore the RX timeout.

So more generally, I think it could be better to just leave it to its current defaults and then issue a 3.8.0 release with this newly added feature , and we document in the wiki and release notes that there is a new extra timeout methods people can set.

The timeout method is not new—it is already implemented and based on AsyncTCP. The Async WebServer simply registers the onTimeout() callback and then decides whether to close the connection on its own.
However, 👍 documentation will be useful for maintenance and future improvements to the fantastic Async WebServer library.

Thanks so much for your comments!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK!
If @me-no-dev is happy with your proposal, then we can merge.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need a bit of time to check everything out

#endif

class AsyncWebServer;
class AsyncWebServerRequest;
class AsyncWebServerResponse;
Expand Down Expand Up @@ -87,8 +92,13 @@ class FileOpenMode {
#endif

// if this value is returned when asked for data, packet will not be sent and you will be asked for data again
#define RESPONSE_TRY_AGAIN 0xFFFFFFFF
#define RESPONSE_TRY_AGAIN 0xFFFFFFFF

#ifndef CONFIG_TCP_MSS
#define RESPONSE_STREAM_BUFFER_SIZE 1460
#else
#define RESPONSE_STREAM_BUFFER_SIZE CONFIG_TCP_MSS
#endif

typedef uint8_t WebRequestMethodComposite;
typedef std::function<void(void)> ArDisconnectHandler;
Expand Down Expand Up @@ -250,6 +260,7 @@ class AsyncWebServerRequest {
String _itemValue;
uint8_t *_itemBuffer;
size_t _itemBufferIndex;
uint32_t _rx_timeout;
bool _itemIsFile;

void _onPoll();
Expand Down
6 changes: 4 additions & 2 deletions src/WebRequest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ AsyncWebServerRequest::AsyncWebServerRequest(AsyncWebServer *s, AsyncClient *c)
: _client(c), _server(s), _handler(NULL), _response(NULL), _onDisconnectfn(NULL), _temp(), _parseState(PARSE_REQ_START), _version(0), _method(HTTP_ANY),
_url(), _host(), _contentType(), _boundary(), _authorization(), _reqconntype(RCT_HTTP), _authMethod(AsyncAuthType::AUTH_NONE), _isMultipart(false),
_isPlainPost(false), _expectingContinue(false), _contentLength(0), _parsedLength(0), _multiParseState(0), _boundaryPosition(0), _itemStartIndex(0),
_itemSize(0), _itemName(), _itemFilename(), _itemType(), _itemValue(), _itemBuffer(0), _itemBufferIndex(0), _itemIsFile(false), _tempObject(NULL) {
_itemSize(0), _itemName(), _itemFilename(), _itemType(), _itemValue(), _itemBuffer(0), _itemBufferIndex(0), _itemIsFile(false), _tempObject(NULL),
_rx_timeout(ASYNCWEBSERVER_RX_TIMEOUT) {
c->onError(
[](void *r, AsyncClient *c, int8_t error) {
(void)c;
Expand Down Expand Up @@ -720,7 +721,6 @@ void AsyncWebServerRequest::_send() {
}

// here, we either have a response give nfrom user or one of the two above
_client->setRxTimeout(0);
_response->_respond(this);
_sent = true;
}
Expand All @@ -730,6 +730,7 @@ AsyncWebServerRequestPtr AsyncWebServerRequest::pause() {
if (_paused) {
return _this;
}
_rx_timeout = client()->getRxTimeout(); // backup client rx timeout
client()->setRxTimeout(0);
// this shared ptr will hold the request pointer until it gets destroyed following a disconnect.
// this is just used as a holder providing weak observers, so the deleter is a no-op.
Expand Down Expand Up @@ -937,6 +938,7 @@ void AsyncWebServerRequest::send(AsyncWebServerResponse *response) {
// if request was paused, we need to send the response now
if (_paused) {
_paused = false;
_client->setRxTimeout(_rx_timeout); // restore rx timeout
_send();
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/WebServer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ AsyncWebServer::AsyncWebServer(uint16_t port) : _server(port) {
if (c == NULL) {
return;
}
c->setRxTimeout(3);
c->setRxTimeout(ASYNCWEBSERVER_RX_TIMEOUT);
AsyncWebServerRequest *r = new AsyncWebServerRequest((AsyncWebServer *)s, c);
if (r == NULL) {
c->abort();
Expand Down
Loading