wordpress-mobile / wordpress-mobile/GutenbergKit

Dependency fetch has no duration bound and no cancellation path

Open
#667 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

[Type] Bug [Type] Performance iOS
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 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.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.