Skip to content

Commit b2a7086

Browse files
committed
refactor: Harden StreamingTTS stop() method per review
Implements suggestions from the AI PR review to make the shutdown procedure more robust. - Nullifies all WebSocket event handlers (onopen, onmessage, etc.) to prevent lingering callbacks. - Defensively checks WebSocket readyState before calling close(). - Fully resets the audio element by calling load() in a try-catch block.
1 parent 2ee3845 commit b2a7086

1 file changed

Lines changed: 33 additions & 9 deletions

File tree

wiktionary_pron/scripts/tts.js

Lines changed: 33 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -234,28 +234,52 @@ class StreamingTTS {
234234
}
235235

236236
stop() {
237-
// 1. Close the socket first to prevent new messages.
237+
// 1. Completely and safely dismantle the WebSocket connection.
238238
if (this.#currentSocket) {
239-
this.#currentSocket.onclose = null; // Prevent the onclose handler from running
240-
this.#currentSocket.onerror = null; // Prevent the onerror handler
241-
this.#currentSocket.close();
239+
// Nullify ALL event handlers to prevent any lingering callbacks from firing.
240+
this.#currentSocket.onopen = null;
241+
this.#currentSocket.onmessage = null;
242+
this.#currentSocket.onerror = null;
243+
this.#currentSocket.onclose = null;
244+
245+
// Defensively close the socket only if it's in an active state.
246+
try {
247+
const state = this.#currentSocket.readyState;
248+
if (state === WebSocket.OPEN || state === WebSocket.CONNECTING) {
249+
// Use the spec-compliant close method with a normal closure code.
250+
this.#currentSocket.close(1000, "client stop");
251+
}
252+
} catch (e) {
253+
// This can happen in rare cases; it's safe to ignore.
254+
console.debug("Error while closing WebSocket, ignoring:", e);
255+
}
256+
242257
this.#currentSocket = null;
243258
}
244259

245-
// 2. Clear the queue to stop any pending appends.
260+
// 2. Clear internal state.
246261
this.#audioQueue = [];
247262
this.#isAppending = false;
248263

249-
// 3. Gracefully end the media stream.
250-
// This will handle the sourceBuffer and mediaSource correctly.
264+
// 3. Gracefully end the MediaSource stream.
251265
this.#finalizeStream();
252266

253-
// 4. Reset the audio player.
267+
// 4. Fully and safely reset the <audio> element.
254268
this.#audioPlayer.pause();
255-
if (this.#audioPlayer.src.startsWith("blob:")) {
269+
270+
// Revoke any object URL to prevent memory leaks.
271+
if (this.#audioPlayer.src && this.#audioPlayer.src.startsWith("blob:")) {
256272
URL.revokeObjectURL(this.#audioPlayer.src);
257273
}
274+
275+
// Remove the source and call load() to force the element to reset.
258276
this.#audioPlayer.removeAttribute("src");
277+
try {
278+
this.#audioPlayer.load();
279+
} catch (e) {
280+
// This can fail in some browsers/states; it's safe to ignore.
281+
console.debug("Error while resetting audio element, ignoring:", e);
282+
}
259283
}
260284

261285
// --- Private Helper Methods ---

0 commit comments

Comments
 (0)