From b917120f17304506d2972ac1def1474aea6b6445 Mon Sep 17 00:00:00 2001 From: Shadowfacts Date: Wed, 10 Nov 2021 17:15:12 -0500 Subject: [PATCH] Fix crash when conversation loading fails --- .../ConversationTableViewController.swift | 122 +++++++++++++----- ...fableTimelineLikeTableViewController.swift | 29 +---- Tusker/Views/Toast/ToastConfiguration.swift | 22 ++++ 3 files changed, 115 insertions(+), 58 deletions(-) diff --git a/Tusker/Screens/Conversation/ConversationTableViewController.swift b/Tusker/Screens/Conversation/ConversationTableViewController.swift index 1ab52a5269..ed77881c18 100644 --- a/Tusker/Screens/Conversation/ConversationTableViewController.swift +++ b/Tusker/Screens/Conversation/ConversationTableViewController.swift @@ -36,6 +36,8 @@ class ConversationTableViewController: EnhancedTableViewController { var showStatusesAutomatically = false var visibilityBarButtonItem: UIBarButtonItem! + private var loadingState = LoadingState.unloaded + init(for mainStatusID: String, state: StatusState = .unknown, mastodonController: MastodonController) { self.mainStatusID = mainStatusID self.mainStatusState = state @@ -136,9 +138,12 @@ class ConversationTableViewController: EnhancedTableViewController { } private func loadMainStatus() { + guard loadingState == .unloaded else { return } + if let mainStatus = mastodonController.persistentContainer.status(for: mainStatusID) { self.mainStatusLoaded(mainStatus) } else { + loadingState = .loadingMain let request = Client.getStatus(id: mainStatusID) mastodonController.run(request) { (response) in switch response { @@ -148,14 +153,24 @@ class ConversationTableViewController: EnhancedTableViewController { self.mainStatusLoaded(statusMO) } - case .failure(_): - fatalError() + case let .failure(error): + DispatchQueue.main.async { + self.loadingState = .unloaded + + let config = ToastConfiguration(from: error, with: "Error Loading Status") { [weak self] (toast) in + toast.dismissToast(animated: true) + self?.loadMainStatus() + } + self.showToast(configuration: config, animated: true) + } } } } } private func mainStatusLoaded(_ mainStatus: StatusMO) { + mainStatus.incrementReferenceCount() + let mainStatusItem = Item.status(id: mainStatusID, state: mainStatusState) var snapshot = NSDiffableDataSourceSnapshot() @@ -163,47 +178,77 @@ class ConversationTableViewController: EnhancedTableViewController { snapshot.appendItems([mainStatusItem], toSection: .statuses) dataSource.apply(snapshot, animatingDifferences: false) - let mainStatusInReplyToID = mainStatus.inReplyToID - mainStatus.incrementReferenceCount() + loadingState = .loadedMain + loadContext(for: mainStatus) + } + + private func loadContext(for mainStatus: StatusMO) { + guard loadingState == .loadedMain else { return } + + loadingState = .loadingContext + + // save the id here because we can't access the MO from the whatever thread the network callback happens on + let mainStatusInReplyToID = mainStatus.inReplyToID + // todo: it would be nice to cache these contexts let request = Status.getContext(mainStatusID) mastodonController.run(request) { response in - guard case let .success(context, _) = response else { fatalError() } - - let parents = self.getDirectParents(inReplyTo: mainStatusInReplyToID, from: context.ancestors) - let parentStatuses = context.ancestors.filter { parents.contains($0.id) } - - // todo: should this really be blindly adding all the descendants? - self.mastodonController.persistentContainer.addAll(statuses: parentStatuses + context.descendants) { + switch response { + case let .success(context, _): + let parentIDs = self.getDirectParents(inReplyTo: mainStatusInReplyToID, from: context.ancestors) + let parentStatuses = context.ancestors.filter { parentIDs.contains($0.id) } + + // todo: should this really be blindly adding all the descendants? + self.mastodonController.persistentContainer.addAll(statuses: parentStatuses + context.descendants) { + DispatchQueue.main.async { + self.contextLoaded(mainStatus: mainStatus, context: context, parentIDs: parentIDs) + } + } + + case let .failure(error): DispatchQueue.main.async { - var snapshot = self.dataSource.snapshot() - snapshot.insertItems(parents.map { .status(id: $0, state: .unknown) }, beforeItem: mainStatusItem) + self.loadingState = .loadedMain - // fetch all descendant status managed objects - let descendantIDs = context.descendants.map(\.id) - let request: NSFetchRequest = StatusMO.fetchRequest() - request.predicate = NSPredicate(format: "id in %@", descendantIDs) - - if let descendants = try? self.mastodonController.persistentContainer.viewContext.fetch(request) { - // convert array of descendant statuses into tree of sub-threads - let childThreads = self.getDescendantThreads(mainStatus: mainStatus, descendants: descendants) - - // convert sub-threads into items for section and add to snapshot - self.addFlattenedChildThreadsToSnapshot(childThreads, mainStatus: mainStatus, snapshot: &snapshot) - } - - self.dataSource.apply(snapshot, animatingDifferences: false) { - // ensure that the main status is on-screen after newly loaded statuses are added - // todo: should this not happen if the user has already started scrolling (e.g. because the main status is very long)? - if let indexPath = self.dataSource.indexPath(for: mainStatusItem) { - self.tableView.scrollToRow(at: indexPath, at: .middle, animated: false) - } + let config = ToastConfiguration(from: error, with: "Error Loading Content") { [weak self] (toast) in + toast.dismissToast(animated: true) + self?.loadContext(for: mainStatus) } + self.showToast(configuration: config, animated: true) } } } } + + private func contextLoaded(mainStatus: StatusMO, context: ConversationContext, parentIDs: [String]) { + let mainStatusItem = Item.status(id: mainStatusID, state: mainStatusState) + + var snapshot = self.dataSource.snapshot() + snapshot.insertItems(parentIDs.map { .status(id: $0, state: .unknown) }, beforeItem: mainStatusItem) + + // fetch all descendant status managed objects + let descendantIDs = context.descendants.map(\.id) + let request: NSFetchRequest = StatusMO.fetchRequest() + request.predicate = NSPredicate(format: "id in %@", descendantIDs) + + if let descendants = try? self.mastodonController.persistentContainer.viewContext.fetch(request) { + // convert array of descendant statuses into tree of sub-threads + let childThreads = self.getDescendantThreads(mainStatus: mainStatus, descendants: descendants) + + // convert sub-threads into items for section and add to snapshot + self.addFlattenedChildThreadsToSnapshot(childThreads, mainStatus: mainStatus, snapshot: &snapshot) + } + + self.dataSource.apply(snapshot, animatingDifferences: false) { + // ensure that the main status is on-screen after newly loaded statuses are added + // todo: should this not happen if the user has already started scrolling (e.g. because the main status is very long)? + if let indexPath = self.dataSource.indexPath(for: mainStatusItem) { + self.tableView.scrollToRow(at: indexPath, at: .middle, animated: false) + } + } + + self.loadingState = .loadedAll + } private func getDirectParents(inReplyTo inReplyToID: String?, from statuses: [Status]) -> [String] { var statuses = statuses @@ -390,6 +435,16 @@ extension ConversationTableViewController { } } +extension ConversationTableViewController { + private enum LoadingState: Equatable { + case unloaded + case loadingMain + case loadedMain + case loadingContext + case loadedAll + } +} + extension ConversationTableViewController: TuskerNavigationDelegate { func conversation(mainStatusID: String, state: StatusState) -> ConversationTableViewController { let vc = ConversationTableViewController(for: mainStatusID, state: state, mastodonController: mastodonController) @@ -419,3 +474,6 @@ extension ConversationTableViewController: UITableViewDataSourcePrefetching, Sta cancelPrefetchingStatuses(with: ids) } } + +extension ConversationTableViewController: ToastableViewController { +} diff --git a/Tusker/Screens/Utilities/DiffableTimelineLikeTableViewController.swift b/Tusker/Screens/Utilities/DiffableTimelineLikeTableViewController.swift index 8a01cf2a6c..ca53cc2e18 100644 --- a/Tusker/Screens/Utilities/DiffableTimelineLikeTableViewController.swift +++ b/Tusker/Screens/Utilities/DiffableTimelineLikeTableViewController.swift @@ -121,11 +121,7 @@ class DiffableTimelineLikeTableViewController Void) { + self.init(title: title) + self.subtitle = error.localizedDescription + self.systemImageName = error.systemImageName + self.actionTitle = "Retry" + self.action = retryAction + } +} + +fileprivate extension Client.Error { + var systemImageName: String { + switch self { + case .networkError(_): + return "wifi.exclamationmark" + default: + return "exclamationmark.triangle" + } + } +}