Fix unresolvable promises#1270
Conversation
|
Good catch, thanks for your contribution! This also seems to be the behavior for when a synchronous entry point handler returns It would also be great if you could add a unit test that tests for this bug, so we won't have any regressions in the future. It should probably look similar to this test. Edit: I don't think the caveat you mentioned is real, pretty sure it is already not possible to use |
jh0ker
left a comment
There was a problem hiding this comment.
Needs a test and documentation
|
@vasinkd is test and docs something you would like to do an attempt at? |
|
Totally forgot about this. @jsmnbom , thanks for a reminder! Ok, I'll add tests, no problemo. |
|
Thanks a lot :D
So a note in the ConversationHandler class docstring about this behaviour (both sync and async) sounds good yea :) |
|
Done. Sorry for the delay! |
|
Test for this instance passed. Can we merge for V12 @jsmnbom ? |
jsmnbom
left a comment
There was a problem hiding this comment.
I see what you mean. Yea let's merge it :)
|
CI is running, should be able to be merged in v12 |
|
Merged V12 after making small adjustment there regarding conversationhandler. If CI agrees we can merge. |
Also include #1270 even though not merged yet, but it should be very soon :)
Situation:
We use ConversationHandler with run_async decorator and an entry_point function returns
NoneResult:
We stuck within conversation state
(None, Promise)since Promise returnsNoneandNonemeans we do not update conversation state.Solution:
If Promise returns
NoneAND old state isNone- exit Conversation Handler.Caveats:
May be not suitable if a programmer uses
Noneas a valid state in Conversation states dict. But why would anyone do that?