hschickdevs / Telegram-Crypto-Alerts

The most popular open-source cryptocurrency alerting tool for Telegram! Providing alerts for simple price movements from Binance and technical indicators from Taapi.io.
MIT License
67 stars 13 forks source link

Alerts being overwritten when POLLING_PERIOD set to lower number #17

Closed kristapsm closed 1 year ago

kristapsm commented 1 year ago

Hello! First of all, amazing telegram bot, love it!

When testing around I came up to an issue when changing POLLING_PERIOD to something like 2, 3 or even 4 seconds, /newalert command did not save my alert even though there is a message Successfully activated new alert!

By trying to debug it looks like the issue is with changing POLLING_PERIOD to some lower number where it frequently polls existing user alerts using poll_user_alerts from poll_all_alerts where it updates the alerts.json file all the time on this line alert_handler.py#L64

Therefore whenever calling /newalert on the same time when it could interrupt with poll_user_alerts, the alerts.json might get overwritten by the existing alerts.json content and not the new one. I believe this might also happen using default 10 second POLLING_PERIOD, but its much more noticeable when POLLING_PERIOD is set to 2 or 3 seconds where I couldnt set more than 4 new alerts.

Reason why I changed POLLING_PERIOD was to get the alert almost immediately when the price target is hit and not like 10 seconds later. Maybe there is another way I can achieve this?

hschickdevs commented 1 year ago

@kristapsm, thanks for the kind words!

I appreciate you bringing attention to this. Always love having people engage with my projects!

Despite extensive testing on a Singapore VPS, I could not reproduce this issue, even when setting the polling period to 1 second. I tested with both technical and price indicators.

Quite honestly, it may not be optimal to be using a local database for this project, and instead a cloud database somewhere. I plan to re-engineer this component down the line 🤷‍♂️. It is very likely that using a local database could be causing conflicts in one way or another.

Anyhow, I wouldn't advise going below a polling period of 5 seconds, since you may run into some issues with the Binance API depending on how many pairs you've set alerts for. You may want to check their exact rate limits and calculate your current calls per minute like so: Outbound Binance API Calls per Minute = (pairs_with_alerts * polling_period) * (60 / polling period)

Also, please pull the newest version of the bot, as I've made an important but simple bugfix that could've also been causing this. Please let me know if this fixed your issue!

https://github.com/hschickdevs/Telegram-Crypto-Alerts/commit/5916131098f9c6dd7047ed2b3363f892b9a54bbf

kristapsm commented 1 year ago

@hschickdevs I was still experiencing issues even setting polling period to 4 seconds, I would see message that new alert has been successfully added and then using /viewalerts at First time the alert would be there, but next times it wont which made me think that it somehow still gets overwritten. Then I made some changes on alert_handler.py for poll_user_alerts function with needs_update variable where the JSON DB file gets updated only if the alert has been triggered instead of updating all the time. There must be a better way to handle this I believe as by this approach it will still update JSON like two or three times in a row. But this lets me add alerts without any interruptions

def poll_user_alerts(self, tg_user_id: str) -> None:
        """
        1. Load the user's configuration
        2. poll all alerts and create posts
        3. Remove alert conditions
        4. Send alerts if found

        :param tg_user_id: The Telegram user ID from the database
        """
        configuration = UserConfiguration(tg_user_id)
        alerts_database = configuration.load_alerts()
        config = configuration.load_config()
        needs_update = False          <----- ADDED THIS
        post_queue = []
        for pair in alerts_database.copy().keys():

            remove_queue = []
            for alert in alerts_database[pair]:
                if alert['alerted']:
                    remove_queue.append(alert)
                    continue

                if alert['type'] == "s":
                    condition, value, post_string = self.get_simple_indicator(pair, alert)
                elif alert['type'] == "t":
                    condition, value, post_string = self.get_technical_indicator(pair, alert)
                else:
                    raise Exception("Invalid alert type: s = simple, t = technical")

                if condition:  # If there is a condition satisfied
                    post_queue.append(post_string)
                    alert['alerted'] = True
                    needs_update = True          <----- ADDED THIS

            for item in remove_queue:
                alerts_database[pair].remove(item)
                needs_update = True              <----- ADDED THIS
                if len(alerts_database[pair]) == 0:
                    alerts_database.pop(pair)
                    needs_update = True          <----- ADDED THIS

        if needs_update == True:                <----- ADDED THIS
            logger.info(f'Updating alerts...') <----- ADDED THIS
            configuration.update_alerts(alerts_database)
            needs_update = False                 <----- ADDED THIS

        if len(post_queue) > 0:
            print(len(post_queue))
            self.polling = False
            for post in post_queue:
                logger.info(post)
                status = self.tg_alert(post=post, channel_ids=config['channels'])
                if len(status[1]) > 0:
                    logger.warn(f"Failed to send Telegram alert ({post}) to the following IDs: {status[1]}")

                if config['settings']['send_email_alerts']:
                    self.email_alert_sendgrid(post, configuration)

        if not self.polling:
            self.polling = True
            logger.info(f'Bot polling for next alert...')
hschickdevs commented 1 year ago

Hey @kristapsm, thanks so much for your careful analysis and contribution!

I've added this patch to the main code using a do_update variable similar to yours. Let me know if the issue is resolved!

Commit reference: #19

hschickdevs commented 1 year ago

Problem likely solved - closing this issue due to inactivity.