Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@run_async decorator works wrong with converstation timeout #1251

Closed
simonvorobjev opened this issue Oct 11, 2018 · 4 comments
Closed

@run_async decorator works wrong with converstation timeout #1251

simonvorobjev opened this issue Oct 11, 2018 · 4 comments
Labels

Comments

@simonvorobjev
Copy link
Contributor

simonvorobjev commented Oct 11, 2018

Steps to reproduce

from telegram.ext import Updater, CommandHandler, MessageHandler, Filters, ConversationHandler
from telegram.ext.dispatcher import run_async
import epn_parse
import telegram

updater = Updater(token='')

STATE_1, STATE_2 = range(2)

@run_async
def begin(bot, update): 
    bot.send_message(chat_id=update.message.chat_id, text='start!')
    return STATE_1

@run_async
def state_1(bot, update):
    bot.send_message(chat_id=update.message.chat_id, text='end!')
    return ConversationHandler.END

def conversation_timeout(bot, update):
    bot.send_message(chat_id=update.message.chat_id, text='timeout!')

def main():
    dispatcher = updater.dispatcher
    conv_handler = ConversationHandler(
        entry_points=[CommandHandler('begin', begin)],
        states={
            STATE_1: [MessageHandler(Filters.text,
                                           state_1),
                            ],
            ConversationHandler.TIMEOUT: [MessageHandler(Filters.text,
                                                         conversation_timeout),
                                          ]
        },
        fallbacks=[],
        conversation_timeout = 5
    )
    dispatcher.add_handler(conv_handler)
    updater.start_polling(poll_interval = 1.0, timeout=20, clean=True)

main()

Expected behaviour

Timeout job should be deleted in case of ConversationHandler.END and not be triggered

Actual behaviour

In this test after message "end!" message "timeout!" also will be triggered

Configuration

Windows 7

python-telegram-bot 11.1.0
certifi 2018.08.24
future 0.16.0
Python 3.7.0 (v3.7.0:1bf9cc5093, Jun 27 2018, 04:06:47) [MSC v.1914 32 bit (Intel)]

Logs

Simon, [09.10.18 11:04]
/begin

YoutuBot, [09.10.18 11:04]
start!

Simon, [09.10.18 11:04]
test

YoutuBot, [09.10.18 11:04]
end!

YoutuBot, [09.10.18 11:05]
timeout!

Simon, [09.10.18 21:50]
/begin

YoutuBot, [09.10.18 21:50]
begin!

YoutuBot, [09.10.18 21:50]
timeout!

@jsmnbom
Copy link
Member

jsmnbom commented Oct 12, 2018

We expect this to be fixed by a solution to #1250.
Keeping issue open so other users don't report same issue :)

@jh0ker
Copy link
Member

jh0ker commented Oct 15, 2018

@run_async
   bot.send_message(chat_id=update.message.chat_id, text='start!')
   return STATE_1

I believe that is not valid Python code

@simonvorobjev
Copy link
Contributor Author

Edited it, somehow copied it wrong.

@Eldinnie
Copy link
Member

@simonvorobjev It's impossible to stop the async running handler from the timeout job. We've modified a few things more in ConversationHandler, but what it boils down to is that @run_async-callbacks that take a long time in combination with a ConversationHandler is not ideal.

Basically it is what it is, changing it is near impossible (you would have to determine which thread is running the specific callback and terminate that thread, which can't be done when it's blocking).

Hope you can agree that using @run_async decorated callbacks in a ConversationHandler is just a pain we'll have to deal with.

I'm closing this issue for now, but if you want to share some more light on it, feel free to respond and we might re-open

@github-actions github-actions bot locked and limited conversation to collaborators Aug 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants