Bubble up errors while loading Resources (#400)

Author: ettore

.github/workflows/checks.yml

@@ -33,10 +33,12 @@ jobs:
           git diff --exit-code Support/Carthage/Readium.xcodeproj
       - name: Build
         run: |
-          xcodebuild build-for-testing -scheme "$scheme" -destination "platform=$platform,name=$device"
+          set -eo pipefail
+          xcodebuild build-for-testing -scheme "$scheme" -destination "platform=$platform,name=$device" | if command -v xcpretty &> /dev/null; then xcpretty; else cat; fi
       - name: Test
         run: |
-          xcodebuild test-without-building -scheme "$scheme" -destination "platform=$platform,name=$device"
+          set -eo pipefail
+          xcodebuild test-without-building -scheme "$scheme" -destination "platform=$platform,name=$device" | if command -v xcpretty &> /dev/null; then xcpretty; else cat; fi
 
   lint:
     name: Lint
@@ -91,7 +93,8 @@ jobs:
         run: make dev lcp=${{ secrets.LCP_URL_SPM }}
       - name: Build
         run: |
-          xcodebuild build -scheme TestApp -destination "platform=$platform,name=$device"
+          set -eo pipefail
+          xcodebuild build -scheme TestApp -destination "platform=$platform,name=$device" | if command -v xcpretty &> /dev/null; then xcpretty; else cat; fi
 
   int-spm:
     name: Integration (Swift Package Manager)
@@ -118,7 +121,8 @@ jobs:
         run: make spm lcp=${{ secrets.LCP_URL_SPM }} commit=$commit_sha
       - name: Build
         run: |
-          xcodebuild build -scheme TestApp -destination "platform=$platform,name=$device"
+          set -eo pipefail
+          xcodebuild build -scheme TestApp -destination "platform=$platform,name=$device" | if command -v xcpretty &> /dev/null; then xcpretty; else cat; fi
 
   int-carthage:
     name: Integration (Carthage)
@@ -145,7 +149,8 @@ jobs:
         run: make carthage lcp=${{ secrets.LCP_URL_CARTHAGE }} commit=$commit_sha
       - name: Build
         run: |
-          xcodebuild build -scheme TestApp -destination "platform=$platform,name=$device"
+          set -eo pipefail
+          xcodebuild build -scheme TestApp -destination "platform=$platform,name=$device" | if command -v xcpretty &> /dev/null; then xcpretty; else cat; fi
 
   # Warning: This job cannot actually test the state of the current commit,
   # but will check that the latest branch/tag set in the Podspecs are valid.
@@ -168,4 +173,5 @@ jobs:
         run: make cocoapods lcp=${{ secrets.LCP_URL_COCOAPODS }} commit=$commit_sha
       - name: Build
         run: |
-          xcodebuild build -workspace TestApp.xcworkspace -scheme TestApp -destination "platform=$platform,name=$device"
+          set -eo pipefail
+          xcodebuild build -workspace TestApp.xcworkspace -scheme TestApp -destination "platform=$platform,name=$device" | if command -v xcpretty &> /dev/null; then xcpretty; else cat; fi

CHANGELOG.md

@@ -11,6 +11,7 @@ All notable changes to this project will be documented in this file. Take a look
 #### Navigator
 
 * The `AudioNavigator` API has been promoted to stable and ships with a new Preferences API.
+* The new `NavigatorDelegate.didFailToLoadResourceAt(_:didFailToLoadResourceAt:withError:)` delegate API notifies when an error occurs while loading a publication resource (contributed by [@ettore](https://github.com/readium/swift-toolkit/pull/400)).
 
 ### Fixed
 

Sources/Adapters/GCDWebServer/GCDHTTPServer.swift

@@ -17,14 +17,19 @@ public enum GCDHTTPServerError: Error {
 
 /// Implementation of `HTTPServer` using ReadiumGCDWebServer under the hood.
 public class GCDHTTPServer: HTTPServer, Loggable {
+    private struct EndpointHandler {
+        let resourceHandler: (HTTPServerRequest) -> Resource
+        let failureHandler: HTTPServer.FailureHandler?
+    }
+
     /// Shared instance of the HTTP server.
     public static let shared = GCDHTTPServer()
 
     /// The actual underlying HTTP server instance.
     private let server = ReadiumGCDWebServer()
 
     /// Mapping between endpoints and their handlers.
-    private var handlers: [HTTPServerEndpoint: (HTTPServerRequest) -> Resource] = [:]
+    private var handlers: [HTTPServerEndpoint: EndpointHandler] = [:]
 
     /// Mapping between endpoints and resource transformers.
     private var transformers: [HTTPServerEndpoint: [ResourceTransformer]] = [:]
@@ -84,7 +89,7 @@ public class GCDHTTPServer: HTTPServer, Loggable {
     }
 
     private func handle(request: ReadiumGCDWebServerRequest, completion: @escaping ReadiumGCDWebServerCompletionBlock) {
-        responseResource(for: request) { resource in
+        responseResource(for: request) { httpServerRequest, resource, failureHandler in
             let response: ReadiumGCDWebServerResponse
             switch resource.length {
             case let .success(length):
@@ -95,19 +100,26 @@ public class GCDHTTPServer: HTTPServer, Loggable {
                 )
             case let .failure(error):
                 self.log(.error, error)
-                response = ReadiumGCDWebServerErrorResponse(statusCode: error.httpStatusCode)
+                failureHandler?(httpServerRequest, error)
+                response = ReadiumGCDWebServerErrorResponse(
+                    statusCode: error.httpStatusCode,
+                    error: error
+                )
             }
 
-            completion(response)
+            completion(response) // goes back to ReadiumGCDWebServerConnection.m
         }
     }
 
-    private func responseResource(for request: ReadiumGCDWebServerRequest, completion: @escaping (Resource) -> Void) {
-        let completion = { resource in
+    private func responseResource(
+        for request: ReadiumGCDWebServerRequest,
+        completion: @escaping (HTTPServerRequest, Resource, FailureHandler?) -> Void
+    ) {
+        let completion = { request, resource, failureHandler in
             // Escape the queue to avoid deadlocks if something is using the
             // server in the handler.
             DispatchQueue.global().async {
-                completion(resource)
+                completion(request, resource, failureHandler)
             }
         }
 
@@ -130,36 +142,56 @@ public class GCDHTTPServer: HTTPServer, Loggable {
 
             for (endpoint, handler) in handlers {
                 if endpoint == pathWithoutAnchor {
-                    let resource = handler(HTTPServerRequest(url: request.url, href: nil))
-                    completion(transform(resource: resource, at: endpoint))
+                    let request = HTTPServerRequest(url: request.url, href: nil)
+                    let resource = handler.resourceHandler(request)
+                    completion(request,
+                               transform(resource: resource, at: endpoint),
+                               handler.failureHandler)
                     return
 
                 } else if path.hasPrefix(endpoint.addingSuffix("/")) {
-                    let resource = handler(HTTPServerRequest(
+                    let request = HTTPServerRequest(
                         url: request.url,
                         href: path.removingPrefix(endpoint.removingSuffix("/"))
-                    ))
-                    completion(transform(resource: resource, at: endpoint))
+                    )
+                    let resource = handler.resourceHandler(request)
+                    completion(request,
+                               transform(resource: resource, at: endpoint),
+                               handler.failureHandler)
                     return
                 }
             }
 
-            completion(FailureResource(link: Link(href: request.url.absoluteString), error: .notFound(nil)))
+            log(.warning, "Resource not found for request \(request)")
+            completion(HTTPServerRequest(url: request.url, href: nil),
+                       FailureResource(link: Link(href: request.url.absoluteString),
+                                       error: .notFound(nil)),
+                       nil)
         }
     }
 
     // MARK: HTTPServer
 
     public func serve(at endpoint: HTTPServerEndpoint, handler: @escaping (HTTPServerRequest) -> Resource) throws -> URL {
+        try serve(at: endpoint, handler: handler, failureHandler: nil)
+    }
+
+    public func serve(
+        at endpoint: HTTPServerEndpoint,
+        handler: @escaping (HTTPServerRequest) -> Resource,
+        failureHandler: FailureHandler?
+    ) throws -> URL {
         try queue.sync(flags: .barrier) {
             if case .stopped = state {
                 try start()
             }
+
             guard case let .started(port: _, baseURL: baseURL) = state else {
                 throw GCDHTTPServerError.serverNotStarted
             }
 
-            handlers[endpoint] = handler
+            handlers[endpoint] = EndpointHandler(resourceHandler: handler,
+                                                 failureHandler: failureHandler)
 
             return baseURL.appendingPathComponent(endpoint)
         }

Sources/Navigator/CBZ/CBZNavigatorViewController.swift

@@ -26,7 +26,7 @@ open class CBZNavigatorViewController: UIViewController, VisualNavigator, Loggab
 
     private let server: HTTPServer?
     private let publicationEndpoint: HTTPServerEndpoint?
-    private let publicationBaseURL: URL
+    private var publicationBaseURL: URL!
 
     public convenience init(
         publication: Publication,
@@ -39,52 +39,66 @@ open class CBZNavigatorViewController: UIViewController, VisualNavigator, Loggab
         }
 
         let publicationEndpoint: HTTPServerEndpoint?
-        let baseURL: URL
-        if let url = publication.baseURL {
+        let uuidEndpoint = UUID().uuidString
+        if publication.baseURL != nil {
             publicationEndpoint = nil
-            baseURL = url
         } else {
-            let endpoint = UUID().uuidString
-            publicationEndpoint = endpoint
-            baseURL = try httpServer.serve(at: endpoint, publication: publication)
+            publicationEndpoint = uuidEndpoint
         }
 
         self.init(
             publication: publication,
             initialLocation: initialLocation,
             httpServer: httpServer,
-            publicationEndpoint: publicationEndpoint,
-            publicationBaseURL: baseURL
+            publicationEndpoint: publicationEndpoint
         )
+
+        if let url = publication.baseURL {
+            publicationBaseURL = url
+        } else {
+            publicationBaseURL = try httpServer.serve(
+                at: uuidEndpoint,
+                publication: publication,
+                failureHandler: { [weak self] request, error in
+                    DispatchQueue.main.async { [weak self] in
+                        guard let self = self, let href = request.href else {
+                            return
+                        }
+                        self.delegate?.navigator(self, didFailToLoadResourceAt: href, withError: error)
+                    }
+                }
+            )
+        }
+
+        publicationBaseURL = URL(string: publicationBaseURL.absoluteString.addingSuffix("/"))!
     }
 
     @available(*, deprecated, message: "See the 2.5.0 migration guide to migrate the HTTP server")
     public convenience init(publication: Publication, initialLocation: Locator? = nil) {
         precondition(!publication.isRestricted, "The provided publication is restricted. Check that any DRM was properly unlocked using a Content Protection.")
-        guard let baseURL = publication.baseURL else {
+        guard publication.baseURL != nil else {
             preconditionFailure("No base URL provided for the publication. Add it to the HTTP server.")
         }
 
         self.init(
             publication: publication,
             initialLocation: initialLocation,
             httpServer: nil,
-            publicationEndpoint: nil,
-            publicationBaseURL: baseURL
+            publicationEndpoint: nil
         )
+
+        publicationBaseURL = URL(string: publicationBaseURL.absoluteString.addingSuffix("/"))!
     }
 
     private init(
         publication: Publication,
         initialLocation: Locator?,
         httpServer: HTTPServer?,
-        publicationEndpoint: HTTPServerEndpoint?,
-        publicationBaseURL: URL
+        publicationEndpoint: HTTPServerEndpoint?
     ) {
         self.publication = publication
         server = httpServer
         self.publicationEndpoint = publicationEndpoint
-        self.publicationBaseURL = URL(string: publicationBaseURL.absoluteString.addingSuffix("/"))!
 
         initialIndex = {
             guard let initialLocation = initialLocation, let initialIndex = publication.readingOrder.firstIndex(withHREF: initialLocation.href) else {

Sources/Navigator/EPUB/EPUBNavigatorViewController.swift

@@ -944,6 +944,16 @@ extension EPUBNavigatorViewController: EPUBNavigatorViewModelDelegate {
             }
         }
     }
+
+    func epubNavigatorViewModel(
+        _ viewModel: EPUBNavigatorViewModel,
+        didFailToLoadResourceAt href: String,
+        withError error: ResourceError
+    ) {
+        DispatchQueue.main.async {
+            self.delegate?.navigator(self, didFailToLoadResourceAt: href, withError: error)
+        }
+    }
 }
 
 extension EPUBNavigatorViewController: EPUBSpreadViewDelegate {