Skip to content

fix: report modal dialog blocks on connect timeout #13

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
spi3 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: report modal dialog blocks on connect timeout #13

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
127 changes: 106 additions & 21 deletions src/teamspeak_cli/cli/command_router.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,13 @@ const std::vector<CommandDoc>& command_docs() {
{{"--release-tag <tag>", "Install a specific GitHub release tag instead of the latest release.", {}, "latest release"}},
{"ts update", "ts update --release-tag v1.2.3"},
},
{{"connect"}, "ts connect", "Open a TeamSpeak server connection and wait for completion", {}},
{
{"connect"},
"ts connect [--timeout-ms N]",
"Open a TeamSpeak server connection and wait for completion",
{{"--timeout-ms <N>", "How long to wait before reporting connect timeout.", "integer milliseconds", "15000"}},
{"ts connect", "ts connect --timeout-ms 5000"},
},
{{"disconnect"}, "ts disconnect", "Ask the TeamSpeak client plugin to close the current connection", {}},
{{"mute"}, "ts mute", "Mute your TeamSpeak microphone", {}},
{{"unmute"}, "ts unmute", "Unmute your TeamSpeak microphone", {}},
Expand Down Expand Up @@ -602,7 +608,10 @@ auto contextualize_error(const ParsedCommand& command, domain::Error error) -> d
}

if (path == "client inspect-windows" && error.code == "xdotool_not_found") {
add_error_hint(error, "Install xdotool or set `TS_CLIENT_XDOTOOL` to a usable xdotool binary.");
add_error_hint(
error,
"Install xdotool (set `TS_CLIENT_XDOTOOL`) or xwininfo (set `TS_CLIENT_XWININFO`) to inspect windows."
);
}

if (path == "client start" &&
Expand Down Expand Up @@ -1592,6 +1601,18 @@ auto resolve_xdotool_paths() -> std::optional<XdotoolPaths> {
};
}

auto resolve_xwininfo_path() -> std::optional<std::filesystem::path> {
if (const char* explicit_xwininfo = std::getenv("TS_CLIENT_XWININFO");
explicit_xwininfo != nullptr && *explicit_xwininfo != '\0') {
const std::filesystem::path explicit_path(explicit_xwininfo);
if (is_executable_file(explicit_path)) {
return explicit_path;
}
return std::nullopt;
}
return find_executable_on_path("xwininfo");
}

auto managed_xvfb_root_for_path(const std::filesystem::path& xvfb_path) -> std::optional<std::filesystem::path> {
const auto bin_dir = xvfb_path.parent_path();
if (bin_dir.filename() != "bin") {
Expand Down Expand Up @@ -2051,6 +2072,52 @@ auto xdotool_run(const XdotoolPaths& xdotool, std::string_view display, const st
(void)run_command_capture_stdout(argv, xdotool_env(xdotool, display));
}

auto xwininfo_tree_windows(const std::filesystem::path& xwininfo, std::string_view display)
-> std::vector<ClientWindowInfo> {
const auto lines = split_output_lines(run_command_capture_stdout(
{xwininfo.string(), "-root", "-tree", "-display", std::string(display)}
));
std::vector<ClientWindowInfo> windows;
windows.reserve(lines.size());
for (const auto& line : lines) {
const auto trimmed = util::trim(line);
const auto id_end = trimmed.find(' ');
if (id_end == std::string::npos) {
continue;
}

const auto id = util::trim(trimmed.substr(0, id_end));
if (id.empty() || id.rfind("0x", 0) != 0) {
continue;
}

const auto title_begin = trimmed.find('"');
if (title_begin == std::string::npos) {
continue;
}
const auto title_end = trimmed.find('"', title_begin + 1);
if (title_end == std::string::npos) {
continue;
}

const std::string title = trimmed.substr(title_begin + 1, title_end - title_begin - 1);
if (title.empty() || title == "TeamSpeak 3") {
continue;
}
if (title.find("TeamSpeak") == std::string::npos && title.find("ts3client") == std::string::npos &&
line.find("TeamSpeak") == std::string::npos && line.find("ts3client") == std::string::npos) {
continue;
}
if (std::any_of(windows.begin(), windows.end(), [&](const auto& existing) {
return existing.id == id;
})) {
continue;
}
windows.push_back(ClientWindowInfo{.id = std::string(id), .title = title});
}
return windows;
}

auto xdotool_window_size(const XdotoolPaths& xdotool, std::string_view display, std::string_view window_id)
-> std::optional<std::pair<int, int>> {
const auto geometry_lines = split_output_lines(run_command_capture_stdout(
Expand Down Expand Up @@ -2903,31 +2970,39 @@ auto inspect_client_windows() -> domain::Result<ClientWindowInspection> {
}

const auto xdotool = resolve_xdotool_paths();
if (!xdotool.has_value()) {
if (xdotool.has_value()) {
ClientWindowInspection inspection{
.display = display.value(),
.windows = {},
};
for (const auto& window_id : xdotool_visible_window_ids(*xdotool, inspection.display)) {
if (std::any_of(inspection.windows.begin(), inspection.windows.end(), [&](const auto& window) {
return window.id == window_id;
})) {
continue;
}
inspection.windows.push_back(ClientWindowInfo{
.id = window_id,
.title = xdotool_window_name(*xdotool, inspection.display, window_id).value_or("-"),
});
}
return domain::ok(std::move(inspection));
}

const auto xwininfo = resolve_xwininfo_path();
if (!xwininfo.has_value()) {
return domain::fail<ClientWindowInspection>(client_error(
"xdotool_not_found",
"xdotool is required to inspect visible TeamSpeak windows; install xdotool or set TS_CLIENT_XDOTOOL",
"Either xdotool or xwininfo is required to inspect visible TeamSpeak windows; install a tool or set TS_CLIENT_XDOTOOL/TS_CLIENT_XWININFO",
domain::ExitCode::not_found
));
}

ClientWindowInspection inspection{
const auto windows = xwininfo_tree_windows(*xwininfo, display.value());
return domain::ok(ClientWindowInspection{
.display = display.value(),
.windows = {},
};
for (const auto& window_id : xdotool_visible_window_ids(*xdotool, inspection.display)) {
if (std::any_of(inspection.windows.begin(), inspection.windows.end(), [&](const auto& window) {
return window.id == window_id;
})) {
continue;
}
inspection.windows.push_back(ClientWindowInfo{
.id = window_id,
.title = xdotool_window_name(*xdotool, inspection.display, window_id).value_or("-"),
});
}

return domain::ok(std::move(inspection));
.windows = windows,
});
}

auto client_window_value(const ClientWindowInfo& window) -> output::ValueHolder {
Expand Down Expand Up @@ -3022,6 +3097,9 @@ auto connect_timeout_diagnostics(const domain::Profile& profile) -> ConnectTimeo
diagnostics.hints.push_back(
"Resolve the visible TeamSpeak window before retrying: " + util::join(titles, ", ") + "."
);
for (const auto& title : titles) {
diagnostics.hints.push_back("TeamSpeak client is blocked by modal dialog: " + title);
}
if (std::find(titles.begin(), titles.end(), "License agreement") != titles.end()) {
diagnostics.hints.push_back(
"If you accept the TeamSpeak license terms, rerun `ts client start --accept-license` or set `TS_CLIENT_ACCEPT_LICENSE=1` for first-run automation."
Expand Down Expand Up @@ -6286,9 +6364,16 @@ auto CommandRouter::dispatch(const ParsedCommand& command, const ProgressSink& p
if (stream_progress) {
progress(connect_start_message(resolved.profile));
}
const auto timeout = parse_timeout_ms(
command.options,
std::chrono::duration_cast<std::chrono::milliseconds>(kConnectCompletionTimeout).count()
);
if (!timeout) {
return domain::fail<output::CommandOutput>(timeout.error());
}
auto connected = session.connect_and_wait(
build_connect_request(resolved.profile),
kConnectCompletionTimeout,
timeout.value(),
stream_progress ? session::ConnectEventCallback([&](const domain::Event& event) {
progress(output::connect_progress_message(event));
})
Expand Down
79 changes: 79 additions & 0 deletions tests/cli_router_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,12 @@ int main() {
"ts client logs [--count N]",
"client help should include the client logs example"
);
const std::string connect_help = router.render_help({"connect"});
tests::expect_contains(
connect_help,
"--timeout-ms <N> How long to wait before reporting connect timeout. (accepted: integer milliseconds; default: 15000)",
"connect help should describe timeout option"
);

const std::string playback_help = router.render_help({"playback"});
tests::expect_contains(
Expand Down Expand Up @@ -347,6 +353,13 @@ int main() {
output::Format::ndjson,
"events watch should accept ndjson output"
);
auto parsed_connect_timeout = parse_command(router, {"connect", "--timeout-ms", "250"});
tests::expect(parsed_connect_timeout.ok(), "connect parse should accept timeout-ms");
tests::expect_eq(
parsed_connect_timeout.value().options.at("timeout-ms"),
std::string("250"),
"connect parse should record timeout-ms"
);

auto unsupported_ndjson = parse_command(router, {"status", "--output", "ndjson"});
tests::expect(!unsupported_ndjson.ok(), "non-watch command should reject ndjson");
Expand Down Expand Up @@ -1261,6 +1274,34 @@ int main() {
connect_table.find("What Happened") == std::string::npos,
"connect table should not repeat streamed lifecycle narration"
);
auto connect_with_custom_timeout = parse_command(
router,
{
"--profile",
"mock-local",
"--config",
config_path.string(),
"--server",
"voice.example.com:9987",
"--nickname",
"cli-tester",
"connect",
"--timeout-ms",
"250",
}
);
tests::expect(connect_with_custom_timeout.ok(), "connect parse should accept custom timeout");
auto connect_with_custom_timeout_result = router.dispatch(connect_with_custom_timeout.value());
tests::expect(connect_with_custom_timeout_result.ok(), "connect with custom timeout should succeed");
const auto connect_with_custom_timeout_json = output::render(
connect_with_custom_timeout_result.value(),
output::Format::json
);
tests::expect_contains(
connect_with_custom_timeout_json,
"\"timeout_ms\":250",
"connect with custom timeout should report the configured timeout"
);

auto expect_connect_server = [&](const std::string& server_value, const std::string& expected_target) {
auto connect = parse_command(
Expand Down Expand Up @@ -2113,6 +2154,44 @@ int main() {
"\"title\":\"License agreement\"",
"client inspect-windows should report visible TeamSpeak dialog titles"
);
const fs::path fallback_bin_dir = temp_dir / "headless-fallback-bin";
const fs::path fallback_xwininfo_path = fallback_bin_dir / "xwininfo";
{
std::error_code create_ec;
fs::create_directories(fallback_bin_dir, create_ec);
std::ofstream fallback_xwininfo(fallback_xwininfo_path, std::ios::trunc);
fallback_xwininfo << "#!/bin/sh\n";
fallback_xwininfo << "if [ \"${1:-}\" = \"-root\" ] && [ \"${2:-}\" = \"-tree\" ] && [ \"${3:-}\" = \"-display\" ]; then\n";
fallback_xwininfo << " echo ' 0x2a00001 \"License agreement\": (\"ts3client_linux_amd64\" \"TeamSpeak 3\")'\n";
fallback_xwininfo << " echo ' 0x2a00002 \"Identities\": (\"ts3client_linux_amd64\" \"TeamSpeak 3\")'\n";
fallback_xwininfo << "fi\n";
}
fs::permissions(
fallback_xwininfo_path,
fs::perms::owner_read | fs::perms::owner_write | fs::perms::owner_exec,
fs::perm_options::replace
);
{
EnvGuard fake_xdotool_missing_env("TS_CLIENT_XDOTOOL", "");
EnvGuard fake_xwininfo_env("TS_CLIENT_XWININFO", fallback_xwininfo_path.string());
EnvGuard fake_path_env("PATH", fallback_bin_dir.string());
auto fallback_inspect_windows = parse_command(router, {"client", "inspect-windows"});
tests::expect(fallback_inspect_windows.ok(), "client inspect-windows should parse without xdotool");
auto fallback_inspect_windows_result = router.dispatch(fallback_inspect_windows.value());
tests::expect(fallback_inspect_windows_result.ok(), "client inspect-windows should parse via xwininfo");
const auto fallback_inspect_windows_json =
output::render(fallback_inspect_windows_result.value(), output::Format::json);
tests::expect_contains(
fallback_inspect_windows_json,
"\"title\":\"License agreement\"",
"client inspect-windows should include fallback license dialog title"
);
tests::expect_contains(
fallback_inspect_windows_json,
"\"title\":\"Identities\"",
"client inspect-windows should include fallback identity dialog title"
);
}

auto headless_stop_result = router.dispatch(client_stop.value());
tests::expect(headless_stop_result.ok(), "headless client stop should succeed");
Expand Down
Loading