mirror of
https://github.com/ZHANGTIANYAO1/teamspeak-music-bot.git
synced 2026-10-01 20:42:50 +08:00
fix(avatar): don't fire a doomed avatar upload before TeamSpeak connects (#148)
BotInstance loads the persisted custom avatar in its constructor and handed
it to profileManager.setCustomAvatar(). On an idle bot that method
immediately starts the three-step file transfer
(fileTransferInitUpload -> uploadFileData -> clientupdate) — but the
constructor runs long before tsClient.connect(), so TS3Client.client is
still null and the very first step throws "Not connected".
Scope of the bug: setCustomAvatar stores the buffer before attempting the
upload, and profileManager.onConnect() re-applies this.customAvatar once the
handshake completes, so the avatar itself did end up on the server. What the
premature call actually cost was a guaranteed-to-fail file transfer plus a
"Profile update failed" warning on every bot start — and every restart, since
manager.startBot() tears the instance down and reconstructs it. ("Not
connected" is not in handleFeatureError's unrecoverable list, so it never
disabled the avatar feature.)
Add loadCustomAvatar(), which only stores the buffer, and use it at the
constructor call site. onConnect() was already doing the real work, so
nothing is lost. Guard on length > 0 as well: avatarStore.write() is
delete-then-write, so a crash mid-write leaves a 0-byte file, and a 0-byte
Buffer is truthy — previously that took setCustomAvatar's else branch and
fired two more doomed calls (fileTransferDeleteFile + a clear).
setCustomAvatar keeps its immediate-apply behaviour, so editing the avatar
from the WebUI on a live bot still takes effect right away.
Reported-by: @shenmu-rua
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
b92543f337
commit
7c3926a2ae
3 files changed
+78
-1
No files matched your search
+7
-1
@@ -295,7 +295,13 @@ export class BotInstance extends EventEmitter {
|
|||||||
const relPath = this.database.getCustomAvatarPath(this.id);
|
const relPath = this.database.getCustomAvatarPath(this.id);
|
||||||
if (relPath) {
|
if (relPath) {
|
||||||
const buf = this.avatarStore.read(relPath);
|
const buf = this.avatarStore.read(relPath);
|
||||||
if (buf) this.profileManager.setCustomAvatar(buf);
|
// loadCustomAvatar, NOT setCustomAvatar (#148): we are still in the
|
||||||
|
// constructor, so tsClient has not connected. setCustomAvatar would
|
||||||
|
// start a file transfer right here and fail. profileManager.onConnect()
|
||||||
|
// uploads it for real once the handshake completes.
|
||||||
|
// `length > 0` because avatarStore.write is delete-then-write, so a
|
||||||
|
// crash mid-write leaves a 0-byte file that is truthy as a Buffer.
|
||||||
|
if (buf && buf.length > 0) this.profileManager.loadCustomAvatar(buf);
|
||||||
}
|
}
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
this.logger.warn({ err }, "Failed to load custom avatar — skipping");
|
this.logger.warn({ err }, "Failed to load custom avatar — skipping");
|
||||||
|
|||||||
@@ -145,3 +145,60 @@ describe("BotProfileManager custom avatar precedence", () => {
|
|||||||
expect(ts.clearCalls).toBe(0);
|
expect(ts.clearCalls).toBe(0);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// #148: the persisted avatar is loaded in the BotInstance constructor, before
|
||||||
|
// tsClient.connect() has run. Loading it must not touch the wire at all.
|
||||||
|
describe("BotProfileManager loadCustomAvatar (pre-connect load, #148)", () => {
|
||||||
|
let ts: ReturnType<typeof makeMockTs>;
|
||||||
|
beforeEach(() => { ts = makeMockTs(); });
|
||||||
|
|
||||||
|
it("does not upload or clear anything when called before connect", async () => {
|
||||||
|
const pm = new BotProfileManager(ts as any, noopLogger, cfgOn, "Bot");
|
||||||
|
pm.loadCustomAvatar(Buffer.from([7, 7, 7]));
|
||||||
|
await flush();
|
||||||
|
expect(ts.uploadCalls.length).toBe(0);
|
||||||
|
expect(ts.clearCalls).toBe(0);
|
||||||
|
expect(ts.fileTransferInitUpload).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("the loaded avatar is uploaded once onConnect fires", async () => {
|
||||||
|
const pm = new BotProfileManager(ts as any, noopLogger, cfgOn, "Bot");
|
||||||
|
pm.loadCustomAvatar(Buffer.from([7, 7, 7]));
|
||||||
|
await flush();
|
||||||
|
pm.onConnect();
|
||||||
|
await flush();
|
||||||
|
expect(ts.uploadCalls.length).toBe(1);
|
||||||
|
expect(ts.uploadCalls[0].equals(Buffer.from([7, 7, 7]))).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("survives a reconnect: onConnect re-applies the loaded avatar every time", async () => {
|
||||||
|
const pm = new BotProfileManager(ts as any, noopLogger, cfgOn, "Bot");
|
||||||
|
pm.loadCustomAvatar(Buffer.from([8]));
|
||||||
|
pm.onConnect();
|
||||||
|
await flush();
|
||||||
|
pm.onConnect();
|
||||||
|
await flush();
|
||||||
|
expect(ts.uploadCalls.length).toBe(2);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("loading null leaves the wire untouched and onConnect stays quiet", async () => {
|
||||||
|
const pm = new BotProfileManager(ts as any, noopLogger, cfgOn, "Bot");
|
||||||
|
pm.loadCustomAvatar(null);
|
||||||
|
pm.onConnect();
|
||||||
|
await flush();
|
||||||
|
expect(ts.uploadCalls.length).toBe(0);
|
||||||
|
expect(ts.clearCalls).toBe(0);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("setCustomAvatar still uploads immediately after connect (post-connect edit unchanged)", async () => {
|
||||||
|
const pm = new BotProfileManager(ts as any, noopLogger, cfgOn, "Bot");
|
||||||
|
pm.loadCustomAvatar(Buffer.from([1]));
|
||||||
|
pm.onConnect();
|
||||||
|
await flush();
|
||||||
|
ts.uploadCalls.length = 0;
|
||||||
|
pm.setCustomAvatar(Buffer.from([2, 2]));
|
||||||
|
await flush();
|
||||||
|
expect(ts.uploadCalls.length).toBe(1);
|
||||||
|
expect(ts.uploadCalls[0].equals(Buffer.from([2, 2]))).toBe(true);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -65,6 +65,20 @@ export class BotProfileManager {
|
|||||||
|
|
||||||
// --- Public API ---
|
// --- Public API ---
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Store a persisted custom avatar WITHOUT touching TeamSpeak (#148).
|
||||||
|
*
|
||||||
|
* Used during BotInstance construction, when the TS connection does not
|
||||||
|
* exist yet: setCustomAvatar would immediately fire the three-step file
|
||||||
|
* transfer (fileTransferInitUpload → uploadFileData → clientupdate) against
|
||||||
|
* a client that has not connected, so the upload always failed and the
|
||||||
|
* saved avatar never appeared. onConnect() re-applies this.customAvatar
|
||||||
|
* once the handshake completes, so loading it silently here loses nothing.
|
||||||
|
*/
|
||||||
|
loadCustomAvatar(buffer: Buffer | null): void {
|
||||||
|
this.customAvatar = buffer;
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Set/clear the persistent idle avatar. Pass null to remove.
|
* Set/clear the persistent idle avatar. Pass null to remove.
|
||||||
*
|
*
|
||||||
|
|||||||
Reference in new issue
Block a user