Skip to content

Conversation

@dcalhoun
Copy link
Member

@dcalhoun dcalhoun commented Jan 28, 2026

What?

Makes the editor resilient to individual asset download failures during preloading, preventing the editor from failing to load when content blockers (like AdGuard) block analytics scripts.

Why?

Fix CMM-1179.

When AdGuard blocks stats.wp.com/w.js, the EditorAssetLibrary.buildBundle() method was failing entirely because it used withThrowingTaskGroup - any single asset failure would cause the entire bundle download to fail, resulting in an "Editor Error" screen with "bad URL" message.

How?

Production code (EditorAssetLibrary.swift)

  • Changed buildBundle() from withThrowingTaskGroup to withTaskGroup
  • Individual asset download failures are now caught, logged as warnings, and the download continues
  • Updated fetchAsset() to return Void instead of URL (return value was unused)

Test code (EditorAssetLibraryTests.swift)

  • Simplified EditorAssetLibraryMockHTTPClient by:
    • Making urlResponseHandler throwable: ((URL) throws -> Data)
    • Removing shouldThrowError and downloadResponse properties
    • Using the same handler for both perform() and download() methods
  • Added test for graceful handling of blocked assets

Testing Instructions

  1. Use a tool to block stats.wp.com in some manner—e.g., AdGuard, Proxyman.
  2. Open a post in an app using GutenbergKit
  3. Verify the editor loads successfully (previously showed "Editor Error")
  4. Check logs for warning about failed asset download

Automated Testing

  • Run make test-swift-package - all tests pass including new test buildBundle continues when individual asset downloads fail

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).
Compose `urlResponseHandler` for specific scenarios (e.g., unique
responses) rather than additional mock properties.
@dcalhoun dcalhoun added the [Type] Bug An existing feature does not function as intended label Jan 28, 2026
@dcalhoun dcalhoun marked this pull request as ready for review January 28, 2026 21:35
Comment on lines -847 to -848
var downloadResponse: URL?
var shouldThrowError: Error?
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed in favor of composing the urlResponseHandler for unique scenarios—e.g., URL-specific data and errors.

@dcalhoun dcalhoun requested a review from jkmassel January 28, 2026 21:35
Copy link
Contributor

@jkmassel jkmassel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed in Slack – this seems ok – when combined with #301 it'll make it so that we fetch any failed assets at runtime.

Address feedback. Improve the ability to test and debug this logic.

Co-authored-by: Jeremy Massel <1123407+jkmassel@users.noreply.github.com>
@dcalhoun dcalhoun merged commit 83792c3 into trunk Jan 29, 2026
12 checks passed
@dcalhoun dcalhoun deleted the fix/editor-preloading-survives-failed-downloads branch January 29, 2026 21:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Type] Bug An existing feature does not function as intended

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants