Fix bug #1103 in setting TELEGRAM_WEBHOOK_HOST causing Flood control exceeded error

[alexintech]

What this PR does

When TELEGRAM_WEBHOOK_HOST sets into database it calls Telegram's setWebhook twice. The first call on field validation goes correctly. But after saving the field LiveSettings.validate_settings() runs, which validates all settings. It calls Telegram's setWebhook again and in that case receives Flood control exceeded error from Telegram.

I have found that LiveSettings.validate_settings has been invented in commit ef37e6a like a replacement for twilio dependent settings.

This PR revalidates only updated dependent settings. So it does not flood Telegram API.

Which issue(s) this PR fixes

#1103

Checklist

• Unit, integration, and e2e (if applicable) tests updated
• Documentation added (or pr:no public docs PR label added if not required)
• CHANGELOG.md updated (or pr:no changelog PR label added if not required)

[vstpme]

Hi @alexintech 👋 Thank you for opening a PR!
Looks like this change disables live setting validation for everything except Twilio, and I'm not sure this would be a good idea. Please see LiveSettingsValidator for more info on live settings validation.

Have you considered removing the second call to setWebhook here? The first call to setWebhook is performed on serializer save, so the second call indeed seems to be redundant. Let me know if this makes sense 🙂

[alexintech]

Hi @alexintech 👋 Thank you for opening a PR! Looks like this change disables live setting validation for everything except Twilio, and I'm not sure this would be a good idea. Please see LiveSettingsValidator for more info on live settings validation.

Have you considered removing the second call to setWebhook here? The first call to setWebhook is performed on serializer save, so the second call indeed seems to be redundant. Let me know if this makes sense 🙂

Hi, @vadimkerr! Thank you for your attention.

Actually, there are three possible calls to the setWebhook API when updating a live setting.

First case (valid webhook URL entered):

1. The first call to setWebhook is performed on serializer save as you mentioned. It sets a valid URL to Telegram and clears error field if one exists in the database.
2. The second call here won't actually call the setWebhook endpoint because it checks here if the webhook URL has already been set.
3. The second actual call to setWebhook is performed in LiveSettings.validate_settings(). This call results in a Flood control exceeded error, and this error is saved to the database.

Second case (invalid webhook URL entered, for example, https://123.ngrok):

1. The first call to setWebhook is performed on serializer save. Telegram responds with the error Bad Request: bad webhook: Failed to resolve host: Name or service not known and unsets the webhook URL if one was set before. This error is saved to the database.
2. The second call to setWebhook happens here after the webhook URL has been unset, and it sends a real request to Telegram. However, the result of this call does not change the error field in the database.
3. The third call to the setWebhook endpoint happens in LiveSettings.validate_settings(). This call results in a Flood control exceeded error, which overrides the more informative error message from the first step. This means that the actual cause of the error, the invalid webhook URL, is not saved in the database. Instead, the database will contain the Flood control exceeded error, which is not helpful in identifying and resolving the underlying issue.

I have considered removing these lines:

1. In the first case it doesn't change anything;
2. In the second case it also doesn't change anything.

Another option I have tried is modifing _check_telegram_webhook_host() adding validation for existing url:

            url = create_engine_url("/telegram/", override_base=telegram_webhook_host)
            bot = Bot(token=live_settings.TELEGRAM_TOKEN)

            webhook_info = bot.get_webhook_info()
            if webhook_info.url == url:
                return

            bot.set_webhook(url)

1. It actually resolves the issue in the first case;
2. However, in the second case, it shows the same Flood control exceeded error instead of the more informative Bad Request: bad webhook: Failed to resolve host: Name or service not known error. This happens even with the removal of these lines

In LiveSettingsValidator I see only Twilio settings use multiple settings to execute validation. Currently when I change a setting value, it validates one setting, and then runs validations for all existing settings.

When I update one setting value, basically I'm excepting that only that field and other dependent on it would be validated.
But if you really need to revalidate all settings, currently you need to change some value back and forth. Maybe for that case it would be better to have an extra button in UI Revalidate all settings? I'm not sure if revalidation of all live settings is required after every change.

[alexintech]

@vadimkerr @mderynck any ideas? 😄

[vstpme]

@alexintech please see #2100, multiple calls to live setting validation should be fixed there

[vstpme]

The issue should be fixed in #2100, so I'm closing this PR. @alexintech thank you for the contribution!