From 7c3926a2aeea4e4b599206bb4bac58a99d939876 Mon Sep 17 00:00:00 2001 From: saopig1 <4x7sw862st@gmail.com> Date: Fri, 14 Aug 2026 00:59:56 +0800 Subject: [PATCH] fix(avatar): don't fire a doomed avatar upload before TeamSpeak connects (#148) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- src/bot/instance.ts | 8 +++++- src/bot/profile.test.ts | 57 +++++++++++++++++++++++++++++++++++++++++ src/bot/profile.ts | 14 ++++++++++ 3 files changed, 78 insertions(+), 1 deletion(-) diff --git a/src/bot/instance.ts b/src/bot/instance.ts index 8cc285e..58e74ca 100755 --- a/src/bot/instance.ts +++ b/src/bot/instance.ts @@ -295,7 +295,13 @@ export class BotInstance extends EventEmitter { const relPath = this.database.getCustomAvatarPath(this.id); if (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) { this.logger.warn({ err }, "Failed to load custom avatar — skipping"); diff --git a/src/bot/profile.test.ts b/src/bot/profile.test.ts index c0dc95b..59ebf93 100644 --- a/src/bot/profile.test.ts +++ b/src/bot/profile.test.ts @@ -145,3 +145,60 @@ describe("BotProfileManager custom avatar precedence", () => { 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; + 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); + }); +}); diff --git a/src/bot/profile.ts b/src/bot/profile.ts index 332638b..3c04ef7 100644 --- a/src/bot/profile.ts +++ b/src/bot/profile.ts @@ -65,6 +65,20 @@ export class BotProfileManager { // --- 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. *