Skip to content

Commit 8fdbca3

Browse files
Default ConnectionConfig to no timeout and let ClientConfig ask for one
The neutral type should not carry an opinion about how long a connection is worth waiting on, since it cannot know what the connection is for. A client does know, so it is the one that names thirty seconds, and a server takes the absence of a deadline as it comes.
1 parent d944803 commit 8fdbca3

2 files changed

Lines changed: 14 additions & 7 deletions

File tree

‎polyweb.hpp‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -583,19 +583,19 @@ namespace pw {
583583
using TLSWSRoute = BasicWSRoute<pn::tcp::TLSConnection>;
584584

585585
struct ConnectionConfig {
586-
std::chrono::milliseconds send_timeout = std::chrono::seconds(30);
587-
std::chrono::milliseconds recv_timeout = std::chrono::seconds(30);
586+
// A connection may stay request and response, become a WebSocket, or become a long
587+
// poll, and only the first of those tolerates a deadline, so none is imposed here
588+
std::chrono::milliseconds send_timeout = {};
589+
std::chrono::milliseconds recv_timeout = {};
588590
bool tcp_keep_alive = true; // Reaps peers that vanished without closing, not HTTP keep-alive
589591
bool tcp_no_delay = true;
590592

591593
pn::Status apply(pn::tcp::Connection& conn) const;
592594
};
593595

594596
struct ServerConfig {
595-
// A server cannot know at accept time whether a connection will stay request and
596-
// response, become a WebSocket, or become a long poll, so it waits rather than
597-
// impose a deadline that is only right for one of them
598-
ConnectionConfig tcp = {.send_timeout = {}, .recv_timeout = {}};
597+
// A server cannot know at accept time which of those an accepted connection is
598+
ConnectionConfig tcp;
599599
size_t buf_capacity = 4'000;
600600
MessageConfig http;
601601
WSConfig ws;
@@ -663,7 +663,9 @@ namespace pw {
663663

664664
class ClientConfig {
665665
public:
666-
ConnectionConfig tcp;
666+
// A client makes one request and knows what it is waiting for, so unlike a server
667+
// it can say how long that is worth waiting
668+
ConnectionConfig tcp = {.send_timeout = std::chrono::seconds(30), .recv_timeout = std::chrono::seconds(30)};
667669

668670
// Borrowed rather than owned, so one context may serve any number of requests.
669671
// When it is null a default trust store, built once on first use, is used instead

‎tests/http.cpp‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -370,6 +370,11 @@ TEST(a_connection_config_applies_the_options_it_names) {
370370
TEST(a_server_imposes_no_timeout_of_its_own) {
371371
// A WebSocket idles between messages by design, and handle_conn cannot tell an idle
372372
// keep-alive connection from a client that sent nothing, so a server waits instead
373+
pw::ConnectionConfig config;
374+
CHECK(config.recv_timeout == std::chrono::milliseconds(0));
375+
CHECK(config.send_timeout == std::chrono::milliseconds(0));
376+
377+
// A server takes that as it comes, rather than overriding it the way a client does
373378
pw::ServerConfig server_config;
374379
CHECK(server_config.tcp.recv_timeout == std::chrono::milliseconds(0));
375380
CHECK(server_config.tcp.send_timeout == std::chrono::milliseconds(0));

0 commit comments

Comments
 (0)