Skip to content

Check promise states in conversation timeout callback | timeouts & nested conversations #2407

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

Closed
Bibo-Joshi opened this issue Mar 1, 2021 · 0 comments · Fixed by #2417
Closed

Comments

@Bibo-Joshi
Copy link
Member

When using ConversationHandler with async handler callbacks and timeout, the timeout may trigger falsely. This is the case, when the callback returns CH.END, which currently the timeout callback doesn't know.

This can be solved by making CH._trigger_timeout check if the current conversations state as follows:

if isinstance(state, Promise):
    if state.done and state.result() == CH.END:
        return

This will still fail, if the promise is not yet done, when the timeout triggers but

  1. it's a best effort solution
  2. if a promise takes longer to run than the timeout is, then there are probably some other issues in the users code

Also when looking at the code I noticed that we don't handle nested conversations in CH._trigger_timeout, so that's a good opportunity to fix that …

@Bibo-Joshi Bibo-Joshi added this to the v13.4 milestone Mar 1, 2021
@Bibo-Joshi Bibo-Joshi modified the milestones: v13.4, 13.5 Mar 11, 2021
@Bibo-Joshi Bibo-Joshi removed this from the v13.5 milestone Mar 28, 2021
@github-actions github-actions bot locked and limited conversation to collaborators May 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant