Skip to content

Commit d38b023

Browse files
authored
Add ConnectionError; make isSiteUnreachable cover DNS + connection failures (#1524)
* Narrow NonExistentSiteError to DNS-only A refused connection (the host resolves, but nothing is listening) was classified as NonExistentSiteError on Swift but not on Kotlin or reqwest, so the isSiteUnreachable predicate from #1488 returned a different answer per platform for the same outage. Move Swift's .cannotConnectToHost out of the non-existent-site set, leaving NonExistentSiteError to mean a DNS-resolution failure. The follow-up commit gives refused/unreachable connections a dedicated ConnectionError reason across all three executors. Refs #1495. * Add ConnectionError variant; broaden isSiteUnreachable to cover it Refused and unreachable connections were lumped into the generic HttpError (Kotlin, reqwest) or, before the previous commit, NonExistentSiteError (Swift). None of the existing reasons isolate "the host resolved, but a connection couldn't be established." Add a ConnectionError { reason } variant and route connection-establishment failures to it across all three executors: - reqwest: io::ErrorKind::{ConnectionRefused, HostUnreachable, NetworkUnreachable} - Kotlin: ConnectException, NoRouteToHostException - Swift: URLError.cannotConnectToHost Broaden is_site_unreachable to match NonExistentSiteError | ConnectionError, so it is one portable "couldn't reach the site" signal that answers the same on every executor. Callers that need to tell a bad domain from a down server match the two variants directly. A connect timeout is deliberately excluded: neither reqwest's is_timeout() nor OkHttp's SocketTimeoutException separates a connect timeout from a read timeout, so those stay HttpTimeoutError. Refs #1495.
1 parent 8eacdee commit d38b023

11 files changed

Lines changed: 276 additions & 61 deletions

File tree

CHANGELOG.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1515
- WordPress.com `POST /sites/<site_id>/domains/primary` endpoint for setting a site's primary domain
1616
- WordPress.com `GET /sites/<site_id>/stats/post/<post_id>` endpoint for a post's view history, like count, comment count, and metadata
1717
- WordPress.com `GET /sites/<site_id>/plans` endpoint for listing the plans a site can buy, priced for that site, with the plan it's currently on flagged.
18-
- `RequestExecutionErrorReason` gained `isSiteUnreachable` and `isDeviceOffline` for distinguishing a site that could not be reached (most reliably, a DNS failure) from a device with no network connection. Previously consumers had to match the `NonExistentSiteError` / `DeviceIsOfflineError` variants themselves. Available on both platforms as properties on the reason, which is reachable from `WpRequestResult.RequestExecutionFailed` and `WpApiException.RequestExecutionFailed` on Kotlin. Swift additionally exposes both as convenience properties on `WpApiError` and `RequestExecutionError`.
18+
- `RequestExecutionErrorReason` gained `isSiteUnreachable` and `isDeviceOffline` for distinguishing a site that could not be reached (the host did not resolve, or a connection to it could not be established) from a device with no network connection. Previously consumers had to match the `NonExistentSiteError` / `DeviceIsOfflineError` variants themselves. Available on both platforms as properties on the reason, which is reachable from `WpRequestResult.RequestExecutionFailed` and `WpApiException.RequestExecutionFailed` on Kotlin. Swift additionally exposes both as convenience properties on `WpApiError` and `RequestExecutionError`.
19+
- **BREAKING:** `RequestExecutionErrorReason` gained a `ConnectionError` variant — the host resolved, but no connection to the server could be established (the connection was refused, there was no route, or the host was unreachable). Exhaustive matches over `RequestExecutionErrorReason` (Swift `switch`, Kotlin `when`, Rust `match`) must now handle the new case. `isSiteUnreachable` covers it alongside DNS failures, so it's a single portable "we couldn't reach the site" signal that returns the same answer on every executor; match `NonExistentSiteError` / `ConnectionError` directly to tell a bad domain apart from a server that's down. Refused and unreachable connections previously classified as the generic `HttpError` on Kotlin and reqwest. A connect timeout is not included; it remains `HttpTimeoutError`.
1920

2021
### Changed
2122

@@ -42,6 +43,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
4243

4344
- Swift: `WpRequestExecutor.sleep(millis:)` converted milliseconds to nanoseconds with the wrong factor (`* 1_000` instead of `* 1_000_000`), so it slept 1000× too short — a `Retry-After: 30` waited 30 ms instead of 30 s. `RetryAfterMiddleware` then re-sent immediately, the server kept returning 429, and after `max_retries` the caller observed `MisconfiguredRateLimitError` where honoring the backoff would usually have succeeded. The executor now waits the full interval, and no longer risks a `fatalError` if the sleep's task is cancelled.
4445
- Swift: Classify invalid-SSL failures from the failed handshake's `SecTrust` (`URLError.failureURLPeerTrust`), via `SecTrustCopyCertificateChain`, instead of reading the undocumented `NSErrorPeerCertificateChainKey` `userInfo` string that has no public constant. Behavior is unchanged on every platform: iOS/macOS/tvOS still surface the presented certificate as `certificateNotValidForName`, and watchOS — which exposes no peer trust — still degrades to `genericSslError`. ([#1510](https://github.com/Automattic/wordpress-rs/issues/1510))
46+
- `isSiteUnreachable` now returns the same answer for a refused connection — the host resolves, but nothing is listening (server down, wrong port) — on every executor. Previously it was `NonExistentSiteError` on Swift (so `isSiteUnreachable` was `true`) but the generic `HttpError` on Kotlin and reqwest (so it was `false`); a refused connection is now a `ConnectionError` everywhere, which `isSiteUnreachable` covers. `NonExistentSiteError` is reserved for a DNS-resolution failure.
4547

4648
### Security
4749

native/kotlin/api/kotlin/src/integrationTest/kotlin/WpRequestExecutorTest.kt

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,38 @@ class WpRequestExecutorTest {
249249
assertTrue(loggedErrors.isEmpty(), "cancellation should not be logged as an error: $loggedErrors")
250250
}
251251

252+
@Test
253+
fun `refused connection is mapped to ConnectionError`() = runTest {
254+
// Reserve a port with a throwaway server, then shut it down so nothing is
255+
// listening — a connection to it is refused (ConnectException). That must
256+
// classify as ConnectionError, not the generic HttpError and not
257+
// NonExistentSiteError (which is reserved for DNS failures).
258+
val closedServer = MockWebServer()
259+
closedServer.start()
260+
val refusedUrl = closedServer.url("/wp-json")
261+
closedServer.shutdown()
262+
263+
val executor = WpRequestExecutor(
264+
httpClient = WpHttpClient.CustomOkHttpClient(OkHttpClient()),
265+
networkAvailabilityProvider = NetworkAvailabilityProvider { true }
266+
)
267+
268+
val apiClient = WpApiClient(
269+
wpOrgSiteApiRootUrl = URI(refusedUrl.toString()).toURL(),
270+
authProvider = WpAuthenticationProvider.none(),
271+
requestExecutor = executor
272+
)
273+
274+
val result = apiClient.request { requestBuilder ->
275+
requestBuilder.users().listWithEditContext(params = UserListParams())
276+
}
277+
278+
assertIs<WpRequestResult.RequestExecutionFailed<*>>(result)
279+
assertIs<RequestExecutionErrorReason.ConnectionError>(
280+
(result as WpRequestResult.RequestExecutionFailed<*>).reason
281+
)
282+
}
283+
252284
/**
253285
* Resolves media fixtures (e.g. `test_media.jpg`) from the test classpath so uploads can be
254286
* built without touching the real filesystem layout.

native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/RequestExecutionErrorReasonExtensions.kt

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,26 +8,30 @@ import uniffi.wp_api.requestExecutionErrorReasonIsSiteUnreachable
88
* Extension properties for classifying connectivity failures.
99
*
1010
* The request executor maps the underlying platform errors onto
11-
* [RequestExecutionErrorReason.NonExistentSiteError] and
11+
* [RequestExecutionErrorReason.NonExistentSiteError],
12+
* [RequestExecutionErrorReason.ConnectionError], and
1213
* [RequestExecutionErrorReason.DeviceIsOfflineError]. These properties expose
13-
* that distinction without requiring callers to match the variants themselves.
14+
* those distinctions without requiring callers to match the variants themselves.
1415
*
1516
* The reason is available as `reason` on `WpRequestResult.RequestExecutionFailed`
1617
* and on `WpApiException.RequestExecutionFailed`.
1718
*/
1819

1920
/**
20-
* Whether the site could not be reached — most reliably, the host did not
21-
* resolve.
21+
* Whether the site could not be reached at all — either its host did not resolve
22+
* ([RequestExecutionErrorReason.NonExistentSiteError]) or the host resolved but
23+
* no connection could be established
24+
* ([RequestExecutionErrorReason.ConnectionError]).
2225
*
23-
* Distinct from [isDeviceOffline]: this indicates a problem reaching *this
24-
* particular site*, not a loss of device connectivity.
26+
* This is a portable signal: every executor classifies both failure modes the
27+
* same way. To tell them apart — a domain that doesn't resolve vs. a server
28+
* that's down — match the two variants directly.
2529
*
26-
* Note that a refused connection (the host resolves, but nothing is listening)
27-
* is reported here as an HTTP error rather than an unreachable site, whereas
28-
* the Swift executor reports it as unreachable. Only a DNS failure is treated
29-
* as an unreachable site by every executor. A malformed site URL never reaches
30-
* this predicate; it surfaces as `WpApiException.SiteUrlParsingException`.
30+
* Distinct from [isDeviceOffline]: this indicates a problem reaching *this
31+
* particular site*, not a loss of device connectivity. A connect timeout is not
32+
* included (it stays [RequestExecutionErrorReason.HttpTimeoutError]); a malformed
33+
* site URL never reaches this predicate either — it surfaces as
34+
* `WpApiException.SiteUrlParsingException`.
3135
*/
3236
val RequestExecutionErrorReason.isSiteUnreachable: Boolean
3337
get() = requestExecutionErrorReasonIsSiteUnreachable(this)

native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/WpRequestExecutor.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,7 @@ class WpRequestExecutor @JvmOverloads constructor(
236236
} catch (e: NoRouteToHostException) {
237237
RequestExecutionErrorReason.noRouteToHost(e)
238238
} catch (e: ConnectException) {
239-
RequestExecutionErrorReason.HttpError(reason = "Connection failed: ${e.localizedMessage}")
239+
RequestExecutionErrorReason.ConnectionError(reason = "Connection failed: ${e.localizedMessage}")
240240
} catch (e: SocketTimeoutException) {
241241
RequestExecutionErrorReason.HttpTimeoutError
242242
} catch (e: InterruptedIOException) {

native/kotlin/example/composeApp/src/commonMain/kotlin/rs/wordpress/example/shared/ui/components/ErrorMessage.kt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ private fun RequestExecutionErrorReason.description(): String = when (this) {
3939
is RequestExecutionErrorReason.HttpTimeoutError -> "Request timed out"
4040
is RequestExecutionErrorReason.InvalidSslError -> "SSL error: $reason"
4141
is RequestExecutionErrorReason.NonExistentSiteError -> errorMessage ?: "Site not found"
42+
is RequestExecutionErrorReason.ConnectionError -> "Could not connect to the server: $reason"
4243
is RequestExecutionErrorReason.HttpAuthenticationRequiredError -> "Authentication required for $hostname"
4344
is RequestExecutionErrorReason.HttpAuthenticationRejectedError -> "Authentication rejected for $hostname"
4445
is RequestExecutionErrorReason.HttpForbiddenError -> "Access forbidden for $hostname"

native/swift/Sources/wordpress-api/Extensions.swift

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -50,20 +50,24 @@ extension WpApiError {
5050
}
5151

5252
extension RequestExecutionErrorReason {
53-
/// Whether the site could not be reached — most reliably, the host did not
54-
/// resolve.
53+
/// Whether the site could not be reached at all — either its host did not
54+
/// resolve (`NonExistentSiteError`) or the host resolved but no connection
55+
/// could be established (`ConnectionError`).
56+
///
57+
/// This is a portable signal: every executor classifies both failure modes
58+
/// the same way. To tell them apart — a domain that doesn't resolve vs. a
59+
/// server that's down — match `.nonExistentSiteError` and `.connectionError`
60+
/// directly.
5561
///
5662
/// Distinct from ``isDeviceOffline``: this indicates a problem reaching
5763
/// *this particular site*, not a loss of device connectivity.
5864
///
59-
/// - Note: A refused connection (the host resolves, but nothing is
60-
/// listening) is classified differently per platform — Swift reports it
61-
/// here, Kotlin reports it as an HTTP error. Only a DNS failure is treated
62-
/// as an unreachable site by every executor. A site URL rejected while
63-
/// parsing surfaces as `WpApiError.SiteUrlParsingError` and never reaches
64-
/// this predicate; a `.badURL` that only `URLSession` rejects at send time
65-
/// is classified here for lack of a dedicated invalid-URL case, though no
66-
/// known URL actually reaches that branch.
65+
/// - Note: A connect timeout is not included (it stays `HttpTimeoutError`). A
66+
/// site URL rejected while parsing surfaces as
67+
/// `WpApiError.SiteUrlParsingError` and never reaches this predicate; a
68+
/// `.badURL` that only `URLSession` rejects at send time is classified as
69+
/// `NonExistentSiteError` for lack of a dedicated invalid-URL case, though
70+
/// no known URL actually reaches that branch.
6771
public var isSiteUnreachable: Bool {
6872
requestExecutionErrorReasonIsSiteUnreachable(reason: self)
6973
}
@@ -88,11 +92,10 @@ public protocol CarriesRequestExecutionErrorReason {
8892
}
8993

9094
public extension CarriesRequestExecutionErrorReason {
91-
/// Whether the site could not be reached — most reliably, the host did not
92-
/// resolve.
95+
/// Whether the site could not be reached at all — the host did not resolve,
96+
/// or the connection could not be established.
9397
///
94-
/// See ``RequestExecutionErrorReason/isSiteUnreachable`` for the platform
95-
/// differences that apply.
98+
/// See ``RequestExecutionErrorReason/isSiteUnreachable``.
9699
var isSiteUnreachable: Bool {
97100
executionErrorReason?.isSiteUnreachable ?? false
98101
}

native/swift/Sources/wordpress-api/SafeRequestExecutor.swift

Lines changed: 47 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,10 @@ public final class WpRequestExecutor: SafeRequestExecutor {
9898
return handleNonExistentSiteError(error, for: request)
9999
}
100100

101+
if errorIsConnectionError(error) {
102+
return handleConnectionError(error, for: request)
103+
}
104+
101105
if errorIsDeviceIsOffline(error) {
102106
return handleDeviceIsOfflineError(error, for: request)
103107
}
@@ -225,6 +229,21 @@ public final class WpRequestExecutor: SafeRequestExecutor {
225229
)
226230
}
227231

232+
func handleConnectionError(
233+
_ error: Error,
234+
for request: NetworkRequestContent
235+
) -> Result<WpNetworkResponse, RequestExecutionError> {
236+
.failure(
237+
.RequestExecutionFailed(
238+
statusCode: nil,
239+
redirects: executorDelegate.redirects(for: request.requestId()),
240+
reason: .connectionError(reason: error.localizedDescription),
241+
requestUrl: request.url(),
242+
requestMethod: request.method()
243+
)
244+
)
245+
}
246+
228247
public func sleep(millis: UInt64) async {
229248
// `try?`: `Task.sleep` only throws on cancellation, which this non-throwing `sleep`
230249
// leaves to the request machinery to handle.
@@ -247,24 +266,42 @@ public final class WpRequestExecutor: SafeRequestExecutor {
247266
}
248267

249268
private func errorIsNonExistentSiteError(_ error: Error) -> Bool {
250-
// `.badURL` is grouped with the non-existent-site errors deliberately.
251-
// A malformed URL has no dedicated classification at the executor layer:
252-
// `RequestExecutionError` can't produce `WpApiError.SiteUrlParsingError`
253-
// (that's a parse-time error, one layer up) and `RequestExecutionErrorReason`
254-
// has no invalid-URL case, so `NonExistentSiteError` is the nearest fit.
255-
// In practice we could not construct a URL that reaches this branch:
256-
// request URLs are normalized by the Rust `url` crate before they arrive,
257-
// and modern Foundation repairs the leftovers (e.g. an invalid `%zz`
258-
// becomes `%25zz`) rather than raising `.badURL`. It's kept for completeness.
269+
// A refused connection (`.cannotConnectToHost` — the host resolves, but
270+
// nothing is listening) is deliberately *not* here: it is a failed
271+
// connection handled by `errorIsConnectionError`, matching the Kotlin and
272+
// reqwest executors. This keeps `NonExistentSiteError` — and the
273+
// `isSiteUnreachable` predicate built on it — a portable "the host does
274+
// not resolve" signal across platforms. See #1495.
275+
//
276+
// `.badURL` is grouped here deliberately. A malformed URL has no dedicated
277+
// classification at the executor layer: `RequestExecutionError` can't
278+
// produce `WpApiError.SiteUrlParsingError` (that's a parse-time error, one
279+
// layer up) and `RequestExecutionErrorReason` has no invalid-URL case, so
280+
// `NonExistentSiteError` is the nearest fit. In practice we could not
281+
// construct a URL that reaches this branch: request URLs are normalized by
282+
// the Rust `url` crate before they arrive, and modern Foundation repairs
283+
// the leftovers (e.g. an invalid `%zz` becomes `%25zz`) rather than raising
284+
// `.badURL`. It's kept for completeness.
259285
[
260286
.badURL,
261-
.cannotConnectToHost,
262287
.cannotFindHost,
263288
.dnsLookupFailed
264289
]
265290
.contains((error as? URLError)?.code)
266291
}
267292

293+
private func errorIsConnectionError(_ error: Error) -> Bool {
294+
// A failed connection: the host resolves, but nothing accepts the
295+
// connection (server down, wrong port, not listening, or no route). It
296+
// maps to `ConnectionError` — the same classification the Kotlin
297+
// (`ConnectException` / `NoRouteToHostException`) and reqwest (io-error)
298+
// executors use. `isSiteUnreachable` covers it alongside a DNS failure.
299+
[
300+
.cannotConnectToHost
301+
]
302+
.contains((error as? URLError)?.code)
303+
}
304+
268305
private func errorIsDeviceIsOffline(_ error: Error) -> Bool {
269306
[
270307
.networkConnectionLost,

native/swift/Tests/wordpress-api/WordPressAPITests.swift

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,56 @@ struct WordPressAPITests {
9090
let details = try await api.apiRoot.get()
9191
#expect(details.data.siteUrlString() == "https://vanilla.wpmt.co")
9292
}
93+
94+
// A refused connection — the host resolves (loopback), but nothing is
95+
// listening on the port — must be classified as `.connectionError`, matching
96+
// the Kotlin and reqwest executors. It must *not* be `.nonExistentSiteError`,
97+
// which is reserved for DNS failures so `isSiteUnreachable` stays a portable
98+
// "the host does not resolve" signal across platforms. See #1495.
99+
@Test(.enabled(if: !isLinux()))
100+
func testRefusedConnectionIsConnectionError() async throws {
101+
// Port 1 on loopback is privileged, so nothing is bound in any test
102+
// environment, and the OS refuses the connection immediately
103+
// (`URLError.cannotConnectToHost`).
104+
let api = try WordPressAPI(
105+
siteInfo: .selfHosted(
106+
siteUrl: ParsedUrl.parse(input: "http://127.0.0.1:1"),
107+
apiRoot: ParsedUrl.parse(input: "http://127.0.0.1:1/wp-json")
108+
),
109+
authenticationProvider: .none(),
110+
executor: WpRequestExecutor(urlSession: .init(configuration: .ephemeral)),
111+
middlewarePipeline: .default,
112+
appNotifier: nil
113+
)
114+
115+
await #expect(
116+
performing: {
117+
_ = try await api.apiRoot.get()
118+
},
119+
throws: { error in
120+
guard
121+
let apiError = error as? WpApiError,
122+
case .RequestExecutionFailed(
123+
statusCode: _,
124+
redirects: _,
125+
reason: let reason,
126+
requestUrl: _,
127+
requestMethod: _
128+
) = apiError
129+
else {
130+
Issue.record("Expected WpApiError.RequestExecutionFailed, got: \(error)")
131+
return false
132+
}
133+
134+
guard case .connectionError = reason else {
135+
Issue.record("A refused connection must be `.connectionError`, got: \(reason)")
136+
return false
137+
}
138+
139+
return true
140+
}
141+
)
142+
}
93143
}
94144

95145
private actor CounterMiddleware: Middleware {

0 commit comments

Comments
 (0)