search

xmtp / xmtp-js

XMTP client SDKs, content types, and other packages written in TypeScript
https://xmtp.org/docs
211 stars 40 forks source link

Random empty conversations appear sometimes #263

Closed polmaire closed 1 year ago

polmaire commented 1 year ago

Sometimes, for reasons that I ignore, new empty conversations are created on the network with people that I already have conversations with. It seems to be a network issue but I might be wrong.

I can provide examples of conversations if needed.

yash-luna commented 1 year ago

Couple follow ups:

polmaire commented 1 year ago

we're using the walletconnect javascript V2 SDK (people connect via a webview within our RN app).

we're having several difficulties: sometimes walletconnect popup does not show up, sometimes you're redirected to a webview rather than the mobile wallet, sometimes wallets do not "receive" the request we're making, and 100% of the time if we require two signatures in a row it does not work on the mobile wallet

let me know if you need more information!

thank you!

Le mar. 24 janv. 2023 à 22:54, Yash Lunagaria @.***> a écrit :

What SDK version are you using @polmaire https://github.com/polmaire? Can you share details to help us repro?

— Reply to this email directly, view it on GitHub https://github.com/xmtp/xmtp-js/issues/263, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMKP7K4666FUCVSLNBQ7O3WUBFRBANCNFSM6AAAAAAUE2SHLQ . You are receiving this because you were mentioned.Message ID: @.***>

neekolas commented 1 year ago

I've tracked down the origin of the errors to the example-chat-react app.

When a conversation with a conversationId is selected, the AddressInput component is calling client.conversations.newConversation(address) with no conversationId. This has the effect of creating a new V2 conversation with no conversationId in the background. The Inbox app doesn't actually show the conversation, because it removes conversations with no messages, but the conversation still exists and causes issues in other apps.

@bhavya2611 @elisealix22 @daria-github I think the right solution is a bit of a refactor of the AddressInput and RecipientControl. It's weird to me that the AddressInput is trying to create a new conversation at all (newConversation has had the potential to send invites since v7.0), and especially weird that it is doing so on every path change. Not quite sure where to move that logic to, but it feels like it belongs in a higher level component than AddressInput.

neekolas commented 1 year ago

Steps to reproduce:

  1. Go on Lenster and start a new conversation with a user you have not talked to before (either wallet <> wallet or profile <> profile)
  2. Go to the Inbox app and click on that conversation
  3. Check the network tab and you should see a call to /publish
polmaire commented 1 year ago

Thank you Nicholas for these insights - let me know if I can be of any help in the process of solving this issue!

Le jeu. 26 janv. 2023 à 01:17, Nicholas Molnar @.***> a écrit :

Steps to reproduce:

  1. Go on Lenster and start a new conversation with a user you have not talked to before (either wallet <> wallet or profile <> profile)
  2. Go to the Inbox app and click on that conversation
  3. Check the network tab and you should see a call to /publish

— Reply to this email directly, view it on GitHub https://github.com/xmtp/xmtp-js/issues/263#issuecomment-1404391119, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMKP7NXYQ2KZYQVBMDA7KDWUG7A3ANCNFSM6AAAAAAUGUFDDY . You are receiving this because you authored the thread.Message ID: @.***>

polmaire commented 1 year ago

Hello everyone, it seems like @neekolas has an idea of solution for this problem - do you already have an idea on when it can be fixed? Thank you very much in advance!

neekolas commented 1 year ago

We have merged a change to xmtp.chat that should prevent new empty conversations from being created.

For existing conversations, my recommendation would be to hide any chats in your UI that do not have at least 1 message.

polmaire commented 1 year ago

Hey Nicholas, thanks a lot for your answer - I did not know about the change. That's very good news !

About existing chats we'll handle it no worries. Thank you very much

Le mar. 31 janv. 2023 à 18:52, Nicholas Molnar @.***> a écrit :

We have merged a change to xmtp.chat that should prevent new empty conversations from being created.

For existing conversations, my recommendation would be to hide any chats in your UI that do not have at least 1 message.

— Reply to this email directly, view it on GitHub https://github.com/xmtp/xmtp-js/issues/263#issuecomment-1410819758, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMKP7KGYWU5FGAG4N2DZIDWVFGOFANCNFSM6AAAAAAUGUFDDY . You are receiving this because you authored the thread.Message ID: @.***>

neekolas commented 1 year ago

Think we can mark this as closed, and create a new issue if we find any other cases where empty conversations are being created inadvertently