roydejong / timbot

🤖 Discord bot that announces Twitch channels going live
MIT License
102 stars 38 forks source link

Missing live status buffer #10

Closed Khaztaroth closed 4 years ago

Khaztaroth commented 4 years ago

EDIT: as stated by roydejon bellow, this in fact does nothing different than the new implementation.

I've noticed that the bot sometimes posts multiple messages when someone goes live, by checking older files I saw that there used to be a buffer implemented that was removed when porting to the new API

The code in question is this, which I straight up copy and pasted from the older code and it seems to work fine, I added it to line 158

        if (!notifyFailed) {
            // Notify OK, update list
            this.activeStreams = nextOnlineList;

            if (anyChanges) {
                // Twitch has a weird caching problem where channels seem to quickly alternate between on<->off<->on<->off
                // To avoid spamming, we'll create a buffer time-out whenever this happens
               this.eventBufferStartTime = Date.now();
            }
        } else {
            console.log('[TwitchMonitor]', 'Could not notify channel, will try again next update.');
        }
    }

for it to work I assume this bit is also necessary, which I copy and pasted to line 43

    // Check buffer: are we waiting?
        if (this.eventBufferStartTime) {
            let now = Date.now();
            let timeSinceMs = now - this.eventBufferStartTime;

            if (timeSinceMs >= TwitchMonitor.EVENT_BUFFER_MS) {
                // Buffer is done
                this.eventBufferStartTime = null;
            } else {
                // We're in the buffer zone, do nothing
                return;
            }
        }

I'm not sure if this was supposed to be fixed with some other part of the code, I see no mention of that anywhere, but I have also noticed no issues on your version of the bot that I have on my test server.

roydejong commented 4 years ago

Hey, the buffer was removed because the bot now remembers stream data on disk. To be specific, it links Twitch Stream IDs to Discord Message IDs. This should be enough to prevent "double posting" in most situations.

The time-based buffer was just contributing to stream notifications being delayed, so that's why it was removed.

In most cases, if you're seeing multiple announcements for the same stream, it means the stream was actually restarted causing the API to give us a new ID so the bot is behaving as expected if it creates new post.

It could also happen if multiple instances of the bot are running.

I haven't really seen this problem myself, but a proper long-term solution might be to add a new buffer system that prevents double posting if a new stream is started within a few minutes of the previous one ending.

Khaztaroth commented 4 years ago

I knew there was an actual reason why the code wasn't there anymore, but the bot sometimes posts multiple messages every update without the stream actually going down, and I can tell the difference because if a stream goes down for any reason the bot marks the stream as off in the previous message and then posts a new one.

Just this morning it posted 20 messages for a single stream, without duplicating messages for all the others. I have it deployed through Google Cloud on a VM that runs only that at startup, and a second VM that starts the first one, waits an hour, and then restarts it. For the most part works fine, but on rare occasions, it will continuously post a live message for a stream that just went live.

I'm thinking there might be some sort of de-sying when writing the database. I'm sure the bot is getting pushed a little by getting data from 20+ channels, so on rare occasions will have problems like this. A restart solves the issue and it can run a few days without issue.

If you need any data that I might be able to provide to test segments of the bot I'm happy to help. this tool is making my life a lot easier while trying to manage my community.

roydejong commented 4 years ago

Hey, the recent commit might help with this issue.

Whenever the Discord API returned an error while trying to retrieve an existing message, the bot assumed the message was deleted and would repost. This fix (a5c491a) now checks for the specific error indicating a deleted message before doing so.

Khaztaroth commented 4 years ago

Thank you, I will update my instance and see if the issue persists

roydejong commented 4 years ago

I'll close this for now as it seems to fix the issue.