Retry PoolNeedsConnection Errors #1010
Conversation
| @@ -68,6 +68,7 @@ impl RetryableError for StorageError { | |||
| Self::Pool(_) => true, | |||
| Self::Lock(_) => true, | |||
| Self::SqlCipherNotLoaded => true, | |||
| Self::PoolNeedsConnection => true, | |||
There was a problem hiding this comment.
This means the error will retry 5 times before erroring. That might be okay? I also took a pass at just doing it in the code and retrying 3 times here 45adbfe open to others thoughts on how we should handle this.
There was a problem hiding this comment.
This definitely feels like it should be retryable. Thanks for fixing
|
Just reposting here for visibility : Was just thinking about this a little During these sessions, is the XMTP client still in memory/we're not re-instantiating the struct? Instead of retrying, which may still fail depending on the threading situation -- sortof feels like we're just waiting for a race which reconnects the db in another thread -- we could instead add a flag on the struct and a boolean passed to also, this retry would only work if the reconnect is already in a codepath that is in a |
Hey @insipx , I'm not 100% sure I get what you mean, but one thing we DON'T want is for libxmtp tu auto-re-establish a database connection. Because libxmtp has no idea if the app is active or backgrounded, and on iOS opening a connection to the libxmtp db when the app gets suspended will lead to a crash |
xmtp_mls/src/groups/mod.rs
Outdated
| @@ -187,6 +187,8 @@ pub enum GroupError { | |||
| SqlKeyStore(#[from] sql_key_store::SqlKeyStoreError), | |||
| #[error("No pending commit found")] | |||
| MissingPendingCommit, | |||
| #[error("Sync failed to wait for intent: {0}")] | |||
| SyncFailedToWait(String), | |||
There was a problem hiding this comment.
@neekolas adding this so it's easier to catch the error that is causing group creates to fail.
|
|
||
| let current_state = retry_async!( | ||
| Retry::default(), | ||
| (async { | ||
| self.get_association_state(&self.store().conn()?, &inbox_id, None) | ||
| .await | ||
| }) | ||
| )?; |
There was a problem hiding this comment.
This should cover the case converse is hitting when the database is trying to reconnect after getting a signature from a different app for revoking installations.
…disconnect-errors
Makes
PoolNeedsConnectionErrors retryable for actions that require backgrounding the app while doing an action like receiving a signature on adding and revoking installations and applying a signature request.Also adds a new error type that can be caught for the group create issue.
Fixes #996