mirror of
https://github.com/matrix-org/matrix-rust-sdk.git
synced 2026-05-08 07:56:55 -04:00
fix: Only report duplicate one-time key errors once
Since the server will reject any duplicate one-time keys forever, clients which encounter such an error will spam sentry with such reports. This patch ensures that we only send the sentry report once.
This commit is contained in:
@@ -73,6 +73,8 @@ pub trait StateStoreIntegrationTests {
|
||||
async fn test_sync_token_saving(&self) -> TestResult;
|
||||
/// Test UtdHookManagerData saving.
|
||||
async fn test_utd_hook_manager_data_saving(&self) -> TestResult;
|
||||
/// Test the saving of the OneTimeKeyAlreadyUploaded key/value data type.
|
||||
async fn test_one_time_key_already_uploaded_data_saving(&self) -> TestResult;
|
||||
/// Test stripped room member saving.
|
||||
async fn test_stripped_member_saving(&self) -> TestResult;
|
||||
/// Test room power levels saving.
|
||||
@@ -582,6 +584,26 @@ impl StateStoreIntegrationTests for DynStateStore {
|
||||
.expect("not UtdHookManagerData");
|
||||
|
||||
assert_eq!(read_data, data);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn test_one_time_key_already_uploaded_data_saving(&self) -> TestResult {
|
||||
// Before any data is written, the getter should return None.
|
||||
assert!(
|
||||
self.get_kv_data(StateStoreDataKey::OneTimeKeyAlreadyUploaded).await?.is_none(),
|
||||
"Store was not empty at start"
|
||||
);
|
||||
|
||||
self.set_kv_data(
|
||||
StateStoreDataKey::OneTimeKeyAlreadyUploaded,
|
||||
StateStoreDataValue::OneTimeKeyAlreadyUploaded,
|
||||
)
|
||||
.await?;
|
||||
|
||||
let data = self.get_kv_data(StateStoreDataKey::OneTimeKeyAlreadyUploaded).await?;
|
||||
data.expect("The loaded data should be Some");
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -1904,6 +1926,12 @@ macro_rules! statestore_integration_tests {
|
||||
store.test_utd_hook_manager_data_saving().await
|
||||
}
|
||||
|
||||
#[async_test]
|
||||
async fn test_one_time_key_already_uploaded_data_saving() -> TestResult {
|
||||
let store = get_store().await?.into_state_store();
|
||||
store.test_one_time_key_already_uploaded_data_saving().await
|
||||
}
|
||||
|
||||
#[async_test]
|
||||
async fn test_stripped_member_saving() -> TestResult {
|
||||
let store = get_store().await?.into_state_store();
|
||||
|
||||
@@ -58,6 +58,7 @@ struct MemoryStoreInner {
|
||||
server_info: Option<ServerInfo>,
|
||||
filters: HashMap<String, String>,
|
||||
utd_hook_manager_data: Option<GrowableBloom>,
|
||||
one_time_key_uploaded_error: bool,
|
||||
account_data: HashMap<GlobalAccountDataEventType, Raw<AnyGlobalAccountDataEvent>>,
|
||||
profiles: HashMap<OwnedRoomId, HashMap<OwnedUserId, MinimalRoomMemberEvent>>,
|
||||
display_names: HashMap<OwnedRoomId, HashMap<DisplayName, BTreeSet<OwnedUserId>>>,
|
||||
@@ -146,6 +147,7 @@ impl StateStore for MemoryStore {
|
||||
|
||||
async fn get_kv_data(&self, key: StateStoreDataKey<'_>) -> Result<Option<StateStoreDataValue>> {
|
||||
let inner = self.inner.read().unwrap();
|
||||
|
||||
Ok(match key {
|
||||
StateStoreDataKey::SyncToken => {
|
||||
inner.sync_token.clone().map(StateStoreDataValue::SyncToken)
|
||||
@@ -167,6 +169,9 @@ impl StateStore for MemoryStore {
|
||||
StateStoreDataKey::UtdHookManagerData => {
|
||||
inner.utd_hook_manager_data.clone().map(StateStoreDataValue::UtdHookManagerData)
|
||||
}
|
||||
StateStoreDataKey::OneTimeKeyAlreadyUploaded => inner
|
||||
.one_time_key_uploaded_error
|
||||
.then_some(StateStoreDataValue::OneTimeKeyAlreadyUploaded),
|
||||
StateStoreDataKey::ComposerDraft(room_id, thread_root) => {
|
||||
let key = (room_id.to_owned(), thread_root.map(ToOwned::to_owned));
|
||||
inner.composer_drafts.get(&key).cloned().map(StateStoreDataValue::ComposerDraft)
|
||||
@@ -217,6 +222,9 @@ impl StateStore for MemoryStore {
|
||||
.expect("Session data not the hook manager data"),
|
||||
);
|
||||
}
|
||||
StateStoreDataKey::OneTimeKeyAlreadyUploaded => {
|
||||
inner.one_time_key_uploaded_error = true;
|
||||
}
|
||||
StateStoreDataKey::ComposerDraft(room_id, thread_root) => {
|
||||
inner.composer_drafts.insert(
|
||||
(room_id.to_owned(), thread_root.map(ToOwned::to_owned)),
|
||||
@@ -256,6 +264,9 @@ impl StateStore for MemoryStore {
|
||||
inner.recently_visited_rooms.remove(user_id);
|
||||
}
|
||||
StateStoreDataKey::UtdHookManagerData => inner.utd_hook_manager_data = None,
|
||||
StateStoreDataKey::OneTimeKeyAlreadyUploaded => {
|
||||
inner.one_time_key_uploaded_error = false
|
||||
}
|
||||
StateStoreDataKey::ComposerDraft(room_id, thread_root) => {
|
||||
let key = (room_id.to_owned(), thread_root.map(ToOwned::to_owned));
|
||||
inner.composer_drafts.remove(&key);
|
||||
|
||||
@@ -1131,6 +1131,10 @@ pub enum StateStoreDataValue {
|
||||
/// `matrix_sdk_ui::unable_to_decrypt_hook::UtdHookManager`.
|
||||
UtdHookManagerData(GrowableBloom),
|
||||
|
||||
/// A unit value telling us that the client uploaded duplicate one-time
|
||||
/// keys.
|
||||
OneTimeKeyAlreadyUploaded,
|
||||
|
||||
/// A composer draft for the room.
|
||||
/// To learn more, see [`ComposerDraft`].
|
||||
///
|
||||
@@ -1234,6 +1238,10 @@ pub enum StateStoreDataKey<'a> {
|
||||
/// `matrix_sdk_ui::unable_to_decrypt_hook::UtdHookManager`.
|
||||
UtdHookManagerData,
|
||||
|
||||
/// Data remembering if the client already reported that it has uploaded
|
||||
/// duplicate one-time keys.
|
||||
OneTimeKeyAlreadyUploaded,
|
||||
|
||||
/// A composer draft for the room.
|
||||
/// To learn more, see [`ComposerDraft`].
|
||||
///
|
||||
@@ -1247,11 +1255,14 @@ pub enum StateStoreDataKey<'a> {
|
||||
impl StateStoreDataKey<'_> {
|
||||
/// Key to use for the [`SyncToken`][Self::SyncToken] variant.
|
||||
pub const SYNC_TOKEN: &'static str = "sync_token";
|
||||
|
||||
/// Key to use for the [`ServerInfo`][Self::ServerInfo]
|
||||
/// variant.
|
||||
pub const SERVER_INFO: &'static str = "server_capabilities"; // Note: this is the old name, kept for backwards compatibility.
|
||||
//
|
||||
/// Key prefix to use for the [`Filter`][Self::Filter] variant.
|
||||
pub const FILTER: &'static str = "filter";
|
||||
|
||||
/// Key prefix to use for the [`UserAvatarUrl`][Self::UserAvatarUrl]
|
||||
/// variant.
|
||||
pub const USER_AVATAR_URL: &'static str = "user_avatar_url";
|
||||
@@ -1264,6 +1275,10 @@ impl StateStoreDataKey<'_> {
|
||||
/// variant.
|
||||
pub const UTD_HOOK_MANAGER_DATA: &'static str = "utd_hook_manager_data";
|
||||
|
||||
/// Key to use for the flag remembering that we already reported that we
|
||||
/// uploaded duplicate one-time keys.
|
||||
pub const ONE_TIME_KEY_ALREADY_UPLOADED: &'static str = "one_time_key_already_uploaded";
|
||||
|
||||
/// Key prefix to use for the [`ComposerDraft`][Self::ComposerDraft]
|
||||
/// variant.
|
||||
pub const COMPOSER_DRAFT: &'static str = "composer_draft";
|
||||
|
||||
@@ -422,6 +422,9 @@ impl IndexeddbStateStore {
|
||||
StateStoreDataKey::UtdHookManagerData => {
|
||||
self.encode_key(keys::KV, StateStoreDataKey::UTD_HOOK_MANAGER_DATA)
|
||||
}
|
||||
StateStoreDataKey::OneTimeKeyAlreadyUploaded => {
|
||||
self.encode_key(keys::KV, StateStoreDataKey::ONE_TIME_KEY_ALREADY_UPLOADED)
|
||||
}
|
||||
StateStoreDataKey::ComposerDraft(room_id, thread_root) => {
|
||||
if let Some(thread_root) = thread_root {
|
||||
self.encode_key(
|
||||
@@ -563,6 +566,10 @@ impl_state_store!({
|
||||
.map(|f| self.deserialize_value::<GrowableBloom>(&f))
|
||||
.transpose()?
|
||||
.map(StateStoreDataValue::UtdHookManagerData),
|
||||
StateStoreDataKey::OneTimeKeyAlreadyUploaded => value
|
||||
.map(|f| self.deserialize_value::<bool>(&f))
|
||||
.transpose()?
|
||||
.map(|_| StateStoreDataValue::OneTimeKeyAlreadyUploaded),
|
||||
StateStoreDataKey::ComposerDraft(_, _) => value
|
||||
.map(|f| self.deserialize_value::<ComposerDraft>(&f))
|
||||
.transpose()?
|
||||
@@ -603,6 +610,7 @@ impl_state_store!({
|
||||
StateStoreDataKey::UtdHookManagerData => self.serialize_value(
|
||||
&value.into_utd_hook_manager_data().expect("Session data not UtdHookManagerData"),
|
||||
),
|
||||
StateStoreDataKey::OneTimeKeyAlreadyUploaded => self.serialize_value(&true),
|
||||
StateStoreDataKey::ComposerDraft(_, _) => self.serialize_value(
|
||||
&value.into_composer_draft().expect("Session data not a composer draft"),
|
||||
),
|
||||
|
||||
@@ -387,6 +387,9 @@ impl SqliteStateStore {
|
||||
StateStoreDataKey::UtdHookManagerData => {
|
||||
Cow::Borrowed(StateStoreDataKey::UTD_HOOK_MANAGER_DATA)
|
||||
}
|
||||
StateStoreDataKey::OneTimeKeyAlreadyUploaded => {
|
||||
Cow::Borrowed(StateStoreDataKey::ONE_TIME_KEY_ALREADY_UPLOADED)
|
||||
}
|
||||
StateStoreDataKey::ComposerDraft(room_id, thread_root) => {
|
||||
if let Some(thread_root) = thread_root {
|
||||
Cow::Owned(format!(
|
||||
@@ -1012,6 +1015,9 @@ impl StateStore for SqliteStateStore {
|
||||
StateStoreDataKey::UtdHookManagerData => {
|
||||
StateStoreDataValue::UtdHookManagerData(self.deserialize_value(&data)?)
|
||||
}
|
||||
StateStoreDataKey::OneTimeKeyAlreadyUploaded => {
|
||||
StateStoreDataValue::OneTimeKeyAlreadyUploaded
|
||||
}
|
||||
StateStoreDataKey::ComposerDraft(_, _) => {
|
||||
StateStoreDataValue::ComposerDraft(self.deserialize_value(&data)?)
|
||||
}
|
||||
@@ -1047,6 +1053,9 @@ impl StateStore for SqliteStateStore {
|
||||
StateStoreDataKey::UtdHookManagerData => self.serialize_value(
|
||||
&value.into_utd_hook_manager_data().expect("Session data not UtdHookManagerData"),
|
||||
)?,
|
||||
StateStoreDataKey::OneTimeKeyAlreadyUploaded => {
|
||||
self.serialize_value(&true).expect("We should be able to serialize a boolean")
|
||||
}
|
||||
StateStoreDataKey::ComposerDraft(_, _) => self.serialize_value(
|
||||
&value.into_composer_draft().expect("Session data not a composer draft"),
|
||||
)?,
|
||||
|
||||
@@ -34,12 +34,15 @@ use futures_util::{
|
||||
};
|
||||
#[cfg(feature = "experimental-send-custom-to-device")]
|
||||
use matrix_sdk_base::crypto::CollectStrategy;
|
||||
use matrix_sdk_base::crypto::{
|
||||
store::types::{RoomKeyBundleInfo, RoomKeyInfo},
|
||||
types::requests::{
|
||||
OutgoingRequest, OutgoingVerificationRequest, RoomMessageRequest, ToDeviceRequest,
|
||||
use matrix_sdk_base::{
|
||||
crypto::{
|
||||
store::types::{RoomKeyBundleInfo, RoomKeyInfo},
|
||||
types::requests::{
|
||||
OutgoingRequest, OutgoingVerificationRequest, RoomMessageRequest, ToDeviceRequest,
|
||||
},
|
||||
CrossSigningBootstrapRequests, OlmMachine,
|
||||
},
|
||||
CrossSigningBootstrapRequests, OlmMachine,
|
||||
StateStoreDataKey, StateStoreDataValue,
|
||||
};
|
||||
use matrix_sdk_common::{executor::spawn, locks::Mutex as StdMutex};
|
||||
use ruma::{
|
||||
@@ -623,7 +626,9 @@ impl Client {
|
||||
self.keys_query(r.request_id(), request.device_keys.clone()).await?;
|
||||
}
|
||||
AnyOutgoingRequest::KeysUpload(request) => {
|
||||
self.keys_upload(r.request_id(), request).await.inspect_err(|e| {
|
||||
let response = self.keys_upload(r.request_id(), request).await;
|
||||
|
||||
if let Err(e) = &response {
|
||||
match e.as_ruma_api_error() {
|
||||
Some(RumaApiError::ClientApi(e)) if e.status_code == 400 => {
|
||||
if let ErrorBody::Standard { message, .. } = &e.body {
|
||||
@@ -631,18 +636,35 @@ impl Client {
|
||||
// telling us that we already have a one-time key uploaded means
|
||||
// that we forgot about some of our one-time keys. This will lead to
|
||||
// UTDs.
|
||||
if message.starts_with("One time key") {
|
||||
tracing::error!(
|
||||
sentry = true,
|
||||
error_message = message,
|
||||
"Duplicate one-time keys have been uploaded"
|
||||
);
|
||||
{
|
||||
let already_reported = self
|
||||
.state_store()
|
||||
.get_kv_data(StateStoreDataKey::OneTimeKeyAlreadyUploaded)
|
||||
.await?
|
||||
.is_some();
|
||||
|
||||
if message.starts_with("One time key") && !already_reported {
|
||||
tracing::error!(
|
||||
sentry = true,
|
||||
error_message = message,
|
||||
"Duplicate one-time keys have been uploaded"
|
||||
);
|
||||
|
||||
self.state_store()
|
||||
.set_kv_data(
|
||||
StateStoreDataKey::OneTimeKeyAlreadyUploaded,
|
||||
StateStoreDataValue::OneTimeKeyAlreadyUploaded,
|
||||
)
|
||||
.await?;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
})?;
|
||||
|
||||
response?;
|
||||
}
|
||||
}
|
||||
AnyOutgoingRequest::ToDeviceRequest(request) => {
|
||||
let response = self.send_to_device(request).await?;
|
||||
|
||||
Reference in New Issue
Block a user