wordpress-mobile / wordpress-mobile/GutenbergKit
Dependency fetch has no duration bound and no cancellation path
Nobody has claimed this yet.
- Dominant language
- JavaScript
- Stars
- 29
- Forks
- 6
- Avg merge
- 1d 9h
- Merged PRs (30d)
- 41
Description
The editor's dependency fetch has no total-duration bound and no cancellation path. An editor the user has backed out of can hold its WKWebView until the network gives up on its own.
Detail
Three things compose:
- No handle. #651 removed
dependencyTaskHandleand theviewDidDisappearcancel, deliberately — see that PR for why.deinitis unreachable while the task runs, becauseawait self?.prepareEditor()holds a strongselfacross the call. - No cooperative cancellation.
grep -rn "Task.isCancelled\|checkCancellation\|withTaskCancellationHandler"overSources/Services/andStores/EditorAssetLibrary.swiftreturns nothing. Even with a handle, cancellation only ever took effect becauseURLSession's own methods are cancellation-aware. - No request timeout.
EditorViewController.swift:215-218buildsEditorHTTPClient(urlSession: URLSession.shared, authHeader:);requestTimeoutdefaults tonil(EditorHTTPClient.swift:101) andconfigureRequestonly setstimeoutIntervalwhen non-nil (:186-188). So the only ceilings areURLSession.shared's defaults: 60 s of inactivity per request, and a 7-day resource timeout.
How it bites
Cold cache, a connection that is slow but not dead (hotel wifi, congested cellular, captive portal). The user opens a post, watches the spinner, backs out. The editor, its WKWebView, its WKWebViewConfiguration, the GutenbergEditorController and the EditorService all stay alive until the fetch unwinds — up to ~60 s. Repeated attempts stack. When the fetch finally fails, the host receives didFailToLoad for an editor it dismissed a minute ago.
Why the obvious fix is probably wrong
Setting requestTimeout looks like the answer and likely isn't. EditorHTTPClient.swift:163-172 already documents the hazard: it is an inactivity timer, and a short value "would also fire during the silent window while WordPress synchronously generates image sub-sizes inside POST /wp/v2/media, orphaning the attachment server-side and duplicating it on retry." That comment calls out "no total-duration cap" as the deliberate current design, mirroring Android.
A total-duration deadline on EditorService.prepare() is the better-shaped knob.
The parent problem
The reason nothing can cancel this is that the endpoint is terminal. self.error is written in exactly one place (EditorViewController.swift:386) and never cleared; displayError (:1033-1043) renders a ContentUnavailableView with no retry action; hideError() (:1046) has zero callers anywhere in ios/.
Make the load restartable — a retry affordance on the error view, plus a public reload() for hosts — and the calculus inverts. A wrong cancellation costs a restart instead of the session, which is exactly the condition #649 names for adopting its ancestor-walk teardown detector. That is the fix worth doing; the deadline is the stopgap.
Found while reviewing #651.
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
Start with EditorViewController.swift, especially the request setup at 215-218, error handling at 386 and 1033-1046, and EditorService.prepare(). Read EditorHTTPClient.swift at 101 and 163-188, then review #651 and #649 for cancellation and teardown context. Done means the editor load can be cancelled or bounded safely and restarted through an error-view retry action or the host-facing reload() API.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- ios, swift
- Domain
- mobile
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100