wordpress-mobile / wordpress-mobile/GutenbergKit
Ambiguous request failures should degrade optional dependencies rather than fail the editor load
Nobody has claimed this yet.
- Dominant language
- JavaScript
- Stars
- 29
- Forks
- 6
- Avg merge
- 1d 9h
- Merged PRs (30d)
- 41
Description
Description
A transient server failure on an optional dependency currently fails the entire editor load, even though the editor has a perfectly good degraded mode for it.
On iOS this is partly addressed: networkFallbackMode == .automatic degrades to empty dependencies, but only for a fixed set of URLError codes (EditorService.swift#L211-L221). A 5xx or 429 never qualifies. On Android there is no equivalent concept at all — every failure is fatal.
Reproduction (Android, cold cache)
- Disable airplane mode
- Load the site's Editor Configuration (discovery succeeds, theme styles = ON)
- Tap Clear Preload Cache
- Enable airplane mode
- Open the editor
Result: Failed to load editor; unable to resolve host…
The same steps on iOS produce an error with fallback .disabled, and default styles with .automatic. Android has no way to get the second outcome.
For contrast, these already work on Android and should keep working:
- Discovery fails first (offline from the start) → capability negotiated off → default styles
- Warm cache + offline → cached theme styles applied
Proposed change
Replace isNetworkError with a predicate asking "is this failure non-authoritative?" rather than "did we fail to reach the server." The current name becomes inaccurate once 5xx and malformed responses qualify — neither is a network error.
Non-authoritative: URLError/IO failures (current set), 5xx, 429, malformed or undecodable body.
Authoritative, therefore excluded: 404 rest_no_route (handled in #602), 401/403.
Apply only to optional dependencies — editor settings and plugin assets, which have both a capability flag and a defined absent-value:
| Mode | Outcome |
|---|---|
| Disabled | Fatal, as today |
| Automatic | Cached value if present, else absent-value |
For required dependencies (post data, post types, active theme) there is no absent-value, so automatic fallback may only substitute a cache hit — never proceed without them. Automatic means "use what you have," not "proceed regardless."
The cache is already consulted before any request (iOS EditorService.swift#L224, Android EditorService.kt#L260-L269), so this only affects the cache-miss path.
Tasks
- iOS: widen the predicate beyond
URLError; rename to reflect what it asks - Android: introduce
NetworkFallbackModeand the equivalent branch inprepare() - Both: restrict degradation to optional dependencies
- Decide the Android default (see below)
Open question: Android's default
Defaulting Android to disabled preserves today's behavior exactly — the offline cases that work now never reach the fallback check, so there's no regression risk either way. It's a straightforward design choice, not a compatibility constraint.
Worth noting the iOS enum documents .disabled as "current default behavior," which reads more like inherited history than a deliberate decision. Choosing Android's default is a good moment to revisit iOS's too.
Also worth noting: the iOS demo can afford optimistic capability flags offline because .automatic exists; Android's demo instead fails closed to false in SiteCapabilitiesDiscovery. Two workarounds for the same missing capability.
Auth handling
401/403 should stay fatal under both modes. It's authoritative but not a capability answer — typically an expired or broken token, where silently degrading hides a fixable problem behind a subtly-wrong editor.
To make that actionable, the host needs more than an opaque error through editor(_:didFailToLoad:) (EditorViewControllerDelegate.swift#L20). WP-iOS currently downcasts to ClientError and string-matches wpError.code to classify errors for telemetry (PostGBKEditorViewController.swift#L133) — evidence the need is real, not hypothetical. Suggested:
public enum EditorLoadFailure: Error {
/// Credentials rejected. The host should re-authenticate and retry.
case unauthorized(underlying: Error)
/// A required resource was unavailable or the server failed.
case unavailable(underlying: Error)
}
The protocol's empty default implementation (#L81) also means an unopted host gets nothing. Android has no load-failure callback at all — worth adding for parity.
Out of scope
Whether GBK should hand auth classification to the host and let it refresh-and-retry rather than deciding fatal itself. WP-iOS has a token-refresh path GBK knows nothing about, but that needs retry hooks and re-entrancy handling during prepare(). Shipping the typed failure first keeps that open without a later signature change.
Related
Depends on #602 landing first — it removes 404 from this path by construction.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Read the linked iOS EditorService.swift and Android EditorService.kt, especially the cache checks and Android prepare() path, after confirming the dependency on #602. Trace how optional and required dependencies, fallback modes, and existing discovery/cache cases are handled. Done means both platforms distinguish non-authoritative failures, preserve fatal behavior for required or unauthorized resources, and cover the listed offline outcomes.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- kotlin, swift
- Domain
- api, mobile-dev
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 42/100