From 295bac078f6d98c6e58277503924926d0a6f6607 Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Wed, 28 Jan 2026 15:19:31 -0500 Subject: [PATCH 1/3] fix: Editor preloading survives failed downloads Previously, a single failed asset download caused the entire asset bundle fail. This led to the editor displaying an error screen. Now, individual asset download failures are caught independently and other asset downloads may proceed. This is useful for instances where an asset download intermittently fails or is blocked (e.g., ad blocker). --- .../Sources/Stores/EditorAssetLibrary.swift | 22 ++-- .../Stores/EditorAssetLibraryTests.swift | 104 ++++++++++++++++++ 2 files changed, 117 insertions(+), 9 deletions(-) diff --git a/ios/Sources/GutenbergKit/Sources/Stores/EditorAssetLibrary.swift b/ios/Sources/GutenbergKit/Sources/Stores/EditorAssetLibrary.swift index b084eda59..33d4c5a40 100644 --- a/ios/Sources/GutenbergKit/Sources/Stores/EditorAssetLibrary.swift +++ b/ios/Sources/GutenbergKit/Sources/Stores/EditorAssetLibrary.swift @@ -130,16 +130,22 @@ public actor EditorAssetLibrary { let editorRepresentation = try manifest.buildEditorRepresentation(for: self.configuration) try bundle.writeManifest(editorRepresentation: editorRepresentation) - try await withThrowingTaskGroup { group in + await withTaskGroup { group in let links = (manifest.scripts + manifest.styles).filter { self.isSupportedAsset($0) } - + for asset in links { group.addTask { - (asset, try await self.fetchAsset(url: asset, into: bundle)) + do { + try await self.fetchAsset(url: asset, into: bundle) + } catch { + // Log and continue - individual asset failures shouldn't block the editor + // This handles cases like content blockers blocking analytics scripts + log(.warn, "Failed to download asset \(asset.lastPathComponent): \(error.localizedDescription)") + } } } - - for try await _ in group { + + for await _ in group { complete += 1 await progress?(EditorProgress(completed: complete, total: links.count)) } @@ -150,20 +156,18 @@ public actor EditorAssetLibrary { /// Downloads a single asset and copies it into the temporary bundle directory. /// - private func fetchAsset(url: URL, into bundle: EditorAssetBundle) async throws -> URL { + private func fetchAsset(url: URL, into bundle: EditorAssetBundle) async throws { let tempUrl = try await logExecutionTime("Downloading \(url.lastPathComponent)") { try await httpClient.download(URLRequest(method: .GET, url: url)).0 } let destinationPath = bundle.bundleRoot.appending(path: url.path(percentEncoded: false)) let destinationParent = destinationPath.deletingLastPathComponent() - + // Ensure the destination directory exists try FileManager.default.createDirectory(at: destinationParent, withIntermediateDirectories: true) try FileManager.default.copyItem(at: tempUrl, to: destinationPath) - - return destinationPath } /// Checks if the given `url` is eligible to be downloaded into the local bundle diff --git a/ios/Tests/GutenbergKitTests/Stores/EditorAssetLibraryTests.swift b/ios/Tests/GutenbergKitTests/Stores/EditorAssetLibraryTests.swift index 0e354a441..7f24d27da 100644 --- a/ios/Tests/GutenbergKitTests/Stores/EditorAssetLibraryTests.swift +++ b/ios/Tests/GutenbergKitTests/Stores/EditorAssetLibraryTests.swift @@ -816,6 +816,46 @@ struct EditorAssetLibraryTests { #expect(progressTracker.count == 1) #expect(progressTracker.updates.first?.total == 1) } + + @Test("buildBundle continues when individual asset downloads fail") + func buildBundleContinuesWhenAssetDownloadsFail() async throws { + let manifestJSON = """ + { + "scripts": "", + "styles": "", + "allowed_block_types": ["core/paragraph"] + } + """ + + let mockClient = EditorAssetLibrarySelectiveFailureHTTPClient() + mockClient.urlResponseHandler = { _ in Data(manifestJSON.utf8) } + // Simulate content blocker blocking stats.js + mockClient.urlsToFail = [URL(string: "https://blocked.com/stats.js")!] + + let library = makeLibrary(httpClient: mockClient, cachePolicy: .ignore) + + let manifest = try await library.fetchManifest() + + // Should NOT throw - individual asset failures are caught and logged + let bundle = try await library.buildBundle(for: manifest) + + // Bundle should be created successfully + #expect(!bundle.id.isEmpty) + + // Verify progress was reported for all assets (including the failed one) + #expect(mockClient.downloadCallCount == 3) + + // Verify the successful assets were downloaded + let bundleRoot = await library.bundleRoot(for: bundle) + let goodScriptPath = bundleRoot.appending(path: "/good-script.js") + let stylePath = bundleRoot.appending(path: "/style.css") + #expect(FileManager.default.fileExists(at: goodScriptPath)) + #expect(FileManager.default.fileExists(at: stylePath)) + + // The failed asset should not exist + let failedScriptPath = bundleRoot.appending(path: "/stats.js") + #expect(!FileManager.default.fileExists(at: failedScriptPath)) + } } // MARK: - Progress Tracker for Tests @@ -913,3 +953,67 @@ final class EditorAssetLibraryMockHTTPClient: EditorHTTPClientProtocol, @uncheck return (tempURL, response) } } + +// MARK: - Selective Failure Mock HTTP Client for Testing Asset Download Failures + +final class EditorAssetLibrarySelectiveFailureHTTPClient: EditorHTTPClientProtocol, @unchecked Sendable { + + var getCallCount = 0 + var downloadCallCount = 0 + var downloadedURLs: [URL] = [] + var urlsToFail: [URL] = [] + private let lock = NSLock() + + /// Handler for generating response data based on request URL. + var urlResponseHandler: ((URL) -> Data) = { _ in Data() } + + func perform(_ urlRequest: URLRequest) async throws -> (Data, HTTPURLResponse) { + let url = try #require(urlRequest.url) + + lock.withLock { + getCallCount += 1 + } + + let responseData = urlResponseHandler(url) + + let response = HTTPURLResponse( + url: url, + statusCode: 200, + httpVersion: "HTTP/1.1", + headerFields: nil + )! + + return (responseData, response) + } + + func download(_ urlRequest: URLRequest) async throws -> (URL, HTTPURLResponse) { + let url = urlRequest.url! + + lock.withLock { + downloadCallCount += 1 + downloadedURLs.append(url) + } + + // Simulate failure for specified URLs (like content blockers blocking analytics) + if urlsToFail.contains(url) { + throw URLError(.badURL, userInfo: [ + NSLocalizedDescriptionKey: "bad URL", + NSURLErrorFailingURLStringErrorKey: url.absoluteString + ]) + } + + let response = HTTPURLResponse( + url: url, + statusCode: 200, + httpVersion: "HTTP/1.1", + headerFields: nil + )! + + // Create a temporary file with some content + let tempURL = FileManager.default.temporaryDirectory + .appendingPathComponent(UUID().uuidString) + try Data("mock content".utf8).write(to: tempURL) + + return (tempURL, response) + } +} From 8b18fe64a66c1723805f7631009d91ebba859f36 Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Wed, 28 Jan 2026 16:16:46 -0500 Subject: [PATCH 2/3] refactor: Simplify EditorAssetLibraryMockHTTPClient API surface Compose `urlResponseHandler` for specific scenarios (e.g., unique responses) rather than additional mock properties. --- .../Stores/EditorAssetLibraryTests.swift | 122 +++++------------- 1 file changed, 31 insertions(+), 91 deletions(-) diff --git a/ios/Tests/GutenbergKitTests/Stores/EditorAssetLibraryTests.swift b/ios/Tests/GutenbergKitTests/Stores/EditorAssetLibraryTests.swift index 7f24d27da..cd18ec507 100644 --- a/ios/Tests/GutenbergKitTests/Stores/EditorAssetLibraryTests.swift +++ b/ios/Tests/GutenbergKitTests/Stores/EditorAssetLibraryTests.swift @@ -676,7 +676,12 @@ struct EditorAssetLibraryTests { """ let mockClient = EditorAssetLibraryMockHTTPClient() - mockClient.urlResponseHandler = { _ in Data(manifestJSON.utf8) } + mockClient.urlResponseHandler = { url in + if url.path.contains("editor-assets") { + return Data(manifestJSON.utf8) + } + return Data("mock content".utf8) + } let library = makeLibrary(httpClient: mockClient, cachePolicy: .ignore) @@ -769,7 +774,12 @@ struct EditorAssetLibraryTests { """ let mockClient = EditorAssetLibraryMockHTTPClient() - mockClient.urlResponseHandler = { _ in Data(manifestJSON.utf8) } + mockClient.urlResponseHandler = { url in + if url.path.contains("editor-assets") { + return Data(manifestJSON.utf8) + } + return Data("mock content".utf8) + } let library = makeLibrary(httpClient: mockClient, cachePolicy: .ignore) @@ -827,10 +837,17 @@ struct EditorAssetLibraryTests { } """ - let mockClient = EditorAssetLibrarySelectiveFailureHTTPClient() - mockClient.urlResponseHandler = { _ in Data(manifestJSON.utf8) } - // Simulate content blocker blocking stats.js - mockClient.urlsToFail = [URL(string: "https://blocked.com/stats.js")!] + let mockClient = EditorAssetLibraryMockHTTPClient() + mockClient.urlResponseHandler = { url in + if url.path.contains("editor-assets") { + return Data(manifestJSON.utf8) + } + // Simulate content blocker blocking stats.js + if url.host == "blocked.com" { + throw URLError(.badURL) + } + return Data("mock content".utf8) + } let library = makeLibrary(httpClient: mockClient, cachePolicy: .ignore) @@ -884,8 +901,6 @@ final class EditorAssetLibraryMockHTTPClient: EditorHTTPClientProtocol, @uncheck var getCallCount = 0 var downloadCallCount = 0 var downloadedURLs: [URL] = [] - var downloadResponse: URL? - var shouldThrowError: Error? private let lock = NSLock() /// URLs requested via `perform(_:)`. Use this to verify which endpoints were called. @@ -894,8 +909,10 @@ final class EditorAssetLibraryMockHTTPClient: EditorHTTPClientProtocol, @uncheck lock.withLock { _requestedURLs } } - /// Handler for generating response data based on request URL. Defaults to returning empty data. - var urlResponseHandler: ((URL) -> Data) = { _ in Data() } + /// Handler for generating response data based on request URL. + /// Can throw to simulate failures for specific URLs. + /// Used by both `perform()` and `download()` methods. + var urlResponseHandler: ((URL) throws -> Data) = { _ in Data() } func perform(_ urlRequest: URLRequest) async throws -> (Data, HTTPURLResponse) { let url = try #require(urlRequest.url) @@ -905,11 +922,7 @@ final class EditorAssetLibraryMockHTTPClient: EditorHTTPClientProtocol, @uncheck _requestedURLs.append(url) } - if let error = shouldThrowError { - throw error - } - - let responseData = urlResponseHandler(url) + let responseData = try urlResponseHandler(url) let response = HTTPURLResponse( url: url, @@ -929,78 +942,10 @@ final class EditorAssetLibraryMockHTTPClient: EditorHTTPClientProtocol, @uncheck downloadedURLs.append(url) } - if let error = shouldThrowError { - throw error - } - - let response = HTTPURLResponse( - url: url, - statusCode: 200, - httpVersion: "HTTP/1.1", - headerFields: nil - )! - - // Create a temporary file with some content - let tempURL = - downloadResponse - ?? FileManager.default.temporaryDirectory - .appendingPathComponent(UUID().uuidString) - - if downloadResponse == nil { - try Data("mock content".utf8).write(to: tempURL) - } - - return (tempURL, response) - } -} - -// MARK: - Selective Failure Mock HTTP Client for Testing Asset Download Failures - -final class EditorAssetLibrarySelectiveFailureHTTPClient: EditorHTTPClientProtocol, @unchecked Sendable { - - var getCallCount = 0 - var downloadCallCount = 0 - var downloadedURLs: [URL] = [] - var urlsToFail: [URL] = [] - private let lock = NSLock() - - /// Handler for generating response data based on request URL. - var urlResponseHandler: ((URL) -> Data) = { _ in Data() } - - func perform(_ urlRequest: URLRequest) async throws -> (Data, HTTPURLResponse) { - let url = try #require(urlRequest.url) - - lock.withLock { - getCallCount += 1 - } - - let responseData = urlResponseHandler(url) + let data = try urlResponseHandler(url) - let response = HTTPURLResponse( - url: url, - statusCode: 200, - httpVersion: "HTTP/1.1", - headerFields: nil - )! - - return (responseData, response) - } - - func download(_ urlRequest: URLRequest) async throws -> (URL, HTTPURLResponse) { - let url = urlRequest.url! - - lock.withLock { - downloadCallCount += 1 - downloadedURLs.append(url) - } - - // Simulate failure for specified URLs (like content blockers blocking analytics) - if urlsToFail.contains(url) { - throw URLError(.badURL, userInfo: [ - NSLocalizedDescriptionKey: "bad URL", - NSURLErrorFailingURLStringErrorKey: url.absoluteString - ]) - } + let tempURL = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString) + try data.write(to: tempURL) let response = HTTPURLResponse( url: url, @@ -1009,11 +954,6 @@ final class EditorAssetLibrarySelectiveFailureHTTPClient: EditorHTTPClientProtoc headerFields: nil )! - // Create a temporary file with some content - let tempURL = FileManager.default.temporaryDirectory - .appendingPathComponent(UUID().uuidString) - try Data("mock content".utf8).write(to: tempURL) - return (tempURL, response) } } From 728a18880c8e585bbff86f7e81afdf15be72433a Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Thu, 29 Jan 2026 15:57:09 -0500 Subject: [PATCH 3/3] refactor: Reinstate `fetchAsset` return `URL` value Address feedback. Improve the ability to test and debug this logic. Co-authored-by: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> --- .../GutenbergKit/Sources/Stores/EditorAssetLibrary.swift | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ios/Sources/GutenbergKit/Sources/Stores/EditorAssetLibrary.swift b/ios/Sources/GutenbergKit/Sources/Stores/EditorAssetLibrary.swift index 33d4c5a40..51a7e908c 100644 --- a/ios/Sources/GutenbergKit/Sources/Stores/EditorAssetLibrary.swift +++ b/ios/Sources/GutenbergKit/Sources/Stores/EditorAssetLibrary.swift @@ -156,7 +156,8 @@ public actor EditorAssetLibrary { /// Downloads a single asset and copies it into the temporary bundle directory. /// - private func fetchAsset(url: URL, into bundle: EditorAssetBundle) async throws { + @discardableResult + private func fetchAsset(url: URL, into bundle: EditorAssetBundle) async throws -> URL { let tempUrl = try await logExecutionTime("Downloading \(url.lastPathComponent)") { try await httpClient.download(URLRequest(method: .GET, url: url)).0 } @@ -168,6 +169,8 @@ public actor EditorAssetLibrary { try FileManager.default.createDirectory(at: destinationParent, withIntermediateDirectories: true) try FileManager.default.copyItem(at: tempUrl, to: destinationPath) + + return destinationPath } /// Checks if the given `url` is eligible to be downloaded into the local bundle