Skip to content

Commit 698a914

Browse files
EldinnieEldinnie
authored
Fix conversationhandler timeout (python-telegram-bot#1032)
* Fix conversationhandler As found by @nmlorg and described in python-telegram-bot#1031 closes python-telegram-bot#1031 * adding another test and definitely finish conversationhandler It seems another problem was when the job executed the timeout, it wasn;t removed from `self.conversation_timeouts` which made it still fail because job would be present in the handler dict, although it was already disabled. This should fix it properly.
1 parent 5956aae commit 698a914

File tree

2 files changed

+64
-6
lines changed

2 files changed

+64
-6
lines changed

‎telegram/ext/conversationhandler.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -303,11 +303,10 @@ def handle_update(self, update, dispatcher):
303303
304304
"""
305305
new_state = self.current_handler.handle_update(update, dispatcher)
306-
timeout_job = self.timeout_jobs.get(self.current_conversation)
306+
timeout_job = self.timeout_jobs.pop(self.current_conversation, None)
307307

308-
if timeout_job is not None or new_state == self.END:
308+
if timeout_job is not None:
309309
timeout_job.schedule_removal()
310-
del self.timeout_jobs[self.current_conversation]
311310
if self.conversation_timeout and new_state != self.END:
312311
self.timeout_jobs[self.current_conversation] = dispatcher.job_queue.run_once(
313312
self._trigger_timeout, self.conversation_timeout,
@@ -330,4 +329,5 @@ def update_state(self, new_state, key):
330329
self.conversations[key] = new_state
331330

332331
def _trigger_timeout(self, bot, job):
332+
del self.timeout_jobs[job.context]
333333
self.update_state(self.END, job.context)

‎tests/test_conversationhandler.py‎

Lines changed: 61 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#
1717
# You should have received a copy of the GNU Lesser Public License
1818
# along with this program. If not, see [http://www.gnu.org/licenses/].
19+
import logging
1920
from time import sleep
2021

2122
import pytest
@@ -55,7 +56,8 @@ def reset(self):
5556
self.BREWING: [CommandHandler('pourCoffee', self.drink)],
5657
self.DRINKING:
5758
[CommandHandler('startCoding', self.code),
58-
CommandHandler('drinkMore', self.drink)],
59+
CommandHandler('drinkMore', self.drink),
60+
CommandHandler('end', self.end)],
5961
self.CODING: [
6062
CommandHandler('keepCoding', self.code),
6163
CommandHandler('gettingThirsty', self.start),
@@ -73,6 +75,9 @@ def _set_state(self, update, state):
7375
def start(self, bot, update):
7476
return self._set_state(update, self.THIRSTY)
7577

78+
def end(self, bot, update):
79+
return self._set_state(update, self.END)
80+
7681
def start_end(self, bot, update):
7782
return self._set_state(update, self.END)
7883

@@ -123,6 +128,25 @@ def test_conversation_handler(self, dp, bot, user1, user2):
123128
with pytest.raises(KeyError):
124129
self.current_state[user2.id]
125130

131+
def test_conversation_handler_end(self, caplog, dp, bot, user1):
132+
handler = ConversationHandler(entry_points=self.entry_points, states=self.states,
133+
fallbacks=self.fallbacks)
134+
dp.add_handler(handler)
135+
136+
message = Message(0, user1, None, self.group, text='/start', bot=bot)
137+
dp.process_update(Update(update_id=0, message=message))
138+
message.text = '/brew'
139+
dp.process_update(Update(update_id=0, message=message))
140+
message.text = '/pourCoffee'
141+
dp.process_update(Update(update_id=0, message=message))
142+
message.text = '/end'
143+
with caplog.at_level(logging.ERROR):
144+
dp.process_update(Update(update_id=0, message=message))
145+
assert len(caplog.records) == 0
146+
assert self.current_state[user1.id] == self.END
147+
with pytest.raises(KeyError):
148+
print(handler.conversations[(self.group.id, user1.id)])
149+
126150
def test_conversation_handler_fallback(self, dp, bot, user1, user2):
127151
handler = ConversationHandler(entry_points=self.entry_points, states=self.states,
128152
fallbacks=self.fallbacks)
@@ -309,16 +333,50 @@ def test_conversation_timeout(self, dp, bot, user1):
309333
assert handler.conversations.get((self.group.id, user1.id)) is None
310334

311335
# Start state machine, do something, then reach timeout
312-
dp.process_update(Update(update_id=0, message=message))
336+
dp.process_update(Update(update_id=1, message=message))
313337
assert handler.conversations.get((self.group.id, user1.id)) == self.THIRSTY
314338
message.text = '/brew'
315339
dp.job_queue.tick()
316-
dp.process_update(Update(update_id=0, message=message))
340+
dp.process_update(Update(update_id=2, message=message))
317341
assert handler.conversations.get((self.group.id, user1.id)) == self.BREWING
318342
sleep(0.5)
319343
dp.job_queue.tick()
320344
assert handler.conversations.get((self.group.id, user1.id)) is None
321345

346+
def test_conversation_timeout_keeps_extending(self, dp, bot, user1):
347+
handler = ConversationHandler(entry_points=self.entry_points, states=self.states,
348+
fallbacks=self.fallbacks, conversation_timeout=0.5)
349+
dp.add_handler(handler)
350+
351+
# Start state machine, wait, do something, verify the timeout is extended.
352+
# t=0 /start (timeout=.5)
353+
# t=.25 /brew (timeout=.75)
354+
# t=.5 original timeout
355+
# t=.6 /pourCoffee (timeout=1.1)
356+
# t=.75 second timeout
357+
# t=1.1 actual timeout
358+
message = Message(0, user1, None, self.group, text='/start', bot=bot)
359+
dp.process_update(Update(update_id=0, message=message))
360+
assert handler.conversations.get((self.group.id, user1.id)) == self.THIRSTY
361+
sleep(0.25) # t=.25
362+
dp.job_queue.tick()
363+
assert handler.conversations.get((self.group.id, user1.id)) == self.THIRSTY
364+
message.text = '/brew'
365+
dp.process_update(Update(update_id=0, message=message))
366+
assert handler.conversations.get((self.group.id, user1.id)) == self.BREWING
367+
sleep(0.35) # t=.6
368+
dp.job_queue.tick()
369+
assert handler.conversations.get((self.group.id, user1.id)) == self.BREWING
370+
message.text = '/pourCoffee'
371+
dp.process_update(Update(update_id=0, message=message))
372+
assert handler.conversations.get((self.group.id, user1.id)) == self.DRINKING
373+
sleep(.4) # t=1
374+
dp.job_queue.tick()
375+
assert handler.conversations.get((self.group.id, user1.id)) == self.DRINKING
376+
sleep(.1) # t=1.1
377+
dp.job_queue.tick()
378+
assert handler.conversations.get((self.group.id, user1.id)) is None
379+
322380
def test_conversation_timeout_two_users(self, dp, bot, user1, user2):
323381
handler = ConversationHandler(entry_points=self.entry_points, states=self.states,
324382
fallbacks=self.fallbacks, conversation_timeout=0.5)

0 commit comments

Comments
 (0)