some fixes by rly-dev · Pull Request #174 · Tomato6966/lavalink-client

rly-dev · 2026-02-14T12:15:13Z

In Queue.ts there are two fixes

the sync() uses wrong array length for previous

in Queue.utils.sync() when overriding the previous array with data from the queue store the splice call used is this.tracks.length as the delete count instead of this.previous.length

missing null-safety on optional queueOptions in constructor

the constructor parameter queueOptions is typed as queueOptions?: ManagerQueueOptions (optional) but line 153 accessed it as queueOptions.queueChangesWatcher without optional chaining

In Node.ts there are two fixes

race condition on error() handler

when a WebSocket error occurred with closeOnError enabled the handler called this.reconnect() first then called this.socket?.close() the close() call fires a close event on the socket which triggers the close handler and that handler also calls reconnect()

off-bye-one in error rate-limiting ( two location )

both trackStuck() and trackError() filter old timestamps push the current timestamp then check if (oldTimestamps.length > maxAmount) but oldTimestamps was captured before the current error was added to the stored array so it doesn't include the current error

In Filters.ts two fixes

resetFilters() karaoke resets twice but vaporwave doesnt reset

in resetFilters()two consecutive lines both set this.filters.karaoke = false the second line was clearly meant to be this.filters.vaporwave = false

setSpeed / setPitch / setRate overwrite each others values

each of these three methods replaced the entire this.data.timescale object by spreading DEFAULT_FILTER_DATAS.timescale (which has speed: 1, pitch: 1, rate: 1) and then overriding only their own property for example setSpeed(2) produces { speed: 2, pitch: 1, rate: 1 }

In Player.ts one fix

changeNode() enables SponsorBlock on node change default

in changeNode() when the new node supports SponsorBlock, the code checked if sponsorBlockCategories was a non-empty array if it was it re-applied those categories (correct) but the else branch called setSponsorBlock() with no arguments which applies default categories

Commit History and Details
dc406be Fix Queue sync & constructor
56799fe Fix Node reconnect & ratelimit
7d2cdd6 Fix Filters reset & timescale
19ee6aa Fix Player SponsorBlock default

… constructor - sync(): Used this.tracks.length instead of this.previous.length when splicing the previous array, causing incomplete overrides that corrupt queue state on sync. - constructor: queueOptions parameter is optional but was accessed without optional chaining, causing TypeError when omitted.

…r rate-limiting - error(): reconnect() was called before socket.close(), causing duplicate reconnection attempts. Now reconnect only runs when closeOnError is false; otherwise the close event handles it. - trackStuck()/trackError(): Used > instead of >= in rate-limiting, allowing one extra error beyond the configured maxAmount before destroying the player.

…setSpeed/setPitch/setRate - resetFilters(): Duplicate karaoke reset instead of resetting vaporwave, leaving the vaporwave flag active after a full reset. - setSpeed/setPitch/setRate: Each method replaced the entire timescale object from defaults, discarding prior customizations from the other methods. Now preserves existing timescale state.

- changeNode(): The else branch unconditionally called setSponsorBlock() with default categories when moving to a new node, even if the user never enabled SponsorBlock. Now only re-applies if categories were previously set.

Tomato6966

I have a left a few comments. first up: Thanks for the bug fixes, please implement my requested changes.

Tomato6966 · 2026-02-14T16:15:55Z

src/structures/Filters.ts

        this.filters.tremolo = false;
        this.filters.vibrato = false;
        this.filters.karaoke = false;
-        this.filters.karaoke = false;


Thank you, i missed that.

Tomato6966 · 2026-02-14T16:16:39Z

src/structures/Filters.ts


        this.filters.nightcore = false;
        this.filters.vaporwave = false;
-        this.data.timescale = { ...DEFAULT_FILTER_DATAS.timescale, speed };


good thing. could you add that it first spreads the deafult data, then the pre-applied data, and then the speed? or just map the 3 vaslues...

Tomato6966 · 2026-02-14T16:16:47Z

src/structures/Filters.ts


        this.filters.nightcore = false;
        this.filters.vaporwave = false;
-        this.data.timescale = { ...DEFAULT_FILTER_DATAS.timescale, pitch };


good thing. could you add that it first spreads the deafult data, then the pre-applied data, and then the speed? or just map the 3 vaslues...

Tomato6966 · 2026-02-14T16:16:53Z

src/structures/Filters.ts


        this.filters.nightcore = false;
        this.filters.vaporwave = false;
-        this.data.timescale = { ...DEFAULT_FILTER_DATAS.timescale, rate };


good thing. could you add that it first spreads the deafult data, then the pre-applied data, and then the speed? or just map the 3 vaslues...

Tomato6966 · 2026-02-14T16:17:45Z

src/structures/Node.ts

            if (this.heartBeatInterval) clearInterval(this.heartBeatInterval);
            if (this.pingTimeout) clearTimeout(this.pingTimeout);
            this.socket?.close(500, "Node-Error - Force Reconnect");
+        } else {


instead of else, do a return in the if () above.

Tomato6966 · 2026-02-14T16:19:19Z

src/structures/Node.ts

            );
            player.set("internal_erroredTracksTimestamps", [...oldTimestamps, Date.now()]);
-            if (oldTimestamps.length > this._LManager.options.playerOptions.maxErrorsPerTime?.maxAmount) {
+            if (oldTimestamps.length >= this._LManager.options.playerOptions.maxErrorsPerTime?.maxAmount) {


Thank you.
since we are comparing old data and not new data >= is correct.

Tomato6966 · 2026-02-14T16:19:41Z

src/structures/Player.ts

                            functionLayer: "Player > changeNode()",
                        });
                    });
-                } else {


Why is this block removed?

the else branch called setSponsorBlock() with no arguments which applies default sponsor-block categories this means when a player changes nodes even if the user never enabled SponsorBlock they'd suddenly get default filtering enabled silently skipping track segments without opting in by removing the else SponsorBlock is only re-applied on the new node if the user had previously set specific categories via setSponsorBlock(categories)

@rly-dev the else block was there so that the ChapterStarted and ChapterLoaded event will work from the SponsorBlock plugin on the newNode (I didn't investigate why this was the case that's why I left that there).

thanks for the context reverted that change in f525b91 — the else block is restored so SponsorBlock events register properly on the new node

i were correct about the surface-level behavior (it does set defaults) but wrong about the intent (it's a necessary side effect for plugin event registration not an accidental opt-in)

also i have added comments on line 953-954 in 30a5118

you can implement an option in the parent function.

"enforceSponsorBlockRequestForEventEnablement"

Added enforceSponsorBlockRequestForEventEnablement as a configurable option in ManagerPlayerOptions (commit b404f71)

if you approve this then i would gladly update in the docs LavalinkManager

Tomato6966 · 2026-02-14T16:20:08Z

src/structures/Queue.ts

            )
                this.previous.splice(
                    0,
-                    override ? this.tracks.length : 0,


Dang, how did i miss that

rly-dev · 2026-02-14T16:23:05Z

sure thing i would do it in few mins

- Filters.ts: spread defaults first, then existing timescale, then new value in setSpeed/setPitch/setRate for guaranteed key coverage. - Node.ts: use early return in closeOnError block instead of if/else.

rly-dev · 2026-02-14T16:35:17Z

refactor updated the code and committed (4e3d867)

The else block is intentional - setSponsorBlock() with defaults is needed so the SponsorBlock plugin registers ChapterStarted/ChapterLoaded event listeners on the new node.

Clarifies that setSponsorBlock() with defaults is needed on node change so the SponsorBlock plugin registers ChapterStarted/ChapterLoaded event listeners on the new node.

Adds a configurable option to control whether setSponsorBlock() with defaults is called on node change for SponsorBlock event registration. Defaults to true to preserve existing behavior. Set to false to disable if you don't use SponsorBlock events.

rly-dev added 4 commits February 14, 2026 16:55

Tomato6966 requested changes Feb 14, 2026

View reviewed changes

refactor: address PR review feedback from Tomato6966

4e3d867

- Filters.ts: spread defaults first, then existing timescale, then new value in setSpeed/setPitch/setRate for guaranteed key coverage. - Node.ts: use early return in closeOnError block instead of if/else.

rly-dev added 4 commits February 14, 2026 22:17

docs: update latest stable version to v2.9.x in README

5c7fd51

revert(Player): restore default SponsorBlock on node change

f525b91

The else block is intentional - setSponsorBlock() with defaults is needed so the SponsorBlock plugin registers ChapterStarted/ChapterLoaded event listeners on the new node.

docs(Player): add comment explaining SponsorBlock else block intent

30a5118

Clarifies that setSponsorBlock() with defaults is needed on node change so the SponsorBlock plugin registers ChapterStarted/ChapterLoaded event listeners on the new node.

Conversation

rly-dev commented Feb 14, 2026

Uh oh!

Tomato6966 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rly-dev commented Feb 14, 2026

Uh oh!

rly-dev commented Feb 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants