From a6aabaf9e2889991229f17ba72ac9a0149628693 Mon Sep 17 00:00:00 2001 From: Nicolas Mauri Date: Wed, 25 Jan 2023 14:15:00 +0100 Subject: [PATCH 1/5] Fix a deadlock when updating the summary of a room that has a voice broadcast --- Riot/Utils/EventFormatter.m | 21 ++++----------------- 1 file changed, 4 insertions(+), 17 deletions(-) diff --git a/Riot/Utils/EventFormatter.m b/Riot/Utils/EventFormatter.m index 21cd496dd..db526607a 100644 --- a/Riot/Utils/EventFormatter.m +++ b/Riot/Utils/EventFormatter.m @@ -567,7 +567,8 @@ static NSString *const kEventFormatterTimeFormat = @"HH:mm"; return [self session:session updateRoomSummary:summary withVoiceBroadcastInfoStateEvent:lastVoiceBroadcastInfoEvent - voiceBroadcastInfoStartedEvent:voiceBroadcastInfoStartedEvent roomState:roomState]; + voiceBroadcastInfoStartedEvent:voiceBroadcastInfoStartedEvent + roomState:roomState]; } } @@ -624,22 +625,8 @@ withVoiceBroadcastInfoStateEvent:lastVoiceBroadcastInfoEvent } else { - dispatch_group_t group = dispatch_group_create(); - dispatch_group_enter(group); - - __block MXEvent *voiceBroadcastInfoStartedEvent; - - [session eventWithEventId:voiceBroadcastInfo.voiceBroadcastId inRoom:roomId success:^(MXEvent *resultEvent) { - voiceBroadcastInfoStartedEvent = resultEvent; - dispatch_group_leave(group); - } failure:^(NSError *error) { - MXLogErrorDetails(@"[EventFormatter] Fetch eventWithEventId with error = %@", error.description); - dispatch_group_leave(group); - }]; - - dispatch_group_wait(group, DISPATCH_TIME_FOREVER); - - return voiceBroadcastInfoStartedEvent; + // Search for the event only in the store to avoid network calls while updating the room summary (this a synchronous process and we cannot delay it). + return [mxSession.store eventWithEventId:voiceBroadcastInfo.voiceBroadcastId inRoom:roomId]; } } From 06c326603ad1c46c614617d6d3eeb0b54bbf8d6e Mon Sep 17 00:00:00 2001 From: Nicolas Mauri Date: Wed, 25 Jan 2023 14:29:41 +0100 Subject: [PATCH 2/5] Add Towncrier file. --- changelog.d/pr-7300.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/pr-7300.bugfix diff --git a/changelog.d/pr-7300.bugfix b/changelog.d/pr-7300.bugfix new file mode 100644 index 000000000..bd546901f --- /dev/null +++ b/changelog.d/pr-7300.bugfix @@ -0,0 +1 @@ +Fix a deadlock when updating the summary of a room that has a voice broadcast. From 946ca1d1be4da76ad5ef9bee6d662f2ec071f7dd Mon Sep 17 00:00:00 2001 From: Andy Uhnak Date: Thu, 26 Jan 2023 11:15:31 +0000 Subject: [PATCH 3/5] Ensure E2EE never tracks UnknownError --- Riot/Modules/Analytics/DecryptionFailure.swift | 4 ---- Riot/Modules/Analytics/DecryptionFailureTracker.m | 7 ++----- .../Analytics/Helpers/MXCallHangupReason+Analytics.swift | 6 ++++++ changelog.d/pr-7304.change | 1 + 4 files changed, 9 insertions(+), 9 deletions(-) create mode 100644 changelog.d/pr-7304.change diff --git a/Riot/Modules/Analytics/DecryptionFailure.swift b/Riot/Modules/Analytics/DecryptionFailure.swift index 2f33f6369..1c991db88 100644 --- a/Riot/Modules/Analytics/DecryptionFailure.swift +++ b/Riot/Modules/Analytics/DecryptionFailure.swift @@ -16,12 +16,10 @@ import AnalyticsEvents -/// Failure reasons as defined in https://docs.google.com/document/d/1es7cTCeJEXXfRCTRgZerAM2Wg5ZerHjvlpfTW-gsOfI. @objc enum DecryptionFailureReason: Int { case unspecified case olmKeysNotSent case olmIndexError - case unexpected var errorName: AnalyticsEvent.Error.Name { switch self { @@ -31,8 +29,6 @@ import AnalyticsEvents return .OlmKeysNotSentError case .olmIndexError: return .OlmIndexError - case .unexpected: - return .UnknownError } } } diff --git a/Riot/Modules/Analytics/DecryptionFailureTracker.m b/Riot/Modules/Analytics/DecryptionFailureTracker.m index d175f9fd1..4a749b71a 100644 --- a/Riot/Modules/Analytics/DecryptionFailureTracker.m +++ b/Riot/Modules/Analytics/DecryptionFailureTracker.m @@ -105,12 +105,9 @@ NSString *const kDecryptionFailureTrackerAnalyticsCategory = @"e2e.failure"; reason = DecryptionFailureReasonOlmIndexError; break; - case MXDecryptingErrorEncryptionNotEnabledCode: - case MXDecryptingErrorUnableToDecryptCode: - reason = DecryptionFailureReasonUnexpected; - break; - default: + // All other error codes will be tracked as `OlmUnspecifiedError` and will include `context` containing + // the actual error code and localized description reason = DecryptionFailureReasonUnspecified; break; } diff --git a/Riot/Modules/Analytics/Helpers/MXCallHangupReason+Analytics.swift b/Riot/Modules/Analytics/Helpers/MXCallHangupReason+Analytics.swift index 4b8911ce8..c60f35446 100644 --- a/Riot/Modules/Analytics/Helpers/MXCallHangupReason+Analytics.swift +++ b/Riot/Modules/Analytics/Helpers/MXCallHangupReason+Analytics.swift @@ -21,6 +21,9 @@ extension __MXCallHangupReason { switch self { case .userHangup: return .VoipUserHangup + case .userBusy: + // There is no dedicated analytics event for `userBusy` error + return .UnknownError case .inviteTimeout: return .VoipInviteTimeout case .iceFailed: @@ -32,6 +35,9 @@ extension __MXCallHangupReason { case .unknownError: return .UnknownError default: + MXLog.failure("Unknown or unhandled hangup reason", context: [ + "reason": rawValue + ]) return .UnknownError } } diff --git a/changelog.d/pr-7304.change b/changelog.d/pr-7304.change new file mode 100644 index 000000000..174f32497 --- /dev/null +++ b/changelog.d/pr-7304.change @@ -0,0 +1 @@ +Analytics: Ensure E2EE never tracks UnknownError From 45718510bb021d7861564994ed6bcdde2c2a4a1c Mon Sep 17 00:00:00 2001 From: Doug Date: Thu, 26 Jan 2023 11:59:19 +0000 Subject: [PATCH 4/5] Fix avatar loading in SwiftUI. --- .../Common/Avatar/View/AvatarImage.swift | 21 ++++++---- .../Common/Avatar/View/SpaceAvatarImage.swift | 40 +++++++++++-------- .../Avatar/ViewModel/AvatarViewModel.swift | 25 +++++++----- changelog.d/7305.bugfix | 1 + 4 files changed, 52 insertions(+), 35 deletions(-) create mode 100644 changelog.d/7305.bugfix diff --git a/RiotSwiftUI/Modules/Common/Avatar/View/AvatarImage.swift b/RiotSwiftUI/Modules/Common/Avatar/View/AvatarImage.swift index b143d4d30..3ddd4d472 100644 --- a/RiotSwiftUI/Modules/Common/Avatar/View/AvatarImage.swift +++ b/RiotSwiftUI/Modules/Common/Avatar/View/AvatarImage.swift @@ -26,9 +26,11 @@ struct AvatarImage: View { var displayName: String? var size: AvatarSize + @State private var avatar: AvatarViewState = .empty + var body: some View { Group { - switch viewModel.viewState { + switch avatar { case .empty: ProgressView() case .placeholder(let firstCharacter, let colorIndex): @@ -42,13 +44,16 @@ struct AvatarImage: View { .frame(maxWidth: CGFloat(size.rawValue), maxHeight: CGFloat(size.rawValue)) .clipShape(Circle()) .onAppear { - viewModel.loadAvatar( - mxContentUri: mxContentUri, - matrixItemId: matrixItemId, - displayName: displayName, - colorCount: theme.colors.namesAndAvatars.count, - avatarSize: size - ) + avatar = viewModel.placeholderAvatar(matrixItemId: matrixItemId, + displayName: displayName, + colorCount: theme.colors.namesAndAvatars.count) + viewModel.loadAvatar(mxContentUri: mxContentUri, + matrixItemId: matrixItemId, + displayName: displayName, + colorCount: theme.colors.namesAndAvatars.count, + avatarSize: size ) { newState in + avatar = newState + } } } } diff --git a/RiotSwiftUI/Modules/Common/Avatar/View/SpaceAvatarImage.swift b/RiotSwiftUI/Modules/Common/Avatar/View/SpaceAvatarImage.swift index 2662831e1..708c26a2b 100644 --- a/RiotSwiftUI/Modules/Common/Avatar/View/SpaceAvatarImage.swift +++ b/RiotSwiftUI/Modules/Common/Avatar/View/SpaceAvatarImage.swift @@ -26,9 +26,11 @@ struct SpaceAvatarImage: View { var displayName: String? var size: AvatarSize + @State private var avatar: AvatarViewState = .empty + var body: some View { Group { - switch viewModel.viewState { + switch avatar { case .empty: ProgressView() case .placeholder(let firstCharacter, let colorIndex): @@ -48,23 +50,27 @@ struct SpaceAvatarImage: View { .clipShape(RoundedRectangle(cornerRadius: 8)) } } - .onChange(of: displayName, perform: { value in - viewModel.loadAvatar( - mxContentUri: mxContentUri, - matrixItemId: matrixItemId, - displayName: value, - colorCount: theme.colors.namesAndAvatars.count, - avatarSize: size - ) - }) + .onChange(of: displayName) { value in + guard case .placeholder = avatar else { return } + viewModel.loadAvatar(mxContentUri: mxContentUri, + matrixItemId: matrixItemId, + displayName: value, + colorCount: theme.colors.namesAndAvatars.count, + avatarSize: size) { newState in + avatar = newState + } + } .onAppear { - viewModel.loadAvatar( - mxContentUri: mxContentUri, - matrixItemId: matrixItemId, - displayName: displayName, - colorCount: theme.colors.namesAndAvatars.count, - avatarSize: size - ) + avatar = viewModel.placeholderAvatar(matrixItemId: matrixItemId, + displayName: displayName, + colorCount: theme.colors.namesAndAvatars.count) + viewModel.loadAvatar(mxContentUri: mxContentUri, + matrixItemId: matrixItemId, + displayName: displayName, + colorCount: theme.colors.namesAndAvatars.count, + avatarSize: size) { newState in + avatar = newState + } } } } diff --git a/RiotSwiftUI/Modules/Common/Avatar/ViewModel/AvatarViewModel.swift b/RiotSwiftUI/Modules/Common/Avatar/ViewModel/AvatarViewModel.swift index 10055738d..68037f552 100644 --- a/RiotSwiftUI/Modules/Common/Avatar/ViewModel/AvatarViewModel.swift +++ b/RiotSwiftUI/Modules/Common/Avatar/ViewModel/AvatarViewModel.swift @@ -22,14 +22,22 @@ import Foundation final class AvatarViewModel: ObservableObject { private let avatarService: AvatarServiceProtocol - @Published private(set) var viewState = AvatarViewState.empty - init(avatarService: AvatarServiceProtocol) { self.avatarService = avatarService } private var cancellables = Set() + func placeholderAvatar(matrixItemId: String, + displayName: String?, + colorCount: Int) -> AvatarViewState { + let placeholderViewModel = PlaceholderAvatarViewModel(displayName: displayName, + matrixItemId: matrixItemId, + colorCount: colorCount) + + return .placeholder(placeholderViewModel.firstCharacterCapitalized, placeholderViewModel.stableColorIndex) + } + /// Load an avatar /// - Parameters: /// - mxContentUri: The matrix content URI of the avatar. @@ -41,14 +49,10 @@ final class AvatarViewModel: ObservableObject { matrixItemId: String, displayName: String?, colorCount: Int, - avatarSize: AvatarSize) { - let placeholderViewModel = PlaceholderAvatarViewModel(displayName: displayName, - matrixItemId: matrixItemId, - colorCount: colorCount) - - viewState = .placeholder(placeholderViewModel.firstCharacterCapitalized, placeholderViewModel.stableColorIndex) - + avatarSize: AvatarSize, + avatarCompletion: @escaping (AvatarViewState) -> Void) { guard let mxContentUri = mxContentUri, mxContentUri.count > 0 else { + avatarCompletion(placeholderAvatar(matrixItemId: matrixItemId, displayName: displayName, colorCount: colorCount)) return } @@ -56,8 +60,9 @@ final class AvatarViewModel: ObservableObject { .sink { completion in guard case let .failure(error) = completion else { return } UILog.error("[AvatarService] Failed to retrieve avatar", context: error) + // No need to call the completion, there's nothing we can do and the error is logged. } receiveValue: { image in - self.viewState = .avatar(image) + avatarCompletion(.avatar(image)) } .store(in: &cancellables) } diff --git a/changelog.d/7305.bugfix b/changelog.d/7305.bugfix new file mode 100644 index 000000000..5f940f946 --- /dev/null +++ b/changelog.d/7305.bugfix @@ -0,0 +1 @@ +Space Switcher: Fix a bug where the avatars would all be the same. From a6e32517b7312b70a3815a1223fb11aaa44a30f3 Mon Sep 17 00:00:00 2001 From: Doug Date: Thu, 26 Jan 2023 16:24:32 +0000 Subject: [PATCH 5/5] version++ --- CHANGES.md | 12 ++++++++++++ Config/AppVersion.xcconfig | 4 ++-- changelog.d/7305.bugfix | 1 - changelog.d/pr-7300.bugfix | 1 - changelog.d/pr-7304.change | 1 - 5 files changed, 14 insertions(+), 5 deletions(-) delete mode 100644 changelog.d/7305.bugfix delete mode 100644 changelog.d/pr-7300.bugfix delete mode 100644 changelog.d/pr-7304.change diff --git a/CHANGES.md b/CHANGES.md index e4d8ef4d8..3484fcf48 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,3 +1,15 @@ +## Changes in 1.9.17 (2023-01-26) + +🙌 Improvements + +- Analytics: Ensure E2EE never tracks UnknownError ([#7304](https://github.com/vector-im/element-ios/pull/7304)) + +🐛 Bugfixes + +- Fix a deadlock when updating the summary of a room that has a voice broadcast. ([#7300](https://github.com/vector-im/element-ios/pull/7300)) +- Space Switcher: Fix a bug where the avatars would all be the same. ([#7305](https://github.com/vector-im/element-ios/issues/7305)) + + ## Changes in 1.9.16 (2023-01-24) ✨ Features diff --git a/Config/AppVersion.xcconfig b/Config/AppVersion.xcconfig index 7271fabb8..210603b23 100644 --- a/Config/AppVersion.xcconfig +++ b/Config/AppVersion.xcconfig @@ -15,5 +15,5 @@ // // Version -MARKETING_VERSION = 1.9.16 -CURRENT_PROJECT_VERSION = 1.9.16 +MARKETING_VERSION = 1.9.17 +CURRENT_PROJECT_VERSION = 1.9.17 diff --git a/changelog.d/7305.bugfix b/changelog.d/7305.bugfix deleted file mode 100644 index 5f940f946..000000000 --- a/changelog.d/7305.bugfix +++ /dev/null @@ -1 +0,0 @@ -Space Switcher: Fix a bug where the avatars would all be the same. diff --git a/changelog.d/pr-7300.bugfix b/changelog.d/pr-7300.bugfix deleted file mode 100644 index bd546901f..000000000 --- a/changelog.d/pr-7300.bugfix +++ /dev/null @@ -1 +0,0 @@ -Fix a deadlock when updating the summary of a room that has a voice broadcast. diff --git a/changelog.d/pr-7304.change b/changelog.d/pr-7304.change deleted file mode 100644 index 174f32497..000000000 --- a/changelog.d/pr-7304.change +++ /dev/null @@ -1 +0,0 @@ -Analytics: Ensure E2EE never tracks UnknownError