Crash fixes (#337)

* Add diffing logs and be more aggresive about unexpected states
* Move diff collection on a serial queue
* Handle now optional `addTimelineListener` result
* Treat RoomSummaryProvider invalidations separately
* Bump RustSDK to v0.0.9-demo
This commit is contained in:
Stefan Ceriu 2022-11-24 15:32:23 +02:00 committed by GitHub
parent 83410aa660
commit 7d8e94d3a7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 149 additions and 62 deletions

View File

@ -683,6 +683,7 @@
66F2402D738694F98729A441 /* RoomTimelineProvider.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RoomTimelineProvider.swift; sourceTree = "<group>"; };
68232D336E2B546AD95B78B5 /* XCUIElement.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = XCUIElement.swift; sourceTree = "<group>"; };
6920A4869821BF72FFC58842 /* MockMediaProvider.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockMediaProvider.swift; sourceTree = "<group>"; };
695AD3FAEAFEE32A6C73E8CB /* element-x-ios copy */ = {isa = PBXFileReference; lastKnownFileType = folder; name = "element-x-ios copy"; path = .; sourceTree = SOURCE_ROOT; };
6A1AAC8EB2992918D01874AC /* rue */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = rue; path = rue.lproj/Localizable.strings; sourceTree = "<group>"; };
6A580295A56B55A856CC4084 /* InfoPlistReader.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = InfoPlistReader.swift; sourceTree = "<group>"; };
6A6C4BE591FE5C38CE9C7EF3 /* UserProperties+Element.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "UserProperties+Element.swift"; sourceTree = "<group>"; };
@ -884,7 +885,6 @@
D1A9CCCF53495CF3D7B19FCE /* MockSessionVerificationControllerProxy.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockSessionVerificationControllerProxy.swift; sourceTree = "<group>"; };
D263254AFE5B7993FFBBF324 /* NSE.entitlements */ = {isa = PBXFileReference; lastKnownFileType = text.plist.entitlements; path = NSE.entitlements; sourceTree = "<group>"; };
D2D783758EAE6A88C93564EB /* VideoPlayerViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = VideoPlayerViewModel.swift; sourceTree = "<group>"; };
D31DC8105C6233E5FFD9B84C /* element-x-ios */ = {isa = PBXFileReference; lastKnownFileType = folder; name = "element-x-ios"; path = .; sourceTree = SOURCE_ROOT; };
D33116993D54FADC0C721C1F /* Application.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Application.swift; sourceTree = "<group>"; };
D3D455BC2423D911A62ACFB2 /* NSELogger.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NSELogger.swift; sourceTree = "<group>"; };
D4DA544B2520BFA65D6DB4BB /* target.yml */ = {isa = PBXFileReference; lastKnownFileType = text.yaml; path = target.yml; sourceTree = "<group>"; };
@ -1758,7 +1758,7 @@
9413F680ECDFB2B0DDB0DEF2 /* Packages */ = {
isa = PBXGroup;
children = (
D31DC8105C6233E5FFD9B84C /* element-x-ios */,
695AD3FAEAFEE32A6C73E8CB /* element-x-ios copy */,
);
name = Packages;
sourceTree = SOURCE_ROOT;
@ -3651,7 +3651,7 @@
repositoryURL = "https://github.com/matrix-org/matrix-rust-components-swift";
requirement = {
kind = exactVersion;
version = "0.0.8-demo";
version = "0.0.9-demo";
};
};
96495DD8554E2F39D3954354 /* XCRemoteSwiftPackageReference "posthog-ios" */ = {

View File

@ -86,8 +86,8 @@
"kind" : "remoteSourceControl",
"location" : "https://github.com/matrix-org/matrix-rust-components-swift",
"state" : {
"revision" : "c708a3ec977f02c50ad73e95bb855f99e726e8ca",
"version" : "0.0.8-demo"
"revision" : "2645cfb64f3255f299a63af280b5172a6dd6602c",
"version" : "0.0.9-demo"
}
},
{
@ -111,7 +111,7 @@
{
"identity" : "swift-snapshot-testing",
"kind" : "remoteSourceControl",
"location" : "https://github.com/pointfreeco/swift-snapshot-testing.git",
"location" : "https://github.com/pointfreeco/swift-snapshot-testing",
"state" : {
"revision" : "f29e2014f6230cf7d5138fc899da51c7f513d467",
"version" : "1.10.0"
@ -129,7 +129,7 @@
{
"identity" : "swiftui-introspect",
"kind" : "remoteSourceControl",
"location" : "https://github.com/siteline/SwiftUI-Introspect.git",
"location" : "https://github.com/siteline/SwiftUI-Introspect",
"state" : {
"revision" : "f2616860a41f9d9932da412a8978fec79c06fe24",
"version" : "0.1.4"

View File

@ -88,7 +88,6 @@ class AppCoordinator: AppCoordinatorProtocol {
// We can filter by level, crate and even file. See more details here:
// https://docs.rs/tracing-subscriber/0.2.7/tracing_subscriber/filter/struct.EnvFilter.html#examples
setupTracing(filter: "warn,hyper=warn,sled=warn,matrix_sdk_sled=warn")
loggerConfiguration.logLevel = .debug
#else
setupTracing(filter: "info,hyper=warn,sled=warn,matrix_sdk_sled=warn")

View File

@ -57,7 +57,9 @@ struct MockRoomProxy: RoomProxyProtocol {
func startLiveEventListener() { }
func addTimelineListener(listener: TimelineListener) { }
func addTimelineListener(listener: TimelineListener) -> Result<Void, RoomProxyError> {
.failure(.failedAddingTimelineListener)
}
func paginateBackwards(count: UInt) async -> Result<Void, RoomProxyError> {
.failure(.failedPaginatingBackwards)

View File

@ -141,8 +141,12 @@ class RoomProxy: RoomProxyProtocol {
}
}
func addTimelineListener(listener: TimelineListener) {
slidingSyncRoom.addTimelineListener(listener: listener)
func addTimelineListener(listener: TimelineListener) -> Result<Void, RoomProxyError> {
if let result = slidingSyncRoom.addTimelineListener(listener: listener), result == true {
return .success(())
} else {
return .failure(.failedAddingTimelineListener)
}
}
func paginateBackwards(count: UInt) async -> Result<Void, RoomProxyError> {

View File

@ -26,6 +26,7 @@ enum RoomProxyError: Error {
case failedRetrievingMemberDisplayName
case failedSendingMessage
case failedRedactingEvent
case failedAddingTimelineListener
}
@MainActor
@ -54,7 +55,7 @@ protocol RoomProxyProtocol {
func loadDisplayName() async -> Result<String, RoomProxyError>
func addTimelineListener(listener: TimelineListener)
func addTimelineListener(listener: TimelineListener) -> Result<Void, RoomProxyError>
func paginateBackwards(count: UInt) async -> Result<Void, RoomProxyError>

View File

@ -31,18 +31,21 @@ private class WeakRoomSummaryProviderWrapper: SlidingSyncViewRoomListObserver, S
// MARK: - SlidingSyncViewRoomListObserver
func didReceiveUpdate(diff: SlidingSyncViewRoomsListDiff) {
MXLog.verbose("Received room diff")
roomListDiffPublisher.send(diff)
}
// MARK: - SlidingSyncViewStateObserver
func didReceiveUpdate(newState: SlidingSyncState) {
MXLog.verbose("Updated state: \(newState)")
stateUpdatePublisher.send(newState)
}
// MARK: - SlidingSyncViewRoomsCountObserver
func didReceiveUpdate(count: UInt32) {
MXLog.verbose("Updated room count: \(count)")
countUpdatePublisher.send(UInt(count))
}
}
@ -51,6 +54,7 @@ class RoomSummaryProvider: RoomSummaryProviderProtocol {
private let slidingSyncController: SlidingSyncProtocol
private let slidingSyncView: SlidingSyncViewProtocol
private let roomMessageFactory: RoomMessageFactoryProtocol
private let serialDispatchQueue: DispatchQueue
private var listUpdateObserverToken: StoppableSpawn?
private var stateUpdateObserverToken: StoppableSpawn?
@ -78,6 +82,7 @@ class RoomSummaryProvider: RoomSummaryProviderProtocol {
self.slidingSyncView = slidingSyncView
self.slidingSyncController = slidingSyncController
self.roomMessageFactory = roomMessageFactory
serialDispatchQueue = DispatchQueue(label: "io.element.elementx.roomsummaryprovider")
let weakProvider = WeakRoomSummaryProviderWrapper()
@ -97,7 +102,7 @@ class RoomSummaryProvider: RoomSummaryProviderProtocol {
.store(in: &cancellables)
weakProvider.roomListDiffPublisher
.collect(.byTime(DispatchQueue.global(), 0.25))
.collect(.byTime(serialDispatchQueue, 0.25))
.sink { self.updateRoomsWithDiffs($0) }
.store(in: &cancellables)
@ -140,14 +145,31 @@ class RoomSummaryProvider: RoomSummaryProviderProtocol {
// MARK: - Private
fileprivate func updateRoomsWithDiffs(_ diffs: [SlidingSyncViewRoomsListDiff]) {
MXLog.verbose("Received diffs")
rooms = diffs
.reduce(rooms) { partialResult, diff in
guard let collectionDiff = buildDiff(from: diff, on: partialResult) else {
return partialResult
.reduce(rooms) { currentItems, diff in
// Invalidations are a no-op for the moment
if diff.isInvalidation {
return currentItems
}
return partialResult.applying(collectionDiff) ?? partialResult
guard let collectionDiff = buildDiff(from: diff, on: currentItems) else {
MXLog.error("Failed building CollectionDifference from \(diff)")
return currentItems
}
guard let updatedItems = currentItems.applying(collectionDiff) else {
MXLog.error("Failed applying diff: \(collectionDiff)")
return currentItems
}
MXLog.verbose("Applied diff \(collectionDiff), new count: \(updatedItems.count)")
return updatedItems
}
MXLog.verbose("Finished applying diffs")
}
private func buildEmptyRoomSummary(forIdentifier identifier: String = UUID().uuidString) -> RoomSummary {
@ -192,32 +214,33 @@ class RoomSummaryProvider: RoomSummaryProviderProtocol {
}
private func buildDiff(from diff: SlidingSyncViewRoomsListDiff, on rooms: [RoomSummary]) -> CollectionDifference<RoomSummary>? {
// Invalidations are a no-op for the moment
if diff.isInvalidation {
return nil
}
var changes = [CollectionDifference<RoomSummary>.Change]()
switch diff {
case .push(value: let value):
MXLog.verbose("Push")
let summary = buildSummaryForRoomListEntry(value)
changes.append(.insert(offset: Int(rooms.count), element: summary, associatedWith: nil))
case .updateAt(let index, let value):
MXLog.verbose("Update \(index), current total count: \(rooms.count)")
let summary = buildSummaryForRoomListEntry(value)
changes.append(.remove(offset: Int(index), element: summary, associatedWith: nil))
changes.append(.insert(offset: Int(index), element: summary, associatedWith: nil))
case .insertAt(let index, let value):
MXLog.verbose("Insert at \(index), current total count: \(rooms.count)")
let summary = buildSummaryForRoomListEntry(value)
changes.append(.insert(offset: Int(index), element: summary, associatedWith: nil))
case .move(let oldIndex, let newIndex):
MXLog.verbose("Move from: \(oldIndex) to: \(newIndex), current total count: \(rooms.count)")
let summary = rooms[Int(oldIndex)]
changes.append(.remove(offset: Int(oldIndex), element: summary, associatedWith: nil))
changes.append(.insert(offset: Int(newIndex), element: summary, associatedWith: nil))
case .removeAt(let index):
MXLog.verbose("Remove from: \(index), current total count: \(rooms.count)")
let summary = rooms[Int(index)]
changes.append(.remove(offset: Int(index), element: summary, associatedWith: nil))
case .replace(let values):
MXLog.verbose("Replace all items with new count: \(values.count), current total count: \(rooms.count)")
for (index, summary) in rooms.enumerated() {
changes.append(.remove(offset: index, element: summary, associatedWith: nil))
}

View File

@ -28,6 +28,7 @@ private class RoomTimelineListener: TimelineListener {
class RoomTimelineProvider: RoomTimelineProviderProtocol {
private let roomProxy: RoomProxyProtocol
private let serialDispatchQueue: DispatchQueue
private var cancellables = Set<AnyCancellable>()
let itemsPublisher = CurrentValueSubject<[TimelineItemProxy], Never>([])
@ -45,15 +46,21 @@ class RoomTimelineProvider: RoomTimelineProviderProtocol {
init(roomProxy: RoomProxyProtocol) {
self.roomProxy = roomProxy
serialDispatchQueue = DispatchQueue(label: "io.element.elementx.roomtimelineprovider")
itemProxies = []
Task {
let roomTimelineListener = RoomTimelineListener()
await roomProxy.addTimelineListener(listener: roomTimelineListener)
switch await roomProxy.addTimelineListener(listener: roomTimelineListener) {
case .failure:
MXLog.error("Failed adding timeline listener on room with identifier: \(await roomProxy.id)")
default:
break
}
roomTimelineListener
.itemsUpdatePublisher
.collect(.byTime(DispatchQueue.global(), 0.25))
.collect(.byTime(serialDispatchQueue, 0.25))
.sink { self.updateItemsWithDiffs($0) }
.store(in: &cancellables)
}
@ -63,10 +70,13 @@ class RoomTimelineProvider: RoomTimelineProviderProtocol {
// Set this back to false after actually updating the items or if failed
backPaginationPublisher.send(true)
MXLog.info("Started back pagination request")
switch await roomProxy.paginateBackwards(count: count) {
case .success:
MXLog.info("Finished back pagination request")
return .success(())
case .failure(let error):
MXLog.error("Failed back pagination request with error: \(error)")
backPaginationPublisher.send(false)
if error == .noMoreMessagesToBackPaginate {
@ -78,28 +88,42 @@ class RoomTimelineProvider: RoomTimelineProviderProtocol {
}
func sendMessage(_ message: String, inReplyToItemId: String?) async -> Result<Void, RoomTimelineProviderError> {
if let inReplyToItemId {
MXLog.info("Sending message in reply to: \(inReplyToItemId)")
} else {
MXLog.info("Sending message")
}
switch await roomProxy.sendMessage(message, inReplyToEventId: inReplyToItemId) {
case .success:
MXLog.info("Finished sending message")
return .success(())
case .failure:
case .failure(let error):
MXLog.error("Failed sending message with error: \(error)")
return .failure(.failedSendingMessage)
}
}
func editMessage(_ newMessage: String, originalItemId: String) async -> Result<Void, RoomTimelineProviderError> {
MXLog.info("Editing message: \(originalItemId)")
switch await roomProxy.editMessage(newMessage, originalEventId: originalItemId) {
case .success:
MXLog.info("Finished editing message")
return .success(())
case .failure:
case .failure(let error):
MXLog.error("Failed editing message with error: \(error)")
return .failure(.failedSendingMessage)
}
}
func redact(_ eventID: String) async -> Result<Void, RoomTimelineProviderError> {
MXLog.info("Redacting message: \(eventID)")
switch await roomProxy.redact(eventID) {
case .success:
MXLog.info("Finished redacting message")
return .success(())
case .failure:
case .failure(let error):
MXLog.error("Failed redacting message with error: \(error)")
return .failure(.failedRedactingItem)
}
}
@ -107,66 +131,100 @@ class RoomTimelineProvider: RoomTimelineProviderProtocol {
// MARK: - Private
private func updateItemsWithDiffs(_ diffs: [TimelineDiff]) {
MXLog.verbose("Received timeline diffs")
itemProxies = diffs
.reduce(itemProxies) { partialResult, diff in
guard let collectionDiff = buildDiff(from: diff, on: partialResult) else {
return partialResult
.reduce(itemProxies) { currentItems, diff in
guard let collectionDiff = buildDiff(from: diff, on: currentItems) else {
MXLog.error("Failed building CollectionDifference from \(diff)")
return currentItems
}
return partialResult.applying(collectionDiff) ?? partialResult
guard let updatedItems = currentItems.applying(collectionDiff) else {
MXLog.error("Failed applying diff: \(collectionDiff)")
return currentItems
}
MXLog.verbose("Applied diff \(collectionDiff), new count: \(updatedItems.count)")
return updatedItems
}
MXLog.verbose("Finished applying diffs")
}
// swiftlint:disable:next cyclomatic_complexity
// swiftlint:disable:next cyclomatic_complexity function_body_length
private func buildDiff(from diff: TimelineDiff, on itemProxies: [TimelineItemProxy]) -> CollectionDifference<TimelineItemProxy>? {
var changes = [CollectionDifference<TimelineItemProxy>.Change]()
switch diff.change() {
case .push:
if let item = diff.push() {
let itemProxy = TimelineItemProxy(item: item)
changes.append(.insert(offset: Int(itemProxies.count), element: itemProxy, associatedWith: nil))
guard let item = diff.push() else {
fatalError()
}
MXLog.verbose("Push")
let itemProxy = TimelineItemProxy(item: item)
changes.append(.insert(offset: Int(itemProxies.count), element: itemProxy, associatedWith: nil))
case .updateAt:
if let update = diff.updateAt() {
let itemProxy = TimelineItemProxy(item: update.item)
changes.append(.remove(offset: Int(update.index), element: itemProxy, associatedWith: nil))
changes.append(.insert(offset: Int(update.index), element: itemProxy, associatedWith: nil))
guard let update = diff.updateAt() else {
fatalError()
}
MXLog.verbose("Update \(update.index), current total count: \(itemProxies.count)")
let itemProxy = TimelineItemProxy(item: update.item)
changes.append(.remove(offset: Int(update.index), element: itemProxy, associatedWith: nil))
changes.append(.insert(offset: Int(update.index), element: itemProxy, associatedWith: nil))
case .insertAt:
if let update = diff.insertAt() {
let itemProxy = TimelineItemProxy(item: update.item)
changes.append(.insert(offset: Int(update.index), element: itemProxy, associatedWith: nil))
guard let update = diff.insertAt() else {
fatalError()
}
MXLog.verbose("Insert at \(update.index), current total count: \(itemProxies.count)")
let itemProxy = TimelineItemProxy(item: update.item)
changes.append(.insert(offset: Int(update.index), element: itemProxy, associatedWith: nil))
case .move:
if let update = diff.move() {
let itemProxy = itemProxies[Int(update.oldIndex)]
changes.append(.remove(offset: Int(update.oldIndex), element: itemProxy, associatedWith: nil))
changes.append(.insert(offset: Int(update.newIndex), element: itemProxy, associatedWith: nil))
guard let update = diff.move() else {
fatalError()
}
case .removeAt:
if let index = diff.removeAt() {
let itemProxy = itemProxies[Int(index)]
changes.append(.remove(offset: Int(index), element: itemProxy, associatedWith: nil))
}
case .replace:
if let items = diff.replace() {
for (index, itemProxy) in itemProxies.enumerated() {
changes.append(.remove(offset: index, element: itemProxy, associatedWith: nil))
}
for (index, item) in items.enumerated() {
changes.append(.insert(offset: index, element: TimelineItemProxy(item: item), associatedWith: nil))
}
MXLog.verbose("Move from: \(update.oldIndex) to: \(update.newIndex), current total count: \(itemProxies.count)")
let itemProxy = itemProxies[Int(update.oldIndex)]
changes.append(.remove(offset: Int(update.oldIndex), element: itemProxy, associatedWith: nil))
changes.append(.insert(offset: Int(update.newIndex), element: itemProxy, associatedWith: nil))
case .removeAt:
guard let index = diff.removeAt() else {
fatalError()
}
MXLog.verbose("Remove from: \(index), current total count: \(itemProxies.count)")
let itemProxy = itemProxies[Int(index)]
changes.append(.remove(offset: Int(index), element: itemProxy, associatedWith: nil))
case .replace:
guard let items = diff.replace() else {
fatalError()
}
MXLog.verbose("Replace all items with new count: \(items.count), current total count: \(itemProxies.count)")
for (index, itemProxy) in itemProxies.enumerated() {
changes.append(.remove(offset: index, element: itemProxy, associatedWith: nil))
}
for (index, item) in items.enumerated() {
changes.append(.insert(offset: index, element: TimelineItemProxy(item: item), associatedWith: nil))
}
case .clear:
MXLog.verbose("Clear all items, current total count: \(itemProxies.count)")
for (index, itemProxy) in itemProxies.enumerated() {
changes.append(.remove(offset: index, element: itemProxy, associatedWith: nil))
}
case .pop:
if let itemProxy = itemProxies.last {
changes.append(.remove(offset: itemProxies.count - 1, element: itemProxy, associatedWith: nil))
MXLog.verbose("Pop, current total count: \(itemProxies.count)")
guard let itemProxy = itemProxies.last else {
fatalError()
}
changes.append(.remove(offset: itemProxies.count - 1, element: itemProxy, associatedWith: nil))
}
return CollectionDifference(changes)

View File

@ -39,7 +39,7 @@ include:
packages:
MatrixRustSDK:
url: https://github.com/matrix-org/matrix-rust-components-swift
exactVersion: 0.0.8-demo
exactVersion: 0.0.9-demo
# path: ../matrix-rust-components-swift
DesignKit:
path: ./