search

grammyjs / conversations

Conversational interfaces for grammY.
https://grammy.dev/plugins/conversations
MIT License
52 stars 17 forks source link

[Known Issue; v1.0.3 and before] ctx.conversation cannot be used if ctx.skip is called after conversation.waitX #59

Closed KnightNiwrem closed 1 year ago

KnightNiwrem commented 2 years ago

ctx.conversation is mutated when _replayWait is called from some flavor of conversation.waitX method. In the case where this is a non-replaying wait, the original ctx object will have it's ctx.conversation mutated by https://github.com/grammyjs/conversations/blob/v1.0.3/src/conversation.ts#L562.

If a conversation.skip immediately succeeds this, then consequent non-conversation middlewares will encounter an error if they attempt to use some flavor of ctx.conversation.X due to the persistent mutation on the original ctx. This can also happen if the skip is caused by the waitX method itself - e.g. waitUntil.

Minimal reproducible code by Carl: https://hastebin.com/ihidecexoc.js

Madnex commented 1 year ago

I think this is the issue that I am encountering using the conversation plugin in group chats. I would expect that in a group chat multiple people can start the same type of conversation with the bot in parallel.

However, in order to tell the different people apart I need to use waitFrom and then check if the message is the type of message that I am waiting for or otherwise I need to do a conversation.skip(). Alternatively, I can use waitUntil and check for from.id and the message type at the same time but the resulting behavior will be the same:

In both cases this bug is caused. If another user tries to start the same conversation in parallel in the same chat it throws an error: You cannot use `ctx.conversation` from within a conversation!

So in conclusion this means with this current issue it is not possible to have individual parallel conversations of the same type in a group chat, correct?

KnorpelSenf commented 1 year ago

waitFrom already calls skip so usually it just works if you never call skip yourself.

However, this is still a bug and it will have to be fixed.

KnightNiwrem commented 1 year ago

waitFrom already calls skip so usually it just works if you never call skip yourself.

However, this is still a bug and it will have to be fixed.

Hmm.. I'm afraid waitFrom is an indirect call to waitUntil - and since the reproduction demonstrates the bug using waitUntil, waitFrom necessarily suffers from the same issue too.

KnorpelSenf commented 1 year ago

So that means that example at https://grammy.dev/plugins/conversations.html#how-you-can-use-it is also broken? 🤔

KnightNiwrem commented 1 year ago

So that means that example at https://grammy.dev/plugins/conversations.html#how-you-can-use-it is also broken? 🤔

Yes.

Reproduction: https://pastebin.com/diMxi5ef

KnorpelSenf commented 1 year ago

Nasty.

KnorpelSenf commented 1 year ago

This is already fixed on main, isn't it?

KnightNiwrem commented 1 year ago

Until when we decide to add back the ctx.conversation replacers. For now, I'd say main "deletes" the issue. 😛

KnorpelSenf commented 1 year ago

I have played a bit around with how to improve the behaviour even further when someone calls enter from within a conversation. I have figured that no other approach makes any sense, so I guess we'll just leave it as-is for now.

This subsequently means that there are no plans to mess around with ctx.conversation anymore. Thus, this is fixed. Closing.