Skip to content

Dependency fetch has no duration bound and no cancellation path #667

Description

@jkmassel

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. fix(ios): keep the dependency fetch running when the editor is covered #651 removed dependencyTaskHandle and the viewDidDisappear cancel, deliberately — see that PR for why. deinit is unreachable while the task runs, because await self?.prepareEditor() holds a strong self across the call.
  • No cooperative cancellation. grep -rn "Task.isCancelled\|checkCancellation\|withTaskCancellationHandler" over Sources/Services/ and Stores/EditorAssetLibrary.swift returns nothing. Even with a handle, cancellation only ever took effect because URLSession's own methods are cancellation-aware.
  • No request timeout. EditorViewController.swift:215-218 builds EditorHTTPClient(urlSession: URLSession.shared, authHeader:); requestTimeout defaults to nil (EditorHTTPClient.swift:101) and configureRequest only sets timeoutInterval when non-nil (:186-188). So the only ceilings are URLSession.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.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    [Type] BugAn existing feature does not function as intended[Type] PerformanceRelated to performance effortsiOS

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions