From 4279bed74df5e6eb965e5cbd8f923def15078580 Mon Sep 17 00:00:00 2001 From: Satindar Dhillon Date: Wed, 4 May 2022 14:48:12 -0700 Subject: [PATCH 1/3] show progress view on web reader if content doesn't load in one second --- .../App/Views/WebReader/WebReaderContainer.swift | 12 ++++++++++-- .../App/Views/WebReader/WebReaderViewModel.swift | 7 ++++--- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/apple/OmnivoreKit/Sources/App/Views/WebReader/WebReaderContainer.swift b/apple/OmnivoreKit/Sources/App/Views/WebReader/WebReaderContainer.swift index c8e26c311..182f3aca2 100644 --- a/apple/OmnivoreKit/Sources/App/Views/WebReader/WebReaderContainer.swift +++ b/apple/OmnivoreKit/Sources/App/Views/WebReader/WebReaderContainer.swift @@ -15,6 +15,7 @@ import WebKit @State private var navBarVisibilityRatio = 1.0 @State private var showDeleteConfirmation = false @State private var showOverlay = true + @State private var progressViewOpacity = 0.0 @State var increaseFontActionID: UUID? @State var decreaseFontActionID: UUID? @State var annotationSaveTransactionID: UUID? @@ -186,9 +187,16 @@ import WebKit } ) } + } else if let errorMessage = viewModel.errorMessage { + Text(errorMessage).padding() } else { - Text(viewModel.contentFetchFailed ? "Unable to fetch content." : "Processing...") - .padding() + ProgressView() + .opacity(progressViewOpacity) + .onAppear { + DispatchQueue.main.asyncAfter(deadline: .now() + .milliseconds(1000)) { + progressViewOpacity = 1 + } + } .task { await viewModel.loadContent(dataService: dataService, itemID: item.unwrappedID) } diff --git a/apple/OmnivoreKit/Sources/App/Views/WebReader/WebReaderViewModel.swift b/apple/OmnivoreKit/Sources/App/Views/WebReader/WebReaderViewModel.swift index 97017cfd8..2daef7731 100644 --- a/apple/OmnivoreKit/Sources/App/Views/WebReader/WebReaderViewModel.swift +++ b/apple/OmnivoreKit/Sources/App/Views/WebReader/WebReaderViewModel.swift @@ -10,15 +10,16 @@ struct SafariWebLink: Identifiable { @MainActor final class WebReaderViewModel: ObservableObject { @Published var articleContent: ArticleContent? - @Published var contentFetchFailed = false + @Published var errorMessage: String? func loadContent(dataService: DataService, itemID: String) async { - contentFetchFailed = false + errorMessage = nil do { articleContent = try await dataService.fetchArticleContent(itemID: itemID) } catch { - contentFetchFailed = true + // TODO: check if this is a network or parsing error + errorMessage = "Unable to extract your content." } } From a2a7a2a232507488a43090d183a08f236826f4a4 Mon Sep 17 00:00:00 2001 From: Satindar Dhillon Date: Wed, 4 May 2022 15:20:44 -0700 Subject: [PATCH 2/3] handle content fetch errors in web reader --- .../Views/WebReader/WebReaderViewModel.swift | 12 +++++-- .../Models/ErrorModels/SaveArticleError.swift | 2 ++ .../Queries/ArticleContentQuery.swift | 34 ++++++++++--------- 3 files changed, 30 insertions(+), 18 deletions(-) diff --git a/apple/OmnivoreKit/Sources/App/Views/WebReader/WebReaderViewModel.swift b/apple/OmnivoreKit/Sources/App/Views/WebReader/WebReaderViewModel.swift index 2daef7731..1a03c96dd 100644 --- a/apple/OmnivoreKit/Sources/App/Views/WebReader/WebReaderViewModel.swift +++ b/apple/OmnivoreKit/Sources/App/Views/WebReader/WebReaderViewModel.swift @@ -18,8 +18,16 @@ struct SafariWebLink: Identifiable { do { articleContent = try await dataService.fetchArticleContent(itemID: itemID) } catch { - // TODO: check if this is a network or parsing error - errorMessage = "Unable to extract your content." + if let fetchError = error as? ContentFetchError { + switch fetchError { + case .network: + errorMessage = "We were unable to retrieve your content. Please ccheck network connectivity and try again." + default: + errorMessage = "We were unable to parse your content." + } + } else { + errorMessage = "We were unable to retrieve your content." + } } } diff --git a/apple/OmnivoreKit/Sources/Models/ErrorModels/SaveArticleError.swift b/apple/OmnivoreKit/Sources/Models/ErrorModels/SaveArticleError.swift index adbb15246..3c806bb3c 100644 --- a/apple/OmnivoreKit/Sources/Models/ErrorModels/SaveArticleError.swift +++ b/apple/OmnivoreKit/Sources/Models/ErrorModels/SaveArticleError.swift @@ -1,5 +1,7 @@ import Foundation +public typealias ContentFetchError = SaveArticleError + public enum SaveArticleError: Error { case unauthorized case network diff --git a/apple/OmnivoreKit/Sources/Services/DataService/Queries/ArticleContentQuery.swift b/apple/OmnivoreKit/Sources/Services/DataService/Queries/ArticleContentQuery.swift index f2beed079..a4dd8fec5 100644 --- a/apple/OmnivoreKit/Sources/Services/DataService/Queries/ArticleContentQuery.swift +++ b/apple/OmnivoreKit/Sources/Services/DataService/Queries/ArticleContentQuery.swift @@ -20,7 +20,7 @@ extension DataService { func prefetchPage(pendingLink: PendingLink, username: String) async { let content = try? await articleContent(username: username, itemID: pendingLink.itemID, useCache: false) - if content?.contentStatus == .processing, pendingLink.retryCount < 6 { + if content?.contentStatus == .processing, pendingLink.retryCount < 7 { let retryDelayInNanoSeconds = UInt64(pendingLink.retryCount * 2 * 1_000_000_000) do { @@ -45,26 +45,24 @@ extension DataService { username: String? = nil, requestCount: Int = 1 ) async throws -> ArticleContent { - guard let username = username ?? currentViewer?.username else { - throw BasicError.message(messageText: "username could not be fetched from core data") + guard requestCount < 7 else { + throw ContentFetchError.badData } - guard let fetchedContent = try? await articleContent(username: username, itemID: itemID, useCache: true) else { - throw BasicError.message(messageText: "networking error") + guard let username = username ?? currentViewer?.username else { + throw ContentFetchError.unauthorized } + let fetchedContent = try await articleContent(username: username, itemID: itemID, useCache: true) + switch fetchedContent.contentStatus { case .failed: - throw BasicError.message(messageText: "content processing failed") + throw ContentFetchError.badData case .processing: - do { - let retryDelayInNanoSeconds = UInt64(requestCount * 2 * 1_000_000_000) - try await Task.sleep(nanoseconds: retryDelayInNanoSeconds) - logger.debug("fetching content for \(itemID). request count: \(requestCount)") - return try await fetchArticleContent(itemID: itemID, username: username, requestCount: requestCount + 1) - } catch { - throw BasicError.message(messageText: "content fetch failed") - } + let retryDelayInNanoSeconds = UInt64(requestCount * 2 * 1_000_000_000) + try await Task.sleep(nanoseconds: retryDelayInNanoSeconds) + logger.debug("fetching content for \(itemID). request count: \(requestCount)") + return try await fetchArticleContent(itemID: itemID, username: username, requestCount: requestCount + 1) case .succeeded, .unknown: return fetchedContent } @@ -120,7 +118,7 @@ extension DataService { return try await withCheckedThrowingContinuation { continuation in send(query, to: path, headers: headers) { [weak self] queryResult in guard let payload = try? queryResult.get() else { - continuation.resume(throwing: BasicError.message(messageText: "network error")) + continuation.resume(throwing: ContentFetchError.network) return } @@ -129,6 +127,10 @@ extension DataService { // Default to suceeded since older links will return a nil status // (but the content is almost always there) let status = result.contentStatus ?? .succeeded + if status == .failed { + continuation.resume(throwing: ContentFetchError.badData) + return + } if status == .succeeded { self?.persistArticleContent( @@ -146,7 +148,7 @@ extension DataService { continuation.resume(returning: articleContent) case .error: - continuation.resume(throwing: BasicError.message(messageText: "LinkedItem fetch error")) + continuation.resume(throwing: ContentFetchError.badData) } } } From 4e5377d7f45f9e15fa03c468949cde43281a239b Mon Sep 17 00:00:00 2001 From: Satindar Dhillon Date: Wed, 4 May 2022 15:22:21 -0700 Subject: [PATCH 3/3] remove web reader loading overlay --- .../App/Views/WebReader/WebReaderContainer.swift | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/apple/OmnivoreKit/Sources/App/Views/WebReader/WebReaderContainer.swift b/apple/OmnivoreKit/Sources/App/Views/WebReader/WebReaderContainer.swift index 182f3aca2..2a102607a 100644 --- a/apple/OmnivoreKit/Sources/App/Views/WebReader/WebReaderContainer.swift +++ b/apple/OmnivoreKit/Sources/App/Views/WebReader/WebReaderContainer.swift @@ -14,7 +14,6 @@ import WebKit @State var safariWebLink: SafariWebLink? @State private var navBarVisibilityRatio = 1.0 @State private var showDeleteConfirmation = false - @State private var showOverlay = true @State private var progressViewOpacity = 0.0 @State var increaseFontActionID: UUID? @State var decreaseFontActionID: UUID? @@ -157,21 +156,6 @@ import WebKit annotationSaveTransactionID: $annotationSaveTransactionID, annotation: $annotation ) - .overlay( - Group { - if showOverlay { - Color.systemBackground - .transition(.opacity) - .onAppear { - DispatchQueue.main.asyncAfter(deadline: .now() + .milliseconds(100)) { - withAnimation(.linear(duration: 0.2)) { - showOverlay = false - } - } - } - } - } - ) .sheet(item: $safariWebLink) { SafariView(url: $0.url) }