Highlighting superchats doesn't work sometimes

[cohlexyz]

I'm not sure what the last minute fix was in the last stream, but with the current state my assumption is that super chats cannot be featured because they happened too long ago:

https://github.com/jaw-sh/stream-nexus/blob/33a197159b8a30e61cc99d50d0c074634ac73516/public/script.js#L120-L127

From what I can tell the highlight system works by sending the uuid of the message to be highlighted from the dashboard to the overlay. However only the last 1000 messages are kept so super chats from the start of the stream are no longer present in the overlay.

A stupid fix would be to just not delete them:

    // remove first old message that is not sticky and not a superchat
    while (chat_history.children.length > 1000) {
        for (let i = 0; i < chat_history.children.length; i++) {
            if (!chat_history.childNodes[i].classList.contains("msg--sticky") && !chat_history.childNodes[i].classList.contains("msg--t")) {
                chat_history.childNodes[i].remove();
                break;
            }
        }
    }

https://github.com/user-attachments/assets/474c9548-7d2c-4155-965d-827cef23e312

[cohlexyz]

Seems to work as of the latest stream.

[cohlexyz]

Well, I guess it wasn't completely fixed since it didn't work last stream. @jaw-sh is this repo up to date with your local changes? I'm not sure what fixes you've tried so far but maybe you want to give the change I suggested in this issue a try if it's not something you've already tried.

[jaw-sh]

the only thing i can think of is the ids are being removed from the overlay and I do not know why that would happen unless I somehow refreshed the overlay without thinking. i should probably change the system to just pass the entire HTML instead of, or in addition to, IDs.

[cohlexyz]

So I guess you already tried what I suggested. Strange, since it worked in my tests.

[y-a-t-s]

handle_message is called by an EventListener, so the call is async. Perhaps what we're seeing here is a data race.

[jaw-sh]

It's definitely an issue with the information simply not existing. It worked great today.

[y-a-t-s]

In https://github.com/jaw-sh/stream-nexus/blob/33a197159b8a30e61cc99d50d0c074634ac73516/public/script.js#L120-L127 we see the old message node is removed by referencing the list of children using an array index. If this array changes because of another async call, and a premium message now fills that array slot, it will get discarded. Race condition.

[jaw-sh]

in local commits that code also checks for msg--t.

[y-a-t-s]

Regardless, we can't trust async processes to manage a single array without any sort of locking or safety setup. I think this perfectly explains why it sometimes works and sometimes doesn't.

[jaw-sh]

????? there is no race condition, these messages are hours old bro

[y-a-t-s]

childNodes returns a live NodeList, which reflects any changes made by other async calls. Any time the EventListener calls handle_message, it will do so asynchronously and will be messing with any other active calls to handle_message. If one call removes a message node, the other calls can no longer be sure of the state of childNodes's data.

While the if condition that checks for the classes may be true for childNodes[i] at the time of evaluation, it may no longer be at the time of the remove() call. So while index i may not point to a superchat when the if is evaluated, it may end up pointing to a superchat when remove() gets called on it, all due to modifications to childNodes made by another thread. Thus, superchats get wiped off the screen early.

Correct me if I'm wrong, but this sounds like the issue.