Swift executor URLSession error audit: wrong conversions, unchecked codes, and crash-instead-of-error paths

An audit of the Swift `WpRequestExecutor`'s URLSession error handling, in the same vein as Automattic/wordpress-rs#1491 and Automattic/wordpress-rs#1492: errors we convert to the **wrong** Rust error, URLError codes we could classify better but don't, and adjacent paths where a failure crashes instead of classifying at all. Filed as one umbrella issue for validation; individual findings can be split out once confirmed.

All findings exclude Automattic/wordpress-rs#1491 (`.timedOut`), Automattic/wordpress-rs#1492 (Kotlin cancellation), and the platform differences already documented in the `is_site_unreachable` / `is_device_offline` rustdoc (`.cannotConnectToHost`, `.badURL`). File references are to `native/swift/Sources/wordpress-api/SafeRequestExecutor.swift` unless noted; line numbers were verified against `fix/converge-executor-error-classification`.

## Provenance

Surfaced from a multi-agent audit (🤖 six independent review lenses over the executor, findings merged and then each checked by two adversarial verification passes against the code). 44 raw findings reduced to 23 confirmed; 2 were refuted during verification and 3 late additions could not be verified — both groups are listed at the bottom so they aren't silently dropped. Severities are provisional pending human validation.

---

## A. Converted to the wrong Rust error

### 1\. Every TLS failure with a peer certificate chain becomes `CertificateNotValidForName` — expired, self-signed, and unknown-root included

**Severity: high.** `errorIsHttpsError` (`:233-246`) matches five codes — `.secureConnectionFailed`, `.serverCertificateUntrusted`, `.serverCertificateHasBadDate`, `.serverCertificateNotYetValid`, `.serverCertificateHasUnknownRoot` — but `handleHttpsError` (`:172-208`) never inspects `urlError.code`: chain present → `.certificateNotValidForName`, chain missing → `.genericSslError`. An expired or self-signed certificate issued for the **correct** hostname — the common misconfigured self-hosted case — is reported as a hostname mismatch, often with `hostname == presentedHostnames[0]`, a self-contradictory payload. ❌

reqwest reserves `CertificateNotValidForName` for rustls's actual `NotValidForNameContext` and maps every other TLS error away from it (`wp_api/src/reqwest_request_executor.rs:230-247`). ✅

This also compounds with finding 15: an app seeing `certificateNotValidForName` may offer the `allowSSL(altNames:forCommonName:)` remediation (`:75-77`), steering users into trusting an expired/untrusted certificate they believed was a mere name mismatch.

**Fix:** map `.serverCertificateHasBadDate` / `.serverCertificateNotYetValid` / `.serverCertificateHasUnknownRoot` to `.genericSslError` (they are by definition not name problems). For the remaining two codes, only claim `certificateNotValidForName` when the request host is genuinely absent from the certificate's CN + SANs — `SslCertificateInfo` already exposes both (`wp_api/src/ssl.rs:73-79`).

**Caveat:** genuine name mismatches surface as `.serverCertificateUntrusted` on Darwin, so a per-code switch alone would break the existing mismatch test (`LoginTests.swift`, `testInvalidHTTPsFails`) — the host-vs-names comparison is required to keep true mismatches while fixing the false positives.

### 2\. `.networkConnectionLost` is classified as `DeviceIsOfflineError` even when the server severed the connection

**Severity: high.** Apple documents -1005 as "a client or server connection was severed in the middle of an in-progress load" — it fires for server/proxy RSTs, load-balancer idle timeouts, and the notorious iOS stale keep-alive reuse case, with the device fully online. Apps show offline banners or queue for connectivity restoration when a plain retry would succeed. Swift is the only executor reporting offline for a server-side abort: reqwest maps the equivalent io `UnexpectedEof` to `HttpError { "The server terminated the connection unexpectedly" }` and never constructs `DeviceIsOfflineError`; Kotlin only claims offline after `NetworkAvailabilityProvider` confirms the network is down.

**Fix:** keep `.notConnectedToInternet` as `DeviceIsOfflineError`. For `.networkConnectionLost`, either (a) emit `HttpError` matching reqwest, or (b) — the faithful fix, since -1005 also fires on genuine mid-request connectivity drops — gate on a connectivity signal (`NWPathMonitor`, mirroring Kotlin's provider) and emit `DeviceIsOfflineError` only when the path is actually unsatisfied.

**Caveat:** `.networkConnectionLost` is listed as a Swift offline signal in the new `is_device_offline` rustdoc, so the contract text and the variant-coverage tests from 8aa3cf20 must move with the fix. The `waitsForConnectivity` caveat from Automattic/wordpress-rs#1491 doesn't help here — -1005 is reported for established connections.

### 3\. All multipart field-construction failures flatten to `MediaFileNotFound`

**Severity: low.** The catch-all at `:618-629` rethrows **every** `MultipartFormField(fileAtPath:)` failure — EACCES, sandbox denial, I/O errors on external volumes — as `MediaFileNotFound(filePath:)`, discarding the underlying error carried by `MultipartFormError.inaccessbileFile`. A file that exists but is unreadable is reported as "not found", sending users looking for the wrong problem. Map only genuine absence (`CocoaError.fileReadNoSuchFile` / `ENOENT`) to `MediaFileNotFound`; surface the rest as `.genericError` with the underlying description. (reqwest is worse here — `Part::file(...).unwrap()` panics — but the Rust side is out of this audit's scope.)

## B. Unchecked URLError codes we could classify better

### 4\. `.dataNotAllowed`, `.internationalRoamingOff`, `.callIsActive` fall to `GenericError` instead of `DeviceIsOfflineError`

**Severity: medium.** All three mean the device cannot use the network right now; the site is not implicated. `.dataNotAllowed` is the high-frequency one (Wi-Fi off with the per-app cellular toggle disabled, carrier policy). `errorIsDeviceIsOffline` (`:267-273`) only checks two codes today. Adding these stays within the documented contract ("Swift derives it from the OS-reported `URLError` codes") and matches what Android callers effectively get through the `NetworkAvailabilityProvider` gate.

### 5\. `HttpError { reason }` is unconstructible from the Swift executor

**Severity: medium.** The direct sibling of Automattic/wordpress-rs#1491's shape: the Swift executor's full output vocabulary is `{ cancellationError, genericError, invalidSslError, nonExistentSiteError, deviceIsOfflineError }` — no construction site for `.httpError` exists under `native/swift/Sources`. Protocol-level codes (`.badServerResponse`, `.cannotParseResponse`, `.cannotDecodeRawData`, `.cannotDecodeContentData`, `.zeroByteResource`, `.requestBodyStreamExhausted`, `.dataLengthExceedsMaximum`) all flatten into `GenericError`, while both peer executors use `HttpError` for exactly this class (reqwest: io/h2-level failures; Kotlin: `ConnectException` / `NoRouteToHostException`). An `errorIsHttpProtocolError` predicate mapping those codes to `.httpError(reason: error.localizedDescription)` closes the gap with no vocabulary change and keeps `GenericError` meaning "truly unclassified". Related Rust-side response-format issues: Automattic/wordpress-rs#606, [#182]() (link, not duplicate).

### 6\. `.httpTooManyRedirects` / `.redirectToNonExistentLocation` land in `GenericError` and discard the recorded redirect chain

**Severity: medium.** A redirect loop produces `GenericError` with `redirects: nil` — even though `RequestExecutorDelegate.willPerformHTTPRedirection` recorded the full chain for exactly this diagnostic purpose, and the HTTPS/non-existent-site handlers do attach it. http↔https and www redirect loops from misconfigured `siteurl`/`home` values are a recurring real-world failure mode. Classify both codes as `HttpError` and attach `executorDelegate.redirects(for:)`.

**Caveat:** reqwest also sends redirect-policy errors to `GenericError` (no `is_redirect()` check), so this is an improvement over parity rather than a parity fix — ideally fix reqwest in the same pass.

### 7\. `.appTransportSecurityRequiresSecureConnection` has no classification

**Severity: medium.** An http:// site blocked by ATS fails before any connection is attempted and lands in `GenericError`. No existing variant fits: `NonExistentSiteError` is wrong (the site exists), `InvalidSslError` is wrong (no certificate was presented — the variant is documented as present-but-untrusted), and it is not offline. This wants a new variant (e.g. `InsecureConnectionNotAllowed { hostname }`) so apps can show the actionable "this site doesn't support HTTPS" guidance that Automattic/wordpress-rs#1192 asks for — this finding is the Swift-executor-side enabler for that issue. Apps setting `NSAllowsArbitraryLoads` never hit the code, which caps the severity.

### 8\. `.userAuthenticationRequired`, `.clientCertificateRequired`, `.clientCertificateRejected` fall to `GenericError` despite closer variants existing

**Severity: low.** `HttpAuthenticationRequiredError { hostname, method: Option }` already exists with an optional method, so `.userAuthenticationRequired` (authenticating proxies) can map there with `method: nil`. The mTLS codes could take either the auth variants or `InvalidSslError(.genericSslError)` — either beats `GenericError`. Occurrence is limited to authenticating proxies and mTLS-fronted corporate WordPress — rare but real.

### 9\. `.unsupportedURL` is not grouped with `.badURL`

**Severity: low.** `errorIsNonExistentSiteError` includes `.badURL` ("kept for completeness" per its own comment, `:248-265`) but omits its sibling `.unsupportedURL` (-1002), which therefore falls to `GenericError`. The two codes represent the same user situation — the entered site URL cannot be requested — and the branch comment's rationale applies verbatim. The rustdoc note documenting the `badURL` grouping should mention it too.

## C. SSL payload and platform gaps

### 10\. `presentedHostnames` carries only the leaf certificate's CN, omitting SANs

**Severity: medium.** `handleHttpsError` builds `presentedHostnames` as `[siteCertificate.commonName()]` even though the parsed `SslCertificateInfo` exports `alternativeNames()` over UniFFI. Modern certificates carry their identities in SANs — CN is often empty or unrelated — so the payload can present an empty or misleading name list. reqwest forwards the full presented-names list (`presented.to_vec()`). One-line fix; `LoginTests.swift` pins the CN-only shape and needs updating. Relates to Automattic/wordpress-rs#657 (that issue is test coverage; this is the payload), and becomes load-bearing if finding 1's host-vs-names comparison lands.

### 11\. A CN-less (SAN-only) leaf certificate is unparseable, so the intermediate CA's CN can be reported as the presented hostname

**Severity: medium.** `parse_certificate` requires a subject CN (the `?` at `wp_api/src/ssl.rs:23`) even though SANs parse independently. `getPeerCertificateChain` compactMaps the `nil` away, and `handleHttpsError` then treats element 0 of the surviving array as the site certificate — with a CN-less leaf and parseable intermediates, `presentedHostnames` becomes the intermediate's CN (e.g. `["R10"]`). The same parse failure means `allowSSL` exceptions silently stop matching CN-less leaves. CA/Browser Forum direction is to omit CN, so frequency will grow. Fix: tolerate a missing subject CN in the Rust parser, and stop assuming index 0 survived the compactMap.

### 12\. On Linux, the SSL predicate never fires — all certificate failures become `GenericError`

**Severity: medium.** Verified against swift-corelibs-foundation `swift-6.2.1-RELEASE`: the libcurl bridge (`MultiHandle.swift`, `urlErrorCode(for:)`) maps no curl SSL error to any `NSURLError` SSL code — `CURLE_PEER_FAILED_VERIFICATION`, `CURLE_SSL_CONNECT_ERROR`, expired/self-signed/wrong-name certs all fall to `NSURLErrorUnknown`. So `errorIsHttpsError` never matches on Linux and the Linux accommodation in `getPeerCertificateChain` (return `[]`, `:296-297`) is dead code. Similarly `.notConnectedToInternet` is never produced there. At minimum, the platform-differences rustdoc (which covers Kotlin/reqwest/Darwin but says nothing about Linux SSL) should document the degradation; the durable fix is an upstream corelibs mapping. Steering Linux consumers to the reqwest executor is the pragmatic alternative.

### 13\. Peer chain extraction relies on the undocumented `"NSErrorPeerCertificateChainKey"` string

**Severity: low.** `getPeerCertificateChain` reads a hard-coded CFNetwork-internal key (`:299`) with no public constant. The repo's own tests document that watchOS doesn't populate it, so every watchOS SSL failure already degrades to `.genericSslError`; if Apple stops setting the key elsewhere, iOS/macOS silently degrade the same way. The documented `NSURLErrorFailingURLPeerTrustErrorKey` (a `SecTrust`) + `SecTrustCopyCertificateChain` — the same API the delegate override already uses at `:354` — is the sturdier source and may restore detail on watchOS.

## D. Crash instead of classified error

### 14\. `buildURLRequest` force-unwraps `URL(string:)`, crashing where the documented `badURL → NonExistentSiteError` path should fire

**Severity: high.** `let url = URL(string: self.url())!` (`:530`). The package supports iOS 16 / macOS 13, where Foundation's strict parser returns `nil` for characters the Rust `url` crate legally leaves unencoded in paths and queries (`|`, `^`, `[`, `]`; query also `{`, `}`). A user-typed site URL like `https://example.com/blog|dev/` parses through `ParsedUrl`, reaches the executor as a valid `WpEndpointUrl`, and the force-unwrap traps — a hard crash from user input. Throwing `URLError(.badURL)` instead routes it through the existing dispatch to `NonExistentSiteError` with no new plumbing. Unreachable on iOS 17+/macOS 14+ (lenient parser); the in-file rationale at `:249-257` only reasons about modern Foundation, which doesn't hold on the declared minimums.

### 15\. The `allowSSL` server-trust override accepts the challenge without evaluating the trust

**Severity: high (security).** The delegate's `didReceive challenge` handler (`:347-363`) returns `(.useCredential, URLCredential(trust:))` as soon as the leaf certificate parses and its CN matches the allowlist — `SecTrustEvaluateWithError` is never called, so chain signature, issuer, and validity dates are never checked. A MITM can self-sign a certificate whose CN copies the legitimate certificate's CN (public information) and be accepted for every host previously passed to `allowSSL(altNames:forCommonName:)`. The comment above the storage states the feature exists to tolerate a hostname missing from an otherwise-valid certificate; the fix is to re-evaluate the trust with hostname checking relaxed but chain validation intact (`SecTrustSetPolicies` with `SecPolicyCreateSSL(true, nil)`, then `SecTrustEvaluateWithError`), falling through to `.performDefaultHandling` on failure. Compounds finding 1: apps steered into `allowSSL` by a bogus name-mismatch reason find the exception "works" precisely because validation is fully bypassed.

### 16\. `sleep(millis:)` sleeps 1000× too short, breaking every Retry-After backoff — and `try!`s

**Severity: medium.** `try! await Task.sleep(nanoseconds: millis * 1000)` (`:230`) — nanoseconds require `millis * 1_000_000`, so a `Retry-After: 30` sleeps 30ms. `RetryAfterMiddleware` re-sends immediately, the server keeps returning 429, and after `max_retries` the caller observes `MisconfiguredRateLimitError` where honoring the backoff would typically have succeeded. Swift is the only executor that doesn't actually wait (reqwest uses `Duration::from_millis`, Kotlin uses `delay(millis)`). ❌ The `try!` additionally converts a cancelled sleep into `fatalError` — currently latent (uniffi 0.32 never cancels the backing Task), but a loaded gun. Fix is one line: `try? await Task.sleep(nanoseconds: millis * 1_000_000)`.

### 17\. Multipart body serialization crashes on a failed stream read

**Severity: medium.** `writeMultipartFormData` feeds the result of `InputStream.read` straight into `Data(bytesNoCopy: &buffer, count: bytes, deallocator: .none)` (`MultipartForm.swift:157`) without checking it. `read` returns `-1` on failure — file deleted between the field-construction check and the stream open, or a mid-read I/O error from an external/file-provider/iCloud-evicted volume — and a negative count traps in `Data`'s initializer, crashing inside `perform()` instead of surfacing `MediaFileNotFound`. On `-1`, abort serialization and throw `MultipartFormError.inaccessbileFile`; on `0`, exit the loop.

### 18\. `WpNetworkResponse.init` uses `preconditionFailure` where the throwing error path already exists

**Severity: low.** `guard let response = response as? HTTPURLResponse else { preconditionFailure(...) }` (`Extensions.swift:17`). The initializer is already `throws` and is called inside `perform()`'s do-block, so `throw URLError(.badServerResponse)` degrades gracefully to `.genericError` today. Verified effectively unreachable for http(s) loads on Darwin and Linux — defense-in-depth, retained because the fix is one line. (The same verification cleared the completion-handler `data!`/`response!` force-unwraps on both platforms: no finding there.)

### 19\. `execute()` on an owner-invalidated injected session crashes with an uncatchable NSException

**Severity: low.** After the session owner calls `invalidateAndCancel`, the next `execute()`/`upload()` calls `session.dataTask`/`uploadTask` on the invalidated session, which raises Foundation's "task created in a session that has been invalidated" NSException — uncatchable from Swift. Requires caller misuse; worth a doc note that the injected `URLSession` must outlive the executor (or track `didBecomeInvalidWithError` and fail fast with `GenericError`). Incidentally, this lens confirmed the `.cancelled → CancellationError` mapping is honest: the library's own two cancel paths and an in-flight `invalidateAndCancel` all funnel there, and the challenge handler never returns `.cancelAuthenticationChallenge`, so no non-cancellation source of `.cancelled` exists in this executor. ✅

## E. Cancellation gaps (`CancellationError` silently never produced)

### 20\. `cancel(context:)` silently no-ops when no URLSession task exists within its 1-second window

**Severity: medium.** `cancelRequest(withId:)` (`:151-170`) looks up `session.allTasks`, then waits at most 1s for a `didCreateTask` notification; if no task appears, nothing records that the id was cancelled. During `RetryAfterMiddleware`'s backoff no task is alive for up to `max_retry_wait_seconds`, and the retry request's uuid is only added to the context after `cancel(context:)` snapshotted `context.requestIds()` — the retry is never even targeted. Because uniffi 0.32's generated Swift never cancels Rust futures on Swift Task cancellation, `cancel(context:)` is the **only** effective cancellation for calls through the Rust core (`fulfill(progress:)` wires `Progress.cancel()` → `cancel(context:)`). Net: cancelling a media upload during a 429 backoff cancels nothing — the retry runs, the upload completes, and the caller sees success with no `CancellationError`. Fix: latch cancelled ids in the executor and cancel-on-creation in the `didCreateTask` callback, replacing the fire-and-forget 1s Combine timeout.

### 21\. A Swift Task cancelled before the URLSession task is registered runs the request to completion

**Severity: medium.** In both `perform` implementations, `withTaskCancellationHandler`'s `onCancel` calls `cancellation.cancel()`, which nils a still-unset `_task`; `TaskCancellation` has no cancelled latch, and `withTaskCancellationHandler` invokes `onCancel` immediately when the surrounding Task is already cancelled — before the continuation body creates the URLSession task. A pre-cancelled or racing cancellation therefore cancels nothing: the request executes fully (a cancelled POST/DELETE still sends the mutation) and the caller receives success instead of `CancellationError`. Fix: latch `cancel()` and cancel any task assigned after the fact, and/or check `Task.isCancelled` before resuming.

### 22\. `cancelRequest(withId:)` is entirely compiled out where Combine is unavailable

**Severity: low.** The whole body sits inside `#if canImport(Combine)` (`:152`), including the `session.allTasks` lookup and `task?.cancel()` that need no Combine — only the notification fallback does. On Linux, `cancel(context:)` therefore cancels nothing, ever, and cancellation never produces `CancellationError`. Scope the `#if` to just the publisher fallback.

### 23\. The offline, cancelled, and generic-fallback branches hardcode `redirects: nil`

**Severity: low.** `handleHttpsError` and `handleNonExistentSiteError` attach `executorDelegate.redirects(for:)`; the other three failure branches don't, losing the redirect trail for a request that redirected and then died mid-transfer, was cancelled, or failed unclassified. `redirects(for:)` returns `nil` when no redirects occurred, so attaching it everywhere is behavior-neutral in the common case. Overlaps the payload half of finding 6; looks like an oversight rather than a decision.

---

## Refuted during verification

Recorded so they aren't re-reported later:

* **"A successful response with a non-ASCII header value is converted into a** `GenericError` **failure"** — the claimed path does not behave as described.
* **"File-read URLError codes from** `uploadTask(fromFile:)` **should map to** `MediaFileNotFound`**"** — refuted for this executor's configuration.

## Unverified (verification agents did not complete)

Plausible-only; treat with more suspicion than the 23 above:

* Multipart serialization ignores `OutputStream` `open()`/`write()` results, so a disk-full or I/O failure mid-write could silently truncate the body.
* On Linux the `RequestExecutorDelegate` may never be attached to any task or session, which would make redirect recording and `allowSSL` dead there.
* Injecting a background `URLSession` may crash at task creation (completion-handler APIs are unsupported on background sessions).

## Sub-issues

Each finding above is now split into its own sub-issue for individual validation: findings 1–23 map to #1498–#1520 in order (finding _N_ → issue _1497 + N_). Titles and current status are in the linked **Sub-issues** list.

## Related issues

Automattic/wordpress-rs#1491, Automattic/wordpress-rs#1492 (siblings from the same review area) · Automattic/wordpress-rs#657 (SAN test coverage — findings 10/11) · Automattic/wordpress-rs#1192 (no-HTTPS guidance — finding 7) · Automattic/wordpress-rs#606 / [#182]() (Rust-side response-format errors — finding 5)

Start by reading native/swift/Sources/wordpress-api/SafeRequestExecutor.swift, especially the error predicates and handlers, then compare the stated behavior with wp_api/src/reqwest_request_executor.rs and wp_api/src/ssl.rs. Run the referenced LoginTests.swift and variant-coverage tests, validate each finding against the current code, and split confirmed fixes into focused issues with updated tests and rustdoc where required.

Written by the indexing model from the issue text.