Skip to content

Commit 645ff30

Browse files
GaspardKiriraGaspardKirira
committed
fix(websocket): improve config and disconnect handling
1 parent fb36597 commit 645ff30

5 files changed

Lines changed: 201 additions & 23 deletions

File tree

‎CMakeLists.txt‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -309,9 +309,18 @@ else()
309309
)
310310
endif()
311311

312+
# Tests (optional)
313+
option(VIX_WEBSOCKET_BUILD_TESTS "Build websocket module tests" OFF)
314+
315+
if (VIX_WEBSOCKET_BUILD_TESTS)
316+
include(CTest)
317+
enable_testing()
318+
add_subdirectory(tests)
319+
endif()
320+
312321
# Examples (optional)
313322
option(VIX_WEBSOCKET_BUILD_EXAMPLES
314-
"Build WebSocket examples (server + HTML chat demo)" ON)
323+
"Build WebSocket examples (server + HTML chat demo)" OFF)
315324

316325
if (VIX_WEBSOCKET_BUILD_EXAMPLES)
317326
add_subdirectory(examples)

‎src/session.cpp‎

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include <vector>
3131

3232
#include <vix/async/core/spawn.hpp>
33+
#include <vix/utils/NetworkError.hpp>
3334

3435
namespace vix::websocket
3536
{
@@ -643,24 +644,10 @@ namespace vix::websocket
643644

644645
trigger_write_flush();
645646
}
647+
646648
void Session::emit_error(const std::string &message)
647649
{
648-
std::string lower = message;
649-
std::transform(
650-
lower.begin(),
651-
lower.end(),
652-
lower.begin(),
653-
[](unsigned char c)
654-
{
655-
return static_cast<char>(std::tolower(c));
656-
});
657-
658-
if (lower.find("end of file") != std::string::npos ||
659-
lower.find("eof") != std::string::npos ||
660-
lower.find("broken pipe") != std::string::npos ||
661-
lower.find("connection reset") != std::string::npos ||
662-
lower.find("canceled") != std::string::npos ||
663-
lower.find("cancelled") != std::string::npos)
650+
if (vix::utils::is_normal_network_disconnect_message(message))
664651
{
665652
log().log(
666653
Logger::Level::Debug,

‎src/websocket.cpp‎

Lines changed: 62 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,34 @@ namespace vix::websocket
5656
log.setFormatFromEnv("VIX_LOG_FORMAT");
5757
});
5858
}
59+
60+
std::string get_config_string_fallback(
61+
vix::config::Config &config,
62+
const std::string &primary,
63+
const std::string &fallback,
64+
const std::string &defaultValue)
65+
{
66+
const std::string value = config.getString(primary, "");
67+
68+
if (!value.empty())
69+
return value;
70+
71+
return config.getString(fallback, defaultValue);
72+
}
73+
74+
int get_config_int_fallback(
75+
vix::config::Config &config,
76+
const std::string &primary,
77+
const std::string &fallback,
78+
int defaultValue)
79+
{
80+
const int primaryValue = config.getInt(primary, -1);
81+
82+
if (primaryValue >= 0)
83+
return primaryValue;
84+
85+
return config.getInt(fallback, defaultValue);
86+
}
5987
} // namespace
6088

6189
LowLevelServer::LowLevelServer(
@@ -81,7 +109,12 @@ namespace vix::websocket
81109
"vix::websocket::LowLevelServer requires a valid runtime executor");
82110
}
83111

84-
const int port = coreConfig_.getInt("websocket.port", 9090);
112+
const int port =
113+
get_config_int_fallback(
114+
coreConfig_,
115+
"websocket.port",
116+
"websocket_port",
117+
9090);
85118
if ((port != 0 && port < 1024) || port > 65535)
86119
{
87120
logger().log(
@@ -111,8 +144,20 @@ namespace vix::websocket
111144
vix::async::net::tcp_endpoint LowLevelServer::make_bind_endpoint() const
112145
{
113146
vix::async::net::tcp_endpoint ep{};
114-
ep.host = coreConfig_.getString("websocket.host", "0.0.0.0");
115-
ep.port = static_cast<std::uint16_t>(coreConfig_.getInt("websocket.port", 9090));
147+
ep.host =
148+
get_config_string_fallback(
149+
const_cast<vix::config::Config &>(coreConfig_),
150+
"websocket.host",
151+
"websocket_host",
152+
"0.0.0.0");
153+
154+
ep.port =
155+
static_cast<std::uint16_t>(
156+
get_config_int_fallback(
157+
const_cast<vix::config::Config &>(coreConfig_),
158+
"websocket.port",
159+
"websocket_port",
160+
9090));
116161
return ep;
117162
}
118163

@@ -127,7 +172,13 @@ namespace vix::websocket
127172
try
128173
{
129174
vix::async::net::tcp_endpoint endpoint{};
130-
endpoint.host = coreConfig_.getString("websocket.host", "0.0.0.0");
175+
endpoint.host =
176+
get_config_string_fallback(
177+
coreConfig_,
178+
"websocket.host",
179+
"websocket_host",
180+
"0.0.0.0");
181+
131182
endpoint.port = port;
132183

133184
co_await listener_->async_listen(endpoint);
@@ -149,7 +200,12 @@ namespace vix::websocket
149200

150201
vix::async::core::task<void> LowLevelServer::start_server()
151202
{
152-
const int port = coreConfig_.getInt("websocket.port", 9090);
203+
const int port =
204+
get_config_int_fallback(
205+
coreConfig_,
206+
"websocket.port",
207+
"websocket_port",
208+
9090);
153209

154210
co_await init_listener(static_cast<unsigned short>(port));
155211

@@ -176,8 +232,8 @@ namespace vix::websocket
176232
init_logger_from_env_once();
177233
vix::utils::console_wait_banner();
178234

179-
start_io_threads();
180235
spawn_detached(*ioContext_, start_server());
236+
start_io_threads();
181237
}
182238

183239
void LowLevelServer::start_accept()

‎tests/CMakeLists.txt‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
cmake_minimum_required(VERSION 3.20)
2+
3+
include(CTest)
4+
5+
if (BUILD_TESTING)
6+
enable_testing()
7+
endif()
8+
9+
set(VIX_WEBSOCKET_TEST_TARGET vix::websocket)
10+
if (NOT TARGET ${VIX_WEBSOCKET_TEST_TARGET} AND TARGET vix_websocket)
11+
set(VIX_WEBSOCKET_TEST_TARGET vix_websocket)
12+
endif()
13+
14+
if (NOT TARGET ${VIX_WEBSOCKET_TEST_TARGET})
15+
message(FATAL_ERROR "[websocket/tests] Missing websocket target (expected vix::websocket or vix_websocket).")
16+
endif()
17+
18+
add_executable(websocket_disconnect_tests
19+
websocket_disconnect_tests.cpp
20+
)
21+
22+
target_link_libraries(websocket_disconnect_tests PRIVATE
23+
${VIX_WEBSOCKET_TEST_TARGET}
24+
)
25+
26+
target_compile_features(websocket_disconnect_tests PRIVATE cxx_std_20)
27+
28+
if (BUILD_TESTING)
29+
add_test(NAME websocket_disconnect_tests COMMAND websocket_disconnect_tests)
30+
endif()

‎tests/websocket_disconnect_tests.cpp‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
#include <vix/utils/NetworkError.hpp>
2+
3+
#include <cstdlib>
4+
#include <iostream>
5+
#include <string>
6+
7+
namespace
8+
{
9+
int failures = 0;
10+
11+
void expect_true(bool value, const std::string &name)
12+
{
13+
if (!value)
14+
{
15+
std::cerr << "FAILED: expected true: " << name << "\n";
16+
++failures;
17+
}
18+
}
19+
20+
void expect_false(bool value, const std::string &name)
21+
{
22+
if (value)
23+
{
24+
std::cerr << "FAILED: expected false: " << name << "\n";
25+
++failures;
26+
}
27+
}
28+
29+
void test_websocket_disconnect_messages()
30+
{
31+
expect_true(
32+
vix::utils::is_normal_network_disconnect_message("Broken pipe"),
33+
"Broken pipe");
34+
35+
expect_true(
36+
vix::utils::is_normal_network_disconnect_message("connection reset"),
37+
"connection reset");
38+
39+
expect_true(
40+
vix::utils::is_normal_network_disconnect_message("Connection reset by peer"),
41+
"Connection reset by peer");
42+
43+
expect_true(
44+
vix::utils::is_normal_network_disconnect_message("canceled"),
45+
"canceled");
46+
47+
expect_true(
48+
vix::utils::is_normal_network_disconnect_message("cancelled"),
49+
"cancelled");
50+
51+
expect_true(
52+
vix::utils::is_normal_network_disconnect_message("unexpected EOF while reading websocket HTTP head"),
53+
"unexpected EOF while reading websocket HTTP head");
54+
}
55+
56+
void test_websocket_protocol_errors_stay_visible()
57+
{
58+
expect_false(
59+
vix::utils::is_normal_network_disconnect_message("websocket handshake must use GET"),
60+
"websocket handshake must use GET");
61+
62+
expect_false(
63+
vix::utils::is_normal_network_disconnect_message("missing Upgrade: websocket"),
64+
"missing Upgrade: websocket");
65+
66+
expect_false(
67+
vix::utils::is_normal_network_disconnect_message("missing Sec-WebSocket-Key"),
68+
"missing Sec-WebSocket-Key");
69+
70+
expect_false(
71+
vix::utils::is_normal_network_disconnect_message("unsupported Sec-WebSocket-Version"),
72+
"unsupported Sec-WebSocket-Version");
73+
74+
expect_false(
75+
vix::utils::is_normal_network_disconnect_message("websocket frame write failed"),
76+
"websocket frame write failed");
77+
}
78+
}
79+
80+
int main()
81+
{
82+
test_websocket_disconnect_messages();
83+
test_websocket_protocol_errors_stay_visible();
84+
85+
if (failures != 0)
86+
{
87+
std::cerr << "websocket_disconnect_tests failed with "
88+
<< failures
89+
<< " failure(s)\n";
90+
91+
return EXIT_FAILURE;
92+
}
93+
94+
std::cout << "websocket_disconnect_tests passed\n";
95+
return EXIT_SUCCESS;
96+
}

0 commit comments

Comments
 (0)