Conversation
Signed-off-by: starry69 <starry369126@outlook.com>
Signed-off-by: starry69 <starry369126@outlook.com>
|
BTW, #2466 gave me the idea that we could do a warning when a Here IG that I'd also go with |
+1, will be easier to debug CH! |
|
Aaaand another warning we could add: if in CH |
When returning state that is unknown to conversation handler, the conversation gets stucked as long as user cancels it manually (via fallback) or until timeout gets triggered, shouldn't we also close the conversation after warning like: "Conversation handler returned unknown state, closing the current conversation..." Instead of just warning |
Mh, I don't think that's a good idea. Firstly, there's a chance we might close falsely in some edge cases. Also I think that it's rather surprising behaviour for CH to close conversation on its own and makes debugging even harder. And finally it would be somewhat breaking… |
Oh right, makes sense. |
Signed-off-by: starry69 <starry369126@outlook.com>
telegram/ext/conversationhandler.py
Outdated
| elif new_state is not None: | ||
| if new_state not in self.states: | ||
| warnings.warn( | ||
| "Handler returned state which is unknown to the ConversationHandler." |
There was a problem hiding this comment.
Just an idea: If the hanlder has a name, we could include that in the message. Makes debugging easier :) And maybe including the unknown state is useful, too …
Signed-off-by: starry69 <starry369126@outlook.com>
telegram/ext/conversationhandler.py
Outdated
| warnings.warn( | ||
| "Handler returned state which is unknown to the ConversationHandler." | ||
| f"Handler returned state {new_state} which is unknown to the " | ||
| f"ConversationHandler {self.name if self.name is not None else ''}." |
There was a problem hiding this comment.
Just one very minor nitpick left: when the CH has no name, there will be a trailing whitespace. We can do better, even in warnings :D
After this, I promise I'll merge! 😉
Signed-off-by: starry69 <starry369126@outlook.com>
|
Wen can ignore deepsource until I'm done with #2454. Merging |
No description provided.