Fix timeout errors and add more fields to GroupWrapper #470
Conversation
… noe/rn-thread-blocked
… noe/rn-thread-blocked
nplasterer
changed the title
|
This PR now Fixes ephemeraHQ/converse-app#422 and Fixes xmtp/libxmtp#955 |
| @@ -13,11 +13,12 @@ class GroupWrapper { | |||
| "clientAddress" to client.address, | |||
| "id" to group.id, | |||
| "createdAt" to group.createdAt.time, | |||
| "peerInboxIds" to group.peerInboxIds(), | |||
| "members" to group.members().map { MemberWrapper.encode(it) }, | |||
There was a problem hiding this comment.
I didn't want to duplicate the DB call to members so this is a breaking change for anyone who is using peerInboxIds directly but is easily mitigated by mapping on inboxID if desired.
| } | ||
|
|
||
| return true | ||
| }) |
There was a problem hiding this comment.
Only left this one test that was previously failing for me and passing now.
| async membersList(): Promise<Member[]> { | ||
| return await XMTP.listGroupMembers(this.client.inboxId, this.id) | ||
| } |
There was a problem hiding this comment.
Instead of removing this method renamed it. Since typescript wouldn't allow the name collision. It's maybe not necessary?
There was a problem hiding this comment.
@nplasterer the method is still useful
If we call group.sync() , I don't think the value of the attribute group.members will be updated in place? So being able to call group.membersList() to get the refreshed value is useful?
| const wallet1 = Wallet.createRandom() | ||
| const wallet2 = Wallet.createRandom() |
There was a problem hiding this comment.
These wallets are in the bad key package state so had to use random ones. I think this is better anyways. But good to know for future testing when we have a fix.
|
|
🎉 This PR is included in version 2.4.5 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Exploring the RN thread getting completely locked sometimes, I managed to create 3 reproducible tests:
These lock last 30 seconds (I guess that's the duration of the db lock?)
During these 30 seconds, no other call to the SDK resolves (even calls to the SDK that don't rely on the database, like
canMessagev2 version - the call just does not reach the native code)Also in Converse it happens in a loop because during the sync process
There is an increasing backoff for retries but it explains why it looks like the app is "bricked" forever, as soon as it gets unlocked, it goes into a lock again.
We should probably understand why these calls cause a timeout and fix it (creating 10 groups should be possible, even if we need to wait a bit for the pool, here it seems we never get back the pool connection and it times out)
We should also understand why if one async call to the sdk is locking, other calls to the sdk, even not using the database, lock as well => should a pool connection awaitance cause a thread lock?