Fix 7 corner cases found in second review pass

Critical:
- Persist serverProtocol/ts6ApiKey in database schema so TS6 config
  survives restarts (added columns + manager save/load)

Medium:
- Clear udpErrorTimer on disconnect to prevent memory leak and stale
  log messages from firing after teardown
- Guard against double connect() by disconnecting old client first
- Add settled guard in TS6HttpQuery.request() to prevent double
  reject when both res error and req error fire
- Add res.on("error") handler to TS6HttpQuery response stream

Low:
- Cap probeTS3Query banner buffer at 256 bytes to prevent memory abuse
  from non-TS3 services sending large data on port 10011
- Remove unnecessary EventEmitter inheritance from TS6HttpQuery
- Update database test fixtures with new serverProtocol/ts6ApiKey fields

https://claude.ai/code/session_016WhH58avUD9xy2dgADJgTh
This commit is contained in:
Claude committed 2026-04-03 13:38:49 +00:00
1 parent 9e45193e2c
commit 6184f38330
6 files changed
+54 -15

No files matched your search

+8 -2
View File
@@ -85,6 +85,8 @@ export class BotManager {
defaultChannel: params.defaultChannel ?? "", defaultChannel: params.defaultChannel ?? "",
channelPassword: params.channelPassword ?? "", channelPassword: params.channelPassword ?? "",
autoStart: params.autoStart ?? false, autoStart: params.autoStart ?? false,
serverProtocol: params.serverProtocol ?? "",
ts6ApiKey: params.ts6ApiKey ?? "",
}); });
this.logger.info({ botId: id, name: params.name }, "Bot instance created"); this.logger.info({ botId: id, name: params.name }, "Bot instance created");
@@ -114,6 +116,8 @@ export class BotManager {
nickname: params.nickname ?? existing.nickname, nickname: params.nickname ?? existing.nickname,
defaultChannel: params.defaultChannel ?? existing.defaultChannel, defaultChannel: params.defaultChannel ?? existing.defaultChannel,
channelPassword: params.channelPassword ?? existing.channelPassword, channelPassword: params.channelPassword ?? existing.channelPassword,
serverProtocol: params.serverProtocol ?? existing.serverProtocol,
ts6ApiKey: params.ts6ApiKey ?? existing.ts6ApiKey,
}); });
// Update in-memory name immediately (other fields need reconnect) // Update in-memory name immediately (other fields need reconnect)
const bot = this.bots.get(id); const bot = this.bots.get(id);
@@ -150,17 +154,19 @@ export class BotManager {
async loadSavedBots(): Promise<void> { async loadSavedBots(): Promise<void> {
const savedInstances = this.database.getBotInstances(); const savedInstances = this.database.getBotInstances();
for (const saved of savedInstances) { for (const saved of savedInstances) {
const proto = saved.serverProtocol as "ts3" | "ts6" | "" | undefined;
const bot = new BotInstance({ const bot = new BotInstance({
id: saved.id, id: saved.id,
name: saved.name, name: saved.name,
tsOptions: { tsOptions: {
host: saved.serverAddress, host: saved.serverAddress,
port: saved.serverPort, port: saved.serverPort,
queryPort: 10011, // Will be overridden by auto-detection for TS6 queryPort: proto === "ts6" ? 10080 : 10011,
nickname: saved.nickname, nickname: saved.nickname,
defaultChannel: saved.defaultChannel || undefined, defaultChannel: saved.defaultChannel || undefined,
channelPassword: saved.channelPassword || undefined, channelPassword: saved.channelPassword || undefined,
// Protocol will be auto-detected on connect serverProtocol: proto === "ts3" || proto === "ts6" ? proto : undefined,
ts6ApiKey: saved.ts6ApiKey || undefined,
}, },
neteaseProvider: this.neteaseProvider, neteaseProvider: this.neteaseProvider,
qqProvider: this.qqProvider, qqProvider: this.qqProvider,
+4
View File
@@ -60,6 +60,8 @@ describe("database", () => {
defaultChannel: "Music", defaultChannel: "Music",
channelPassword: "", channelPassword: "",
autoStart: true, autoStart: true,
serverProtocol: "",
ts6ApiKey: "",
}; };
botDb.saveBotInstance(instance); botDb.saveBotInstance(instance);
@@ -86,6 +88,8 @@ describe("database", () => {
defaultChannel: "Music", defaultChannel: "Music",
channelPassword: "", channelPassword: "",
autoStart: false, autoStart: false,
serverProtocol: "",
ts6ApiKey: "",
}); });
expect(botDb.deleteBotInstance("bot1")).toBe(true); expect(botDb.deleteBotInstance("bot1")).toBe(true);
+12 -4
View File
@@ -24,6 +24,10 @@ export interface BotInstance {
defaultChannel: string; defaultChannel: string;
channelPassword: string; channelPassword: string;
autoStart: boolean; autoStart: boolean;
/** "ts3" | "ts6" | "" (empty = auto-detect) */
serverProtocol: string;
/** API key for TS6 HTTP Query */
ts6ApiKey: string;
} }
export interface BotDatabase { export interface BotDatabase {
@@ -58,7 +62,9 @@ function initTables(db: Database.Database): void {
nickname TEXT NOT NULL, nickname TEXT NOT NULL,
defaultChannel TEXT NOT NULL, defaultChannel TEXT NOT NULL,
channelPassword TEXT NOT NULL, channelPassword TEXT NOT NULL,
autoStart INTEGER NOT NULL DEFAULT 0 autoStart INTEGER NOT NULL DEFAULT 0,
serverProtocol TEXT NOT NULL DEFAULT '',
ts6ApiKey TEXT NOT NULL DEFAULT ''
); );
`); `);
} }
@@ -78,8 +84,8 @@ export function createDatabase(dbPath: string): BotDatabase {
`); `);
const upsertInstance = db.prepare(` const upsertInstance = db.prepare(`
INSERT INTO bot_instances (id, name, serverAddress, serverPort, nickname, defaultChannel, channelPassword, autoStart) INSERT INTO bot_instances (id, name, serverAddress, serverPort, nickname, defaultChannel, channelPassword, autoStart, serverProtocol, ts6ApiKey)
VALUES (@id, @name, @serverAddress, @serverPort, @nickname, @defaultChannel, @channelPassword, @autoStart) VALUES (@id, @name, @serverAddress, @serverPort, @nickname, @defaultChannel, @channelPassword, @autoStart, @serverProtocol, @ts6ApiKey)
ON CONFLICT(id) DO UPDATE SET ON CONFLICT(id) DO UPDATE SET
name = excluded.name, name = excluded.name,
serverAddress = excluded.serverAddress, serverAddress = excluded.serverAddress,
@@ -87,7 +93,9 @@ export function createDatabase(dbPath: string): BotDatabase {
nickname = excluded.nickname, nickname = excluded.nickname,
defaultChannel = excluded.defaultChannel, defaultChannel = excluded.defaultChannel,
channelPassword = excluded.channelPassword, channelPassword = excluded.channelPassword,
autoStart = excluded.autoStart autoStart = excluded.autoStart,
serverProtocol = excluded.serverProtocol,
ts6ApiKey = excluded.ts6ApiKey
`); `);
const selectInstances = db.prepare(`SELECT * FROM bot_instances`); const selectInstances = db.prepare(`SELECT * FROM bot_instances`);
+15 -3
View File
@@ -53,6 +53,7 @@ export class TS3Client extends EventEmitter {
private disconnecting = false; private disconnecting = false;
private detectedProtocol: ServerProtocol = "unknown"; private detectedProtocol: ServerProtocol = "unknown";
private httpQuery: TS6HttpQuery | null = null; private httpQuery: TS6HttpQuery | null = null;
private udpErrorTimer: ReturnType<typeof setTimeout> | null = null;
constructor(private options: TS3ClientOptions, logger: Logger) { constructor(private options: TS3ClientOptions, logger: Logger) {
super(); super();
@@ -118,6 +119,14 @@ export class TS3Client extends EventEmitter {
}); });
} }
// Guard against calling connect() while already connected
if (this.client) {
this.logger.warn("connect() called while already connected, disconnecting first");
this.disconnect();
// Give the old client a moment to tear down
await new Promise((r) => setTimeout(r, 100));
}
this.logger.info( this.logger.info(
{ addr, protocol: this.detectedProtocol }, { addr, protocol: this.detectedProtocol },
"Connecting to TeamSpeak server (full client protocol)", "Connecting to TeamSpeak server (full client protocol)",
@@ -125,19 +134,18 @@ export class TS3Client extends EventEmitter {
// Throttle repeated "udp send error" warnings (fires every 20ms during playback if UDP breaks) // Throttle repeated "udp send error" warnings (fires every 20ms during playback if UDP breaks)
let udpErrorCount = 0; let udpErrorCount = 0;
let udpErrorTimer: ReturnType<typeof setTimeout> | null = null;
const throttledWarn = (msg: string, ...args: unknown[]) => { const throttledWarn = (msg: string, ...args: unknown[]) => {
if (typeof msg === "string" && msg.includes("udp send error")) { if (typeof msg === "string" && msg.includes("udp send error")) {
udpErrorCount++; udpErrorCount++;
if (udpErrorCount === 1) { if (udpErrorCount === 1) {
this.logger.warn(msg); this.logger.warn(msg);
// After 2 seconds, log a summary and reset // After 2 seconds, log a summary and reset
udpErrorTimer = setTimeout(() => { this.udpErrorTimer = setTimeout(() => {
if (udpErrorCount > 1) { if (udpErrorCount > 1) {
this.logger.warn(`udp send error (repeated ${udpErrorCount} times, connection may be lost)`); this.logger.warn(`udp send error (repeated ${udpErrorCount} times, connection may be lost)`);
} }
udpErrorCount = 0; udpErrorCount = 0;
udpErrorTimer = null; this.udpErrorTimer = null;
}, 2000); }, 2000);
} }
return; return;
@@ -285,6 +293,10 @@ export class TS3Client extends EventEmitter {
this.clientId = 0; this.clientId = 0;
this.httpQuery = null; this.httpQuery = null;
this.detectedProtocol = "unknown"; this.detectedProtocol = "unknown";
if (this.udpErrorTimer) {
clearTimeout(this.udpErrorTimer);
this.udpErrorTimer = null;
}
this.logger.info("Disconnected from TeamSpeak server"); this.logger.info("Disconnected from TeamSpeak server");
} }
} }
+13 -6
View File
@@ -1,6 +1,5 @@
import http from "node:http"; import http from "node:http";
import https from "node:https"; import https from "node:https";
import { EventEmitter } from "node:events";
export interface HttpQueryOptions { export interface HttpQueryOptions {
host: string; host: string;
@@ -31,11 +30,10 @@ export interface HttpQueryResult {
* GET /1/channellist?sid={sid} → list channels * GET /1/channellist?sid={sid} → list channels
* POST /1/clientupdate → update client properties * POST /1/clientupdate → update client properties
*/ */
export class TS6HttpQuery extends EventEmitter { export class TS6HttpQuery {
private options: Required<HttpQueryOptions>; private options: Required<HttpQueryOptions>;
constructor(options: HttpQueryOptions) { constructor(options: HttpQueryOptions) {
super();
this.options = { this.options = {
host: options.host, host: options.host,
port: options.port, port: options.port,
@@ -68,6 +66,13 @@ export class TS6HttpQuery extends EventEmitter {
} }
return new Promise((resolve, reject) => { return new Promise((resolve, reject) => {
let settled = false;
const fail = (err: Error) => {
if (settled) return;
settled = true;
reject(err);
};
const req = transport.request( const req = transport.request(
{ {
hostname: host, hostname: host,
@@ -82,8 +87,10 @@ export class TS6HttpQuery extends EventEmitter {
let data = ""; let data = "";
res.setEncoding("utf-8"); res.setEncoding("utf-8");
res.on("data", (chunk: string) => (data += chunk)); res.on("data", (chunk: string) => (data += chunk));
res.on("error", reject); res.on("error", fail);
res.on("end", () => { res.on("end", () => {
if (settled) return;
settled = true;
let parsed: unknown; let parsed: unknown;
try { try {
parsed = JSON.parse(data); parsed = JSON.parse(data);
@@ -98,10 +105,10 @@ export class TS6HttpQuery extends EventEmitter {
}, },
); );
req.on("error", reject); req.on("error", fail);
req.on("timeout", () => { req.on("timeout", () => {
req.destroy(); req.destroy();
reject(new Error("TS6 HTTP Query timeout")); fail(new Error("TS6 HTTP Query timeout"));
}); });
if (bodyStr) { if (bodyStr) {
+2
View File
@@ -67,11 +67,13 @@ function probeTS3Query(host: string, port: number, timeoutMs: number): Promise<b
const socket = net.createConnection({ host, port, timeout: timeoutMs }); const socket = net.createConnection({ host, port, timeout: timeoutMs });
let banner = ""; let banner = "";
const MAX_BANNER = 256; // TS3 banner is ~50 bytes; cap to avoid memory abuse
socket.setTimeout(timeoutMs); socket.setTimeout(timeoutMs);
socket.on("data", (data: Buffer) => { socket.on("data", (data: Buffer) => {
banner += data.toString("utf-8"); banner += data.toString("utf-8");
if (banner.length > MAX_BANNER) banner = banner.slice(0, MAX_BANNER);
if (banner.includes("TS3")) { if (banner.includes("TS3")) {
done(true); done(true);
} }